From a1ffc5345bc6cf001fce18049c746598aa3f08c9 Mon Sep 17 00:00:00 2001 From: "ASP.NET CI" Date: Sun, 16 Sep 2018 12:19:46 -0700 Subject: [PATCH 1/3] Update dependencies.props [auto-updated: dependencies] --- build/dependencies.props | 46 ++++++++++++++++++++-------------------- korebuild-lock.txt | 4 ++-- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index 02d96e2ca8..d5f2663f1b 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -5,34 +5,34 @@ 0.10.13 - 2.2.0-preview3-35202 - 2.2.0-preview1-20180907.8 + 2.2.0-preview3-35252 + 2.2.0-preview1-20180911.1 1.10.0 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 - 2.2.0-preview3-35202 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 + 2.2.0-preview3-35252 2.1.1 2.0.9 2.1.3 2.2.0-preview2-26905-02 - 2.2.0-preview3-35202 + 2.2.0-preview3-35252 15.6.1 4.7.49 2.0.3 diff --git a/korebuild-lock.txt b/korebuild-lock.txt index 312f82f9a5..7124f37441 100644 --- a/korebuild-lock.txt +++ b/korebuild-lock.txt @@ -1,2 +1,2 @@ -version:2.2.0-preview1-20180907.8 -commithash:078918eb5c1f176ee1da351c584fb4a4d7491aa0 +version:2.2.0-preview1-20180911.1 +commithash:ddfecdfc6e8e4859db5a0daea578070b862aac65 From 025aca52df51f923a2d98406f947425cb1a533e2 Mon Sep 17 00:00:00 2001 From: John Luo Date: Thu, 13 Sep 2018 16:37:34 -0700 Subject: [PATCH 2/3] Port long Huffman encoding bug fix https://github.com/dotnet/corefx/pull/32043 --- .../Internal/Http2/HPack/HPackDecoder.cs | 2 +- .../Internal/Http2/HPack/Huffman.cs | 20 +++++++------ test/Kestrel.Core.Tests/HuffmanTests.cs | 28 ++++++++++++++----- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs b/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs index 84f91ca896..ac18c1962d 100644 --- a/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs +++ b/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs @@ -360,7 +360,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack { if (_huffman) { - return Huffman.Decode(_stringOctets, 0, _stringLength, dst); + return Huffman.Decode(new ReadOnlySpan(_stringOctets, 0, _stringLength), dst); } else { diff --git a/src/Kestrel.Core/Internal/Http2/HPack/Huffman.cs b/src/Kestrel.Core/Internal/Http2/HPack/Huffman.cs index f0d489c952..15f4d0bf50 100644 --- a/src/Kestrel.Core/Internal/Http2/HPack/Huffman.cs +++ b/src/Kestrel.Core/Internal/Http2/HPack/Huffman.cs @@ -303,24 +303,26 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack /// Decodes a Huffman encoded string from a byte array. /// /// The source byte array containing the encoded data. - /// The offset in the byte array where the coded data starts. - /// The number of bytes to decode. /// The destination byte array to store the decoded data. /// The number of decoded symbols. - public static int Decode(byte[] src, int offset, int count, byte[] dst) + public static int Decode(ReadOnlySpan src, Span dst) { - var i = offset; + var i = 0; var j = 0; var lastDecodedBits = 0; - while (i < count) + while (i < src.Length) { + // Note that if lastDecodeBits is 3 or more, then we will only get 5 bits (or less) + // from src[i]. Thus we need to read 5 bytes here to ensure that we always have + // at least 30 bits available for decoding. var next = (uint)(src[i] << 24 + lastDecodedBits); next |= (i + 1 < src.Length ? (uint)(src[i + 1] << 16 + lastDecodedBits) : 0); next |= (i + 2 < src.Length ? (uint)(src[i + 2] << 8 + lastDecodedBits) : 0); next |= (i + 3 < src.Length ? (uint)(src[i + 3] << lastDecodedBits) : 0); + next |= (i + 4 < src.Length ? (uint)(src[i + 4] >> (8 - lastDecodedBits)) : 0); var ones = (uint)(int.MinValue >> (8 - lastDecodedBits - 1)); - if (i == count - 1 && lastDecodedBits > 0 && (next & ones) == ones) + if (i == src.Length - 1 && lastDecodedBits > 0 && (next & ones) == ones) { // The remaining 7 or less bits are all 1, which is padding. // We specifically check that lastDecodedBits > 0 because padding @@ -332,8 +334,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack // The longest possible symbol size is 30 bits. If we're at the last 4 bytes // of the input, we need to make sure we pass the correct number of valid bits // left, otherwise the trailing 0s in next may form a valid symbol. - var validBits = Math.Min(30, (8 - lastDecodedBits) + (count - i - 1) * 8); - var ch = Decode(next, validBits, out var decodedBits); + var validBits = Math.Min(30, (8 - lastDecodedBits) + (src.Length - i - 1) * 8); + var ch = DecodeValue(next, validBits, out var decodedBits); if (ch == -1) { @@ -377,7 +379,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack /// /// The number of bits decoded from . /// The decoded symbol. - public static int Decode(uint data, int validBits, out int decodedBits) + internal static int DecodeValue(uint data, int validBits, out int decodedBits) { // The code below implements the decoding logic for a canonical Huffman code. // diff --git a/test/Kestrel.Core.Tests/HuffmanTests.cs b/test/Kestrel.Core.Tests/HuffmanTests.cs index cbe87cd4d9..dfae6afe6e 100644 --- a/test/Kestrel.Core.Tests/HuffmanTests.cs +++ b/test/Kestrel.Core.Tests/HuffmanTests.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Text; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack; using Xunit; @@ -67,7 +68,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public void HuffmanDecodeArray(byte[] encoded, byte[] expected) { var dst = new byte[expected.Length]; - Assert.Equal(expected.Length, Huffman.Decode(encoded, 0, encoded.Length, dst)); + Assert.Equal(expected.Length, Huffman.Decode(new ReadOnlySpan(encoded), dst)); Assert.Equal(expected, dst); } @@ -87,7 +88,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [MemberData(nameof(_longPaddingData))] public void ThrowsOnPaddingLongerThanSevenBits(byte[] encoded) { - var exception = Assert.Throws(() => Huffman.Decode(encoded, 0, encoded.Length, new byte[encoded.Length * 2])); + var exception = Assert.Throws(() => Huffman.Decode(new ReadOnlySpan(encoded), new byte[encoded.Length * 2])); Assert.Equal(CoreStrings.HPackHuffmanErrorIncomplete, exception.Message); } @@ -103,7 +104,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [MemberData(nameof(_eosData))] public void ThrowsOnEOS(byte[] encoded) { - var exception = Assert.Throws(() => Huffman.Decode(encoded, 0, encoded.Length, new byte[encoded.Length * 2])); + var exception = Assert.Throws(() => Huffman.Decode(new ReadOnlySpan(encoded), new byte[encoded.Length * 2])); Assert.Equal(CoreStrings.HPackHuffmanErrorEOS, exception.Message); } @@ -112,7 +113,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { // h e l l o * var encoded = new byte[] { 0b100111_00, 0b101_10100, 0b0_101000_0, 0b0111_1111 }; - var exception = Assert.Throws(() => Huffman.Decode(encoded, 0, encoded.Length, new byte[encoded.Length])); + var exception = Assert.Throws(() => Huffman.Decode(new ReadOnlySpan(encoded), new byte[encoded.Length])); Assert.Equal(CoreStrings.HPackHuffmanErrorDestinationTooSmall, exception.Message); } @@ -144,10 +145,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [MemberData(nameof(_incompleteSymbolData))] public void ThrowsOnIncompleteSymbol(byte[] encoded) { - var exception = Assert.Throws(() => Huffman.Decode(encoded, 0, encoded.Length, new byte[encoded.Length * 2])); + var exception = Assert.Throws(() => Huffman.Decode(new ReadOnlySpan(encoded), new byte[encoded.Length * 2])); Assert.Equal(CoreStrings.HPackHuffmanErrorIncomplete, exception.Message); } + [Fact] + public void DecodeCharactersThatSpans5Octets() + { + var expectedLength = 2; + var decodedBytes = new byte[expectedLength]; + // B LF EOS + var encoded = new byte[] { 0b1011101_1, 0b11111111, 0b11111111, 0b11111111, 0b11100_111 }; + var decodedLength = Huffman.Decode(new ReadOnlySpan(encoded, 0, encoded.Length), decodedBytes); + + Assert.Equal(expectedLength, decodedLength); + Assert.Equal(new byte [] { (byte)'B', (byte)'\n' }, decodedBytes); + } + [Theory] [MemberData(nameof(HuffmanData))] public void HuffmanEncode(int code, uint expectedEncoded, int expectedBitLength) @@ -161,7 +175,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [MemberData(nameof(HuffmanData))] public void HuffmanDecode(int code, uint encoded, int bitLength) { - Assert.Equal(code, Huffman.Decode(encoded, bitLength, out var decodedBits)); + Assert.Equal(code, Huffman.DecodeValue(encoded, bitLength, out var decodedBits)); Assert.Equal(bitLength, decodedBits); } @@ -176,7 +190,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests #pragma warning restore xUnit1026 int bitLength) { - Assert.Equal(code, Huffman.Decode(Huffman.Encode(code).encoded, bitLength, out var decodedBits)); + Assert.Equal(code, Huffman.DecodeValue(Huffman.Encode(code).encoded, bitLength, out var decodedBits)); Assert.Equal(bitLength, decodedBits); } From 69ff195f66a4de134b515dabcc8aaf18c9773ce2 Mon Sep 17 00:00:00 2001 From: John Luo Date: Tue, 11 Sep 2018 21:42:24 -0700 Subject: [PATCH 3/3] Enable hpack/4.2 - Maximum Table Size test - Ensure dynamic tables size updates occur at the beginning of the header block --- src/Kestrel.Core/CoreStrings.resx | 57 ++++++++++--------- .../Internal/Http2/HPack/HPackDecoder.cs | 47 +++++++++++---- .../Properties/CoreStrings.Designer.cs | 14 +++++ test/Kestrel.Core.Tests/HPackDecoderTests.cs | 41 +++++++++++++ .../Http2/H2SpecTests.cs | 2 +- 5 files changed, 121 insertions(+), 40 deletions(-) diff --git a/src/Kestrel.Core/CoreStrings.resx b/src/Kestrel.Core/CoreStrings.resx index c40ba8d917..76953b051c 100644 --- a/src/Kestrel.Core/CoreStrings.resx +++ b/src/Kestrel.Core/CoreStrings.resx @@ -1,17 +1,17 @@  - @@ -578,4 +578,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l A value between {min} and {max} is required. + + Dynamic tables size update did not occur at the beginning of the first header block. + \ No newline at end of file diff --git a/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs b/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs index ac18c1962d..51d6068ae1 100644 --- a/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs +++ b/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs @@ -94,6 +94,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack private int _headerValueLength; private bool _index; private bool _huffman; + private bool _headersObserved; public HPackDecoder(int maxDynamicTableSize) : this(maxDynamicTableSize, new DynamicTable(maxDynamicTableSize)) @@ -115,19 +116,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack OnByte(data[i], handler); } - if (endHeaders && _state != State.Ready) + if (endHeaders) { - throw new HPackDecodingException(CoreStrings.HPackErrorIncompleteHeaderBlock); + if (_state != State.Ready) + { + throw new HPackDecodingException(CoreStrings.HPackErrorIncompleteHeaderBlock); + } + + _headersObserved = false; } } - public void OnByte(byte b, IHttpHeadersHandler handler) + private void OnByte(byte b, IHttpHeadersHandler handler) { switch (_state) { case State.Ready: if ((b & IndexedHeaderFieldMask) == IndexedHeaderFieldRepresentation) { + _headersObserved = true; var val = b & ~IndexedHeaderFieldMask; if (_integerDecoder.BeginDecode((byte)val, IndexedHeaderFieldPrefix)) @@ -141,6 +148,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack } else if ((b & LiteralHeaderFieldWithIncrementalIndexingMask) == LiteralHeaderFieldWithIncrementalIndexingRepresentation) { + _headersObserved = true; _index = true; var val = b & ~LiteralHeaderFieldWithIncrementalIndexingMask; @@ -159,6 +167,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack } else if ((b & LiteralHeaderFieldWithoutIndexingMask) == LiteralHeaderFieldWithoutIndexingRepresentation) { + _headersObserved = true; _index = false; var val = b & ~LiteralHeaderFieldWithoutIndexingMask; @@ -177,6 +186,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack } else if ((b & LiteralHeaderFieldNeverIndexedMask) == LiteralHeaderFieldNeverIndexedRepresentation) { + _headersObserved = true; _index = false; var val = b & ~LiteralHeaderFieldNeverIndexedMask; @@ -195,10 +205,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack } else if ((b & DynamicTableSizeUpdateMask) == DynamicTableSizeUpdateRepresentation) { + // https://tools.ietf.org/html/rfc7541#section-4.2 + // This dynamic table size + // update MUST occur at the beginning of the first header block + // following the change to the dynamic table size. + if (_headersObserved) + { + throw new HPackDecodingException(CoreStrings.HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock); + } + if (_integerDecoder.BeginDecode((byte)(b & ~DynamicTableSizeUpdateMask), DynamicTableSizeUpdatePrefix)) { - // TODO: validate that it's less than what's defined via SETTINGS - _dynamicTable.Resize(_integerDecoder.Value); + SetDynamicHeaderTableSize(_integerDecoder.Value); } else { @@ -295,13 +313,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack case State.DynamicTableSizeUpdate: if (_integerDecoder.Decode(b)) { - if (_integerDecoder.Value > _maxDynamicTableSize) - { - throw new HPackDecodingException( - CoreStrings.FormatHPackErrorDynamicTableSizeUpdateTooLarge(_integerDecoder.Value, _maxDynamicTableSize)); - } - - _dynamicTable.Resize(_integerDecoder.Value); + SetDynamicHeaderTableSize(_integerDecoder.Value); _state = State.Ready; } @@ -402,5 +414,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack throw new HPackDecodingException(CoreStrings.FormatHPackErrorIndexOutOfRange(index), ex); } } + + private void SetDynamicHeaderTableSize(int size) + { + if (size > _maxDynamicTableSize) + { + throw new HPackDecodingException( + CoreStrings.FormatHPackErrorDynamicTableSizeUpdateTooLarge(size, _maxDynamicTableSize)); + } + + _dynamicTable.Resize(size); + } } } diff --git a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs index 06945a5160..c9710cd26e 100644 --- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -2156,6 +2156,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatArgumentOutOfRange(object min, object max) => string.Format(CultureInfo.CurrentCulture, GetString("ArgumentOutOfRange", "min", "max"), min, max); + /// + /// Dynamic tables size update did not occur at the beginning of the first header block. + /// + internal static string HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock + { + get => GetString("HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock"); + } + + /// + /// Dynamic tables size update did not occur at the beginning of the first header block. + /// + internal static string FormatHPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock() + => GetString("HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/test/Kestrel.Core.Tests/HPackDecoderTests.cs b/test/Kestrel.Core.Tests/HPackDecoderTests.cs index 28878c6864..70d1f3e085 100644 --- a/test/Kestrel.Core.Tests/HPackDecoderTests.cs +++ b/test/Kestrel.Core.Tests/HPackDecoderTests.cs @@ -375,6 +375,47 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Empty(_decodedHeaders); } + [Fact] + public void DecodesDynamicTableSizeUpdate_AfterIndexedHeaderStatic_Error() + { + // 001 (Dynamic Table Size Update) + // 11110 (30 encoded with 5-bit prefix - see http://httpwg.org/specs/rfc7541.html#integer.representation) + + Assert.Equal(DynamicTableInitialMaxSize, _dynamicTable.MaxSize); + + var exception = Assert.Throws(() => _decoder.Decode(_indexedHeaderStatic.Concat(new byte[] { 0x3e }).ToArray(), endHeaders: true, handler: this)); + Assert.Equal(CoreStrings.HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock, exception.Message); + } + + [Fact] + public void DecodesDynamicTableSizeUpdate_AfterIndexedHeaderStatic_SubsequentDecodeCall_Error() + { + Assert.Equal(DynamicTableInitialMaxSize, _dynamicTable.MaxSize); + + _decoder.Decode(_indexedHeaderStatic, endHeaders: false, handler: this); + Assert.Equal("GET", _decodedHeaders[HeaderNames.Method]); + + // 001 (Dynamic Table Size Update) + // 11110 (30 encoded with 5-bit prefix - see http://httpwg.org/specs/rfc7541.html#integer.representation) + var exception = Assert.Throws(() => _decoder.Decode(new byte[] { 0x3e }, endHeaders: true, handler: this)); + Assert.Equal(CoreStrings.HPackErrorDynamicTableSizeUpdateNotAtBeginningOfHeaderBlock, exception.Message); + } + + [Fact] + public void DecodesDynamicTableSizeUpdate_AfterIndexedHeaderStatic_ResetAfterEndHeaders_Succeeds() + { + Assert.Equal(DynamicTableInitialMaxSize, _dynamicTable.MaxSize); + + _decoder.Decode(_indexedHeaderStatic, endHeaders: true, handler: this); + Assert.Equal("GET", _decodedHeaders[HeaderNames.Method]); + + // 001 (Dynamic Table Size Update) + // 11110 (30 encoded with 5-bit prefix - see http://httpwg.org/specs/rfc7541.html#integer.representation) + _decoder.Decode(new byte[] { 0x3e }, endHeaders: true, handler: this); + + Assert.Equal(30, _dynamicTable.MaxSize); + } + [Fact] public void DecodesDynamicTableSizeUpdate_GreaterThanLimit_Error() { diff --git a/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs b/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs index baca985366..138b7fbc48 100644 --- a/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs @@ -55,7 +55,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 get { var dataset = new TheoryData(); - var toSkip = new[] { "hpack/4.2/1", "http2/5.1/8" }; + var toSkip = new[] { "http2/5.1/8" }; foreach (var testcase in H2SpecCommands.EnumerateTestCases()) {