diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index a6972e36a6be5b..52a13542fe3dc6 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -360,6 +360,9 @@ The buffer was not long enough. + + The HTTP request headers length exceeded the server limit of {0} bytes. + The HTTP response headers length exceeded the set limit of {0} bytes. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index b632761f1688c1..eca6e11d4f2833 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -50,6 +50,10 @@ internal sealed partial class Http2Connection : HttpConnectionBase private readonly Channel _writeChannel; private bool _lastPendingWriterShouldFlush; + // Server-advertised SETTINGS_MAX_HEADER_LIST_SIZE + // https://www.rfc-editor.org/rfc/rfc9113.html#section-6.5.2-2.12.1 + private uint _maxHeaderListSize = uint.MaxValue; // Defaults to infinite + // This flag indicates that the connection is shutting down and cannot accept new requests, because of one of the following conditions: // (1) We received a GOAWAY frame from the server // (2) We have exhaustead StreamIds (i.e. _nextStream == MaxStreamId) @@ -156,6 +160,14 @@ public Http2Connection(HttpConnectionPool pool, Stream stream) _nextPingRequestTimestamp = Environment.TickCount64 + _keepAlivePingDelay; _keepAlivePingPolicy = _pool.Settings._keepAlivePingPolicy; + uint maxHeaderListSize = _pool._lastSeenHttp2MaxHeaderListSize; + if (maxHeaderListSize > 0) + { + // Previous connections to the same host advertised a limit. + // Use this as an initial value before we receive the SETTINGS frame. + _maxHeaderListSize = maxHeaderListSize; + } + if (HttpTelemetry.Log.IsEnabled()) { HttpTelemetry.Log.Http20ConnectionEstablished(); @@ -800,6 +812,8 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f uint settingValue = BinaryPrimitives.ReadUInt32BigEndian(settings); settings = settings.Slice(4); + if (NetEventSource.Log.IsEnabled()) Trace($"Applying setting {(SettingId)settingId}={settingValue}"); + switch ((SettingId)settingId) { case SettingId.MaxConcurrentStreams: @@ -825,6 +839,11 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f // We don't actually store this value; we always send frames of the minimum size (16K). break; + case SettingId.MaxHeaderListSize: + _maxHeaderListSize = settingValue; + _pool._lastSeenHttp2MaxHeaderListSize = _maxHeaderListSize; + break; + default: // All others are ignored because we don't care about them. // Note, per RFC, unknown settings IDs should be ignored. @@ -1332,17 +1351,19 @@ private void WriteBytes(ReadOnlySpan bytes, ref ArrayBuffer headerBuffer) headerBuffer.Commit(bytes.Length); } - private void WriteHeaderCollection(HttpRequestMessage request, HttpHeaders headers, ref ArrayBuffer headerBuffer) + private int WriteHeaderCollection(HttpRequestMessage request, HttpHeaders headers, ref ArrayBuffer headerBuffer) { if (NetEventSource.Log.IsEnabled()) Trace(""); if (headers.HeaderStore is null) { - return; + return 0; } HeaderEncodingSelector? encodingSelector = _pool.Settings._requestHeaderEncodingSelector; + int headerListSize = headers.HeaderStore.Count * HeaderField.RfcOverhead; + ref string[]? tmpHeaderValuesArray = ref t_headerValues; foreach (KeyValuePair header in headers.HeaderStore) { @@ -1360,6 +1381,10 @@ private void WriteHeaderCollection(HttpRequestMessage request, HttpHeaders heade // The Connection, Upgrade and ProxyConnection headers are also not supported in HTTP2. if (knownHeader != KnownHeaders.Host && knownHeader != KnownHeaders.Connection && knownHeader != KnownHeaders.Upgrade && knownHeader != KnownHeaders.ProxyConnection) { + // The length of the encoded name may be shorter than the actual name. + // Ensure that headerListSize is always >= of the actual size. + headerListSize += knownHeader.Name.Length; + if (header.Key.KnownHeader == KnownHeaders.TE) { // HTTP/2 allows only 'trailers' TE header. rfc7540 8.1.2.2 @@ -1400,6 +1425,8 @@ private void WriteHeaderCollection(HttpRequestMessage request, HttpHeaders heade WriteLiteralHeader(header.Key.Name, headerValues, valueEncoding, ref headerBuffer); } } + + return headerListSize; } private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuffer) @@ -1430,9 +1457,9 @@ private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuff WriteIndexedHeader(_stream is SslStream ? H2StaticTable.SchemeHttps : H2StaticTable.SchemeHttp, ref headerBuffer); - if (request.HasHeaders && request.Headers.Host != null) + if (request.HasHeaders && request.Headers.Host is string host) { - WriteIndexedHeader(H2StaticTable.Authority, request.Headers.Host, ref headerBuffer); + WriteIndexedHeader(H2StaticTable.Authority, host, ref headerBuffer); } else { @@ -1450,9 +1477,11 @@ private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuff WriteIndexedHeader(H2StaticTable.PathSlash, pathAndQuery, ref headerBuffer); } + int headerListSize = 3 * HeaderField.RfcOverhead; // Method, Authority, Path + if (request.HasHeaders) { - WriteHeaderCollection(request, request.Headers, ref headerBuffer); + headerListSize += WriteHeaderCollection(request, request.Headers, ref headerBuffer); } // Determine cookies to send. @@ -1462,9 +1491,9 @@ private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuff if (cookiesFromContainer != string.Empty) { WriteBytes(KnownHeaders.Cookie.Http2EncodedName, ref headerBuffer); - Encoding? cookieEncoding = _pool.Settings._requestHeaderEncodingSelector?.Invoke(KnownHeaders.Cookie.Name, request); WriteLiteralHeaderValue(cookiesFromContainer, cookieEncoding, ref headerBuffer); + headerListSize += HttpKnownHeaderNames.Cookie.Length + HeaderField.RfcOverhead; } } @@ -1476,11 +1505,24 @@ private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuff { WriteBytes(KnownHeaders.ContentLength.Http2EncodedName, ref headerBuffer); WriteLiteralHeaderValue("0", valueEncoding: null, ref headerBuffer); + headerListSize += HttpKnownHeaderNames.ContentLength.Length + HeaderField.RfcOverhead; } } else { - WriteHeaderCollection(request, request.Content.Headers, ref headerBuffer); + headerListSize += WriteHeaderCollection(request, request.Content.Headers, ref headerBuffer); + } + + // The headerListSize is an approximation of the total header length. + // This is acceptable as long as the value is always >= the actual length. + // We must avoid ever sending more than the server allowed. + // This approach must be revisted if we ever support the dynamic table or compression when sending requests. + headerListSize += headerBuffer.ActiveLength; + + uint maxHeaderListSize = _maxHeaderListSize; + if ((uint)headerListSize > maxHeaderListSize) + { + throw new HttpRequestException(SR.Format(SR.net_http_request_headers_exceeded_length, maxHeaderListSize)); } } @@ -1553,10 +1595,10 @@ private async ValueTask SendHeadersAsync(HttpRequestMessage request // streams are created and started in order. await PerformWriteAsync(totalSize, (thisRef: this, http2Stream, headerBytes, endStream: (request.Content == null), mustFlush), static (s, writeBuffer) => { - if (NetEventSource.Log.IsEnabled()) s.thisRef.Trace(s.http2Stream.StreamId, $"Started writing. Total header bytes={s.headerBytes.Length}"); - s.thisRef.AddStream(s.http2Stream); + if (NetEventSource.Log.IsEnabled()) s.thisRef.Trace(s.http2Stream.StreamId, $"Started writing. Total header bytes={s.headerBytes.Length}"); + Span span = writeBuffer.Span; // Copy the HEADERS frame. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs index f1152867a59bc1..64a5911015292f 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs @@ -96,6 +96,14 @@ internal sealed class HttpConnectionPool : IDisposable private SemaphoreSlim? _http3ConnectionCreateLock; internal readonly byte[]? _http3EncodedAuthorityHostHeader; + // These settings are advertised by the server via SETTINGS_MAX_HEADER_LIST_SIZE. + // If we had previous connections to the same host in this pool, memorize the last value seen. + // This value is used as an initial value for new connections before they have a chance to observe the SETTINGS frame. + // Doing so avoids immediately exceeding the server limit on the first request, potentially causing the connection to be torn down. + // 0 means there were no previous connections, or they hadn't advertised this limit. + // There is no need to lock when updating these values - we're only interested in saving _a_ value, not necessarily the min/max/last. + internal uint _lastSeenHttp2MaxHeaderListSize; + /// For non-proxy connection pools, this is the host name in bytes; for proxies, null. private readonly byte[]? _hostHeaderValueBytes; /// Options specialized and cached for this pool and its key. diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs index a4e810aa6047a1..8c1c53117038d3 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs @@ -1205,6 +1205,107 @@ public sealed class SocketsHttpHandler_HttpClientHandler_MaxResponseHeadersLengt public SocketsHttpHandler_HttpClientHandler_MaxResponseHeadersLength_Test(ITestOutputHelper output) : base(output) { } } + [ConditionalClass(typeof(SocketsHttpHandler), nameof(SocketsHttpHandler.IsSupported))] + public sealed class SocketsHttpHandler_HttpClientHandler_MaxResponseHeadersLength_Http2 : HttpClientHandlerTestBase + { + public SocketsHttpHandler_HttpClientHandler_MaxResponseHeadersLength_Http2(ITestOutputHelper output) : base(output) { } + protected override Version UseVersion => HttpVersion.Version20; + + [Fact] + public async Task ServerAdvertisedMaxHeaderListSize_IsHonoredByClient() + { + const int Limit = 10_000; + + using HttpClientHandler handler = CreateHttpClientHandler(); + using HttpClient client = CreateHttpClient(handler); + + // We want to test that the client remembered the setting it received from the previous connection. + // To do this, we trick the client into using the same HttpConnectionPool for both server connections. + Uri lastServerUri = null; + + GetUnderlyingSocketsHttpHandler(handler).ConnectCallback = async (context, ct) => + { + Assert.Equal("foo", context.DnsEndPoint.Host); + + Socket socket = new Socket(SocketType.Stream, ProtocolType.Tcp) { NoDelay = true }; + try + { + await socket.ConnectAsync(lastServerUri.IdnHost, lastServerUri.Port); + return new NetworkStream(socket, ownsSocket: true); + } + catch + { + socket.Dispose(); + throw; + } + }; + + TaskCompletionSource waitingForLastRequest = new(TaskCreationOptions.RunContinuationsAsynchronously); + + await LoopbackServerFactory.CreateClientAndServerAsync(async uri => + { + lastServerUri = uri; + uri = new UriBuilder(uri) { Host = "foo", Port = 42 }.Uri; + + // Send a dummy request to ensure the SETTINGS frame has been received. + Assert.Equal("Hello world", await client.GetStringAsync(uri)); + + HttpRequestMessage request = CreateRequest(HttpMethod.Get, uri, UseVersion, exactVersion: true); + request.Headers.Add("Foo", new string('a', Limit)); + + Exception ex = await Assert.ThrowsAsync(() => client.SendAsync(request)); + Assert.Contains(Limit.ToString(), ex.Message); + + request = CreateRequest(HttpMethod.Get, uri, UseVersion, exactVersion: true); + for (int i = 0; i < Limit / 40; i++) + { + request.Headers.Add($"Foo-{i}", ""); + } + + ex = await Assert.ThrowsAsync(() => client.SendAsync(request)); + Assert.Contains(Limit.ToString(), ex.Message); + + await waitingForLastRequest.Task.WaitAsync(TimeSpan.FromSeconds(10)); + + // Ensure that the connection is still usable for requests that don't hit the limit. + Assert.Equal("Hello world", await client.GetStringAsync(uri)); + }, + async server => + { + var setting = new SettingsEntry { SettingId = SettingId.MaxHeaderListSize, Value = Limit }; + + using GenericLoopbackConnection connection = await ((Http2LoopbackServer)server).EstablishConnectionAsync(setting); + + await connection.ReadRequestDataAsync(); + await connection.SendResponseAsync(content: "Hello world"); + + waitingForLastRequest.SetResult(); + + // HandleRequestAsync will close the connection + await connection.HandleRequestAsync(content: "Hello world"); + }); + + await LoopbackServerFactory.CreateClientAndServerAsync(async uri => + { + lastServerUri = uri; + uri = new UriBuilder(uri) { Host = "foo", Port = 42 }.Uri; + + HttpRequestMessage request = CreateRequest(HttpMethod.Get, uri, UseVersion, exactVersion: true); + request.Headers.Add("Foo", new string('a', Limit)); + + Exception ex = await Assert.ThrowsAsync(() => client.SendAsync(request)); + Assert.Contains(Limit.ToString(), ex.Message); + + // Ensure that the connection is still usable for requests that don't hit the limit. + Assert.Equal("Hello world", await client.GetStringAsync(uri)); + }, + async server => + { + await server.HandleRequestAsync(content: "Hello world"); + }); + } + } + [SkipOnPlatform(TestPlatforms.Browser, "Socket is not supported on Browser")] public sealed class SocketsHttpHandler_HttpClientHandler_Authentication_Test : HttpClientHandler_Authentication_Test {