Merge

JamesNK · JamesNK · commit 82ceb0fb7fb4 · 2021-06-19T10:53:55.000+12:00
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3ControlStream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3ControlStream.cs
@@ -306,7 +306,7 @@ private ValueTask ProcessSettingsFrameAsync(ReadOnlySequence<byte> payload)
                 payload = payload.Slice(consumed);
 
                 var value = VariableLengthIntegerHelper.GetInteger(payload, out consumed, out _);
-                if (id == -1)
+                if (value == -1)
                 {
                     break;
                 }
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3FrameWriter.cs
@@ -23,7 +23,7 @@ internal class Http3FrameWriter
     {
         private readonly object _writeLock = new object();
 
-        private IEnumerator<KeyValuePair<string, string>>? _headersEnumerator;
+        private readonly int _maxTotalHeaderSize;
         private readonly PipeWriter _outputWriter;
         private readonly ConnectionContext _connectionContext;
         private readonly ITimeoutControl _timeoutControl;
@@ -36,10 +36,11 @@ internal class Http3FrameWriter
         private readonly Http3RawFrame _outgoingFrame;
         private readonly TimingPipeFlusher _flusher;
 
+        private IEnumerator<KeyValuePair<string, string>>? _headersEnumerator;
+        private int _headersTotalSize;
         // TODO update max frame size
         private uint _maxFrameSize = 10000; //Http3PeerSettings.MinAllowedMaxFrameSize;
         private byte[] _headerEncodingBuffer;
-
         private long _unflushedBytes;
         private bool _completed;
         private bool _aborted;
@@ -60,13 +61,12 @@ public Http3FrameWriter(PipeWriter output, ConnectionContext connectionContext,
             _outgoingFrame = new Http3RawFrame();
             _flusher = new TimingPipeFlusher(_outputWriter, timeoutControl, log);
             _headerEncodingBuffer = new byte[_maxFrameSize];
-            _qpackEncoder = new QPackEncoder();
 
-            // Note that QPack encoder doesn't react to settings change during a stream.
+            // Note that max total header size value doesn't react to settings change during a stream.
             // Unlikely to be a problem in practice:
             // - Settings rarely change after the start of a connection.
             // - Response header size limits are a best-effort requirement in the spec.
-            _qpackEncoder.MaxTotalHeaderSize = clientPeerSettings.MaxRequestHeaderFieldSectionSize > int.MaxValue
+            _maxTotalHeaderSize = clientPeerSettings.MaxRequestHeaderFieldSectionSize > int.MaxValue
                 ? int.MaxValue
                 : (int)clientPeerSettings.MaxRequestHeaderFieldSectionSize;
         }
@@ -280,10 +280,11 @@ public ValueTask<FlushResult> WriteResponseTrailersAsync(long streamId, HttpResp
                 try
                 {
                     _headersEnumerator = EnumerateHeaders(headers).GetEnumerator();
+                    _headersTotalSize = 0;
 
                     _outgoingFrame.PrepareHeaders();
                     var buffer = _headerEncodingBuffer.AsSpan();
-                    var done = QPackHeaderWriter.BeginEncode(_headersEnumerator, buffer, out var payloadLength);
+                    var done = QPackHeaderWriter.BeginEncode(_headersEnumerator, buffer, ref _headersTotalSize, out var payloadLength);
                     FinishWritingHeaders(payloadLength, done);
                 }
                 catch (QPackEncodingException ex)
@@ -335,7 +336,7 @@ internal void WriteResponseHeaders(int statusCode, IHeaderDictionary headers)
 
                     _outgoingFrame.PrepareHeaders();
                     var buffer = _headerEncodingBuffer.AsSpan();
-                    var done = QPackHeaderWriter.BeginEncode(statusCode, _headersEnumerator, buffer, out var payloadLength);
+                    var done = QPackHeaderWriter.BeginEncode(statusCode, _headersEnumerator, buffer, ref _headersTotalSize, out var payloadLength);
                     FinishWritingHeaders(payloadLength, done);
                 }
                 catch (QPackEncodingException ex)
@@ -356,12 +357,18 @@ private void FinishWritingHeaders(int payloadLength, bool done)
 
             while (!done)
             {
-                done = QPackHeaderWriter.Encode(_headersEnumerator!, buffer, out payloadLength);
+                done = QPackHeaderWriter.Encode(_headersEnumerator!, buffer, ref _headersTotalSize, out payloadLength);
                 _outgoingFrame.Length = payloadLength;
 
                 WriteHeaderUnsynchronized();
                 _outputWriter.Write(buffer.Slice(0, payloadLength));
             }
+
+            // https://quicwg.org/base-drafts/draft-ietf-quic-http.html#section-4.1.1.3
+            if (_headersTotalSize > _maxTotalHeaderSize)
+            {
+                throw new QPackEncodingException($"The encoded HTTP headers length exceeds the limit specified by the peer of {_maxTotalHeaderSize} bytes.");
+            }
         }
 
         public ValueTask CompleteAsync()
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/QPackHeaderWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/QPackHeaderWriter.cs
@@ -4,29 +4,28 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.Net.Http.HPack;
 using System.Net.Http.QPack;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
 {
     internal static class QPackHeaderWriter
     {
-        public static bool BeginEncode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, out int length)
+        public static bool BeginEncode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, ref int totalHeaderSize, out int length)
         {
             bool hasValue = enumerator.MoveNext();
             Debug.Assert(hasValue == true);
 
             buffer[0] = 0;
             buffer[1] = 0;
 
-            bool doneEncode = Encode(enumerator, buffer.Slice(2), out length);
+            bool doneEncode = Encode(enumerator, buffer.Slice(2), ref totalHeaderSize, out length);
 
             // Add two for the first two bytes.
             length += 2;
             return doneEncode;
         }
 
-        public static bool BeginEncode(int statusCode, IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, out int length)
+        public static bool BeginEncode(int statusCode, IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, ref int totalHeaderSize, out int length)
         {
             bool hasValue = enumerator.MoveNext();
             Debug.Assert(hasValue == true);
@@ -36,24 +35,28 @@ public static bool BeginEncode(int statusCode, IEnumerator<KeyValuePair<string,
             buffer[1] = 0;
 
             int statusCodeLength = EncodeStatusCode(statusCode, buffer.Slice(2));
-            bool done = Encode(enumerator, buffer.Slice(statusCodeLength + 2), throwIfNoneEncoded: false, out int headersLength);
+            totalHeaderSize += 42; // name (:status) + value (xxx) + overhead (32)
+
+            bool done = Encode(enumerator, buffer.Slice(statusCodeLength + 2), throwIfNoneEncoded: false, ref totalHeaderSize, out int headersLength);
             length = statusCodeLength + headersLength + 2;
 
             return done;
         }
 
-        public static bool Encode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, out int length)
+        public static bool Encode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, ref int totalHeaderSize, out int length)
         {
-            return Encode(enumerator, buffer, throwIfNoneEncoded: true, out length);
+            return Encode(enumerator, buffer, throwIfNoneEncoded: true, ref totalHeaderSize, out length);
         }
 
-        private static bool Encode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, bool throwIfNoneEncoded, out int length)
+        private static bool Encode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, bool throwIfNoneEncoded, ref int totalHeaderSize, out int length)
         {
             length = 0;
 
             do
             {
-                if (!QPackEncoder.EncodeLiteralHeaderFieldWithoutNameReference(enumerator.Current.Key, enumerator.Current.Value, buffer.Slice(length), out int headerLength))
+                var current = enumerator.Current;
+
+                if (!QPackEncoder.EncodeLiteralHeaderFieldWithoutNameReference(current.Key, current.Value, buffer.Slice(length), out int headerLength))
                 {
                     if (length == 0 && throwIfNoneEncoded)
                     {
@@ -62,6 +65,8 @@ private static bool Encode(IEnumerator<KeyValuePair<string, string>> enumerator,
                     return false;
                 }
 
+                // https://quicwg.org/base-drafts/draft-ietf-quic-http.html#section-4.1.1.3
+                totalHeaderSize += HeaderField.GetLength(current.Key.Length, current.Value.Length);
                 length += headerLength;
             } while (enumerator.MoveNext());
 
@@ -87,7 +92,7 @@ private static int EncodeStatusCode(int statusCode, Span<byte> buffer)
                     buffer[0] = 0b01011111;
                     buffer[1] = 0b00110000;
 
-                    ReadOnlySpan<byte> statusBytes = StatusCodes.ToStatusBytes(statusCode);
+                    ReadOnlySpan<byte> statusBytes = System.Net.Http.HPack.StatusCodes.ToStatusBytes(statusCode);
                     buffer[2] = (byte)statusBytes.Length;
                     statusBytes.CopyTo(buffer.Slice(3));
 
diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3TestBase.cs
@@ -583,10 +583,12 @@ public Http3RequestStream(Http3TestBase testBase, Http3Connection connection)
 
             public async Task SendHeadersAsync(IEnumerable<KeyValuePair<string, string>> headers, bool endStream = false)
             {
+                var headersTotalSize = 0;
+
                 var frame = new Http3RawFrame();
                 frame.PrepareHeaders();
                 var buffer = _headerEncodingBuffer.AsMemory();
-                var done = QPackHeaderWriter.BeginEncode(headers.GetEnumerator(), buffer.Span, out var length);
+                var done = QPackHeaderWriter.BeginEncode(headers.GetEnumerator(), buffer.Span, ref headersTotalSize, out var length);
                 Assert.True(done);
 
                 await SendFrameAsync(frame, buffer.Slice(0, length), endStream);
@@ -716,7 +718,7 @@ internal async Task<Dictionary<long, long>> ExpectSettingsAsync()
                     payload = payload.Slice(consumed);
 
                     var value = VariableLengthIntegerHelper.GetInteger(payload, out consumed, out _);
-                    if (id == -1)
+                    if (value == -1)
                     {
                         break;
                     }

Original file line number	Diff line number	Diff line change
`@@ -306,7 +306,7 @@ private ValueTask ProcessSettingsFrameAsync(ReadOnlySequence<byte> payload)`
`306`	`306`	`payload = payload.Slice(consumed);`
`307`	`307`
`308`	`308`	`var value = VariableLengthIntegerHelper.GetInteger(payload, out consumed, out _);`
`309`		`- if (id == -1)`
	`309`	`+ if (value == -1)`
`310`	`310`	`{`
`311`	`311`	`break;`
`312`	`312`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,29 +4,28 @@`
`4`	`4`	`using System;`
`5`	`5`	`using System.Collections.Generic;`
`6`	`6`	`using System.Diagnostics;`
`7`		`-using System.Net.Http.HPack;`
`8`	`7`	`using System.Net.Http.QPack;`
`9`	`8`
`10`	`9`	`namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3`
`11`	`10`	`{`
`12`	`11`	`internal static class QPackHeaderWriter`
`13`	`12`	`{`
`14`		`- public static bool BeginEncode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, out int length)`
	`13`	`+ public static bool BeginEncode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, ref int totalHeaderSize, out int length)`
`15`	`14`	`{`
`16`	`15`	`bool hasValue = enumerator.MoveNext();`
`17`	`16`	`Debug.Assert(hasValue == true);`
`18`	`17`
`19`	`18`	`buffer[0] = 0;`
`20`	`19`	`buffer[1] = 0;`
`21`	`20`
`22`		`- bool doneEncode = Encode(enumerator, buffer.Slice(2), out length);`
	`21`	`+ bool doneEncode = Encode(enumerator, buffer.Slice(2), ref totalHeaderSize, out length);`
`23`	`22`
`24`	`23`	`// Add two for the first two bytes.`
`25`	`24`	`length += 2;`
`26`	`25`	`return doneEncode;`
`27`	`26`	`}`
`28`	`27`
`29`		`- public static bool BeginEncode(int statusCode, IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, out int length)`
	`28`	`+ public static bool BeginEncode(int statusCode, IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, ref int totalHeaderSize, out int length)`
`30`	`29`	`{`
`31`	`30`	`bool hasValue = enumerator.MoveNext();`
`32`	`31`	`Debug.Assert(hasValue == true);`
`@@ -36,24 +35,28 @@ public static bool BeginEncode(int statusCode, IEnumerator<KeyValuePair<string,`
`36`	`35`	`buffer[1] = 0;`
`37`	`36`
`38`	`37`	`int statusCodeLength = EncodeStatusCode(statusCode, buffer.Slice(2));`
`39`		`- bool done = Encode(enumerator, buffer.Slice(statusCodeLength + 2), throwIfNoneEncoded: false, out int headersLength);`
	`38`	`+ totalHeaderSize += 42; // name (:status) + value (xxx) + overhead (32)`
	`39`	`+`
	`40`	`+ bool done = Encode(enumerator, buffer.Slice(statusCodeLength + 2), throwIfNoneEncoded: false, ref totalHeaderSize, out int headersLength);`
`40`	`41`	`length = statusCodeLength + headersLength + 2;`
`41`	`42`
`42`	`43`	`return done;`
`43`	`44`	`}`
`44`	`45`
`45`		`- public static bool Encode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, out int length)`
	`46`	`+ public static bool Encode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, ref int totalHeaderSize, out int length)`
`46`	`47`	`{`
`47`		`- return Encode(enumerator, buffer, throwIfNoneEncoded: true, out length);`
	`48`	`+ return Encode(enumerator, buffer, throwIfNoneEncoded: true, ref totalHeaderSize, out length);`
`48`	`49`	`}`
`49`	`50`
`50`		`- private static bool Encode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, bool throwIfNoneEncoded, out int length)`
	`51`	`+ private static bool Encode(IEnumerator<KeyValuePair<string, string>> enumerator, Span<byte> buffer, bool throwIfNoneEncoded, ref int totalHeaderSize, out int length)`
`51`	`52`	`{`
`52`	`53`	`length = 0;`
`53`	`54`
`54`	`55`	`do`
`55`	`56`	`{`
`56`		`- if (!QPackEncoder.EncodeLiteralHeaderFieldWithoutNameReference(enumerator.Current.Key, enumerator.Current.Value, buffer.Slice(length), out int headerLength))`
	`57`	`+ var current = enumerator.Current;`
	`58`	`+`
	`59`	`+ if (!QPackEncoder.EncodeLiteralHeaderFieldWithoutNameReference(current.Key, current.Value, buffer.Slice(length), out int headerLength))`
`57`	`60`	`{`
`58`	`61`	`if (length == 0 && throwIfNoneEncoded)`
`59`	`62`	`{`
`@@ -62,6 +65,8 @@ private static bool Encode(IEnumerator<KeyValuePair<string, string>> enumerator,`
`62`	`65`	`return false;`
`63`	`66`	`}`
`64`	`67`
	`68`	`+ // https://quicwg.org/base-drafts/draft-ietf-quic-http.html#section-4.1.1.3`
	`69`	`+ totalHeaderSize += HeaderField.GetLength(current.Key.Length, current.Value.Length);`
`65`	`70`	`length += headerLength;`
`66`	`71`	`} while (enumerator.MoveNext());`
`67`	`72`
`@@ -87,7 +92,7 @@ private static int EncodeStatusCode(int statusCode, Span<byte> buffer)`
`87`	`92`	`buffer[0] = 0b01011111;`
`88`	`93`	`buffer[1] = 0b00110000;`
`89`	`94`
`90`		`- ReadOnlySpan<byte> statusBytes = StatusCodes.ToStatusBytes(statusCode);`
	`95`	`+ ReadOnlySpan<byte> statusBytes = System.Net.Http.HPack.StatusCodes.ToStatusBytes(statusCode);`
`91`	`96`	`buffer[2] = (byte)statusBytes.Length;`
`92`	`97`	`statusBytes.CopyTo(buffer.Slice(3));`
`93`	`98`
Original file line number	Diff line number	Diff line change
`@@ -583,10 +583,12 @@ public Http3RequestStream(Http3TestBase testBase, Http3Connection connection)`
`583`	`583`
`584`	`584`	`public async Task SendHeadersAsync(IEnumerable<KeyValuePair<string, string>> headers, bool endStream = false)`
`585`	`585`	`{`
	`586`	`+ var headersTotalSize = 0;`
	`587`	`+`
`586`	`588`	`var frame = new Http3RawFrame();`
`587`	`589`	`frame.PrepareHeaders();`
`588`	`590`	`var buffer = _headerEncodingBuffer.AsMemory();`
`589`		`- var done = QPackHeaderWriter.BeginEncode(headers.GetEnumerator(), buffer.Span, out var length);`
	`591`	`+ var done = QPackHeaderWriter.BeginEncode(headers.GetEnumerator(), buffer.Span, ref headersTotalSize, out var length);`
`590`	`592`	`Assert.True(done);`
`591`	`593`
`592`	`594`	`await SendFrameAsync(frame, buffer.Slice(0, length), endStream);`
`@@ -716,7 +718,7 @@ internal async Task<Dictionary<long, long>> ExpectSettingsAsync()`
`716`	`718`	`payload = payload.Slice(consumed);`
`717`	`719`
`718`	`720`	`var value = VariableLengthIntegerHelper.GetInteger(payload, out consumed, out _);`
`719`		`- if (id == -1)`
	`721`	`+ if (value == -1)`
`720`	`722`	`{`
`721`	`723`	`break;`
`722`	`724`	`}`