From 025aca52df51f923a2d98406f947425cb1a533e2 Mon Sep 17 00:00:00 2001 From: John Luo Date: Thu, 13 Sep 2018 16:37:34 -0700 Subject: [PATCH] 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); }