Skip to content

Commit

Permalink
[release/6.0] Fix HTTP/2 header decoder buffer allocation (#47950)
Browse files Browse the repository at this point in the history 
* Fix HTTTP/2 header decoder buffer allocation

* Fix HPack test warnings.

---------

Co-authored-by: Aditya Mandaleeka <adityam@microsoft.com>
  • Loading branch information
@ManickaP @adityamandaleeka
ManickaP and adityamandaleeka committed May 11, 2023
1 parent 832ccdc commit b389117
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 11 deletions.
12 changes: 10 additions & 2 deletions .editorconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,12 @@ dotnet_diagnostic.CA1829.severity = warning
dotnet_diagnostic.CA1830.severity = warning

# CA1831: Use AsSpan or AsMemory instead of Range-based indexers when appropriate
# CA1832: Use AsSpan or AsMemory instead of Range-based indexers when appropriate
# CA1833: Use AsSpan or AsMemory instead of Range-based indexers when appropriate
dotnet_diagnostic.CA1831.severity = warning

# CA1832: Use AsSpan or AsMemory instead of Range-based indexers when appropriate
dotnet_diagnostic.CA1832.severity = warning

# CA1833: Use AsSpan or AsMemory instead of Range-based indexers when appropriate
dotnet_diagnostic.CA1833.severity = warning

# CA1834: Consider using 'StringBuilder.Append(char)' when applicable
Expand Down Expand Up @@ -225,6 +227,12 @@ dotnet_diagnostic.CA1826.severity = suggestion
dotnet_diagnostic.CA1827.severity = suggestion
# CA1829: Use Length/Count property instead of Count() when available
dotnet_diagnostic.CA1829.severity = suggestion
# CA1831: Use AsSpan or AsMemory instead of Range-based indexers when appropriate
dotnet_diagnostic.CA1831.severity = suggestion
# CA1832: Use AsSpan or AsMemory instead of Range-based indexers when appropriate
dotnet_diagnostic.CA1832.severity = suggestion
# CA1833: Use AsSpan or AsMemory instead of Range-based indexers when appropriate
dotnet_diagnostic.CA1833.severity = suggestion
# CA1834: Consider using 'StringBuilder.Append(char)' when applicable
dotnet_diagnostic.CA1834.severity = suggestion
# CA1835: Prefer the 'Memory'-based overloads for 'ReadAsync' and 'WriteAsync'
Expand Down
11 changes: 6 additions & 5 deletions src/Shared/runtime/Http2/Hpack/HPackDecoder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,11 @@ private void DecodeInternal(ReadOnlySpan<byte> data, IHttpHeadersHandler handler
// will no longer be valid.
if (_headerNameRange != null)
{
EnsureStringCapacity(ref _headerNameOctets);
EnsureStringCapacity(ref _headerNameOctets, _headerNameLength);
_headerName = _headerNameOctets;

ReadOnlySpan<byte> headerBytes = data.Slice(_headerNameRange.GetValueOrDefault().start, _headerNameRange.GetValueOrDefault().length);
headerBytes.CopyTo(_headerName);
_headerNameLength = headerBytes.Length;
_headerNameRange = null;
}
}
Expand Down Expand Up @@ -427,6 +426,7 @@ private void ParseHeaderName(ReadOnlySpan<byte> data, ref int currentIndex, IHtt
{
// Fast path. Store the range rather than copying.
_headerNameRange = (start: currentIndex, count);
_headerNameLength = _stringLength;
currentIndex += count;

_state = State.HeaderValueLength;
Expand Down Expand Up @@ -616,11 +616,12 @@ int Decode(ref byte[] dst)
_state = nextState;
}

private void EnsureStringCapacity(ref byte[] dst)
private void EnsureStringCapacity(ref byte[] dst, int stringLength = -1)
{
if (dst.Length < _stringLength)
stringLength = stringLength >= 0 ? stringLength : _stringLength;
if (dst.Length < stringLength)
{
dst = new byte[Math.Max(_stringLength, Math.Min(dst.Length * 2, _maxHeadersLength))];
dst = new byte[Math.Max(stringLength, Math.Min(dst.Length * 2, _maxHeadersLength))];
}
}

Expand Down
110 changes: 106 additions & 4 deletions src/Shared/test/Shared.Tests/runtime/Http2/HPackDecoderTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,13 @@ public class HPackDecoderTests

private const string _headerNameString = "new-header";

// On purpose longer than 4096 (DefaultStringOctetsSize from HPackDecoder) to trigger https://github.com/dotnet/runtime/issues/78516
private static readonly string _literalHeaderNameString = string.Concat(Enumerable.Range(0, 4100).Select(c => (char)('a' + (c % 26))));

private static readonly byte[] _headerNameBytes = Encoding.ASCII.GetBytes(_headerNameString);

private static readonly byte[] _literalHeaderNameBytes = Encoding.ASCII.GetBytes(_literalHeaderNameString);

// n e w - h e a d e r *
// 10101000 10111110 00010110 10011100 10100011 10010000 10110110 01111111
private static readonly byte[] _headerNameHuffmanBytes = new byte[] { 0xa8, 0xbe, 0x16, 0x9c, 0xa3, 0x90, 0xb6, 0x7f };
Expand All @@ -64,6 +69,12 @@ public class HPackDecoderTests
.Concat(_headerNameBytes)
.ToArray();

// size = 4096 ==> 0x7f, 0x81, 0x1f (7+) prefixed integer
// size = 4100 ==> 0x7f, 0x85, 0x1f (7+) prefixed integer
private static readonly byte[] _literalHeaderName = new byte[] { 0x7f, 0x85, 0x1f } // 4100
.Concat(_literalHeaderNameBytes)
.ToArray();

private static readonly byte[] _headerNameHuffman = new byte[] { (byte)(0x80 | _headerNameHuffmanBytes.Length) }
.Concat(_headerNameHuffmanBytes)
.ToArray();
Expand Down Expand Up @@ -392,6 +403,101 @@ public void DecodesLiteralHeaderFieldNeverIndexed_IndexedName_OutOfRange_Error()
Assert.Empty(_handler.DecodedHeaders);
}

[Fact]
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_SingleBuffer()
{
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
.Concat(_literalHeaderName)
.Concat(_headerValue)
.ToArray();

_decoder.Decode(encoded, endHeaders: true, handler: _handler);

Assert.Single(_handler.DecodedHeaders);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
}

[Fact]
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_NameLengthBrokenIntoSeparateBuffers()
{
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
.Concat(_literalHeaderName)
.Concat(_headerValue)
.ToArray();

_decoder.Decode(encoded[..1], endHeaders: false, handler: _handler);
_decoder.Decode(encoded[1..], endHeaders: true, handler: _handler);

Assert.Single(_handler.DecodedHeaders);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
}

[Fact]
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_NameBrokenIntoSeparateBuffers()
{
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
.Concat(_literalHeaderName)
.Concat(_headerValue)
.ToArray();

_decoder.Decode(encoded[..(_literalHeaderNameString.Length / 2)], endHeaders: false, handler: _handler);
_decoder.Decode(encoded[(_literalHeaderNameString.Length / 2)..], endHeaders: true, handler: _handler);

Assert.Single(_handler.DecodedHeaders);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
}

[Fact]
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_NameAndValueBrokenIntoSeparateBuffers()
{
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
.Concat(_literalHeaderName)
.Concat(_headerValue)
.ToArray();

_decoder.Decode(encoded[..^_headerValue.Length], endHeaders: false, handler: _handler);
_decoder.Decode(encoded[^_headerValue.Length..], endHeaders: true, handler: _handler);

Assert.Single(_handler.DecodedHeaders);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
}

[Fact]
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_ValueLengthBrokenIntoSeparateBuffers()
{
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
.Concat(_literalHeaderName)
.Concat(_headerValue)
.ToArray();

_decoder.Decode(encoded[..^(_headerValue.Length - 1)], endHeaders: false, handler: _handler);
_decoder.Decode(encoded[^(_headerValue.Length - 1)..], endHeaders: true, handler: _handler);

Assert.Single(_handler.DecodedHeaders);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
}

[Fact]
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_ValueBrokenIntoSeparateBuffers()
{
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
.Concat(_literalHeaderName)
.Concat(_headerValue)
.ToArray();

_decoder.Decode(encoded[..^(_headerValueString.Length / 2)], endHeaders: false, handler: _handler);
_decoder.Decode(encoded[^(_headerValueString.Length / 2)..], endHeaders: true, handler: _handler);

Assert.Single(_handler.DecodedHeaders);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
}

[Fact]
public void DecodesDynamicTableSizeUpdate()
{
Expand Down Expand Up @@ -500,10 +606,8 @@ public void DecodesStringLength_ExceedsLimit_Throws()
string string8191 = new string('a', MaxHeaderFieldSize - 1);
string string8193 = new string('a', MaxHeaderFieldSize + 1);
string string8194 = new string('a', MaxHeaderFieldSize + 2);

var bytes = new byte[3];
var success = IntegerEncoder.Encode(8194, 7, bytes, out var written);

byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
.Concat(new byte[] { 0x7f, 0x80, 0x3f }) // 8191 encoded with 7-bit prefix, no Huffman encoding
.Concat(Encoding.ASCII.GetBytes(string8191))
Expand All @@ -520,14 +624,12 @@ public void DecodesStringLength_ExceedsLimit_Throws()
.Concat(new byte[] { 0x7f, 0x83, 0x3f }) // 8194 encoded with 7-bit prefix, no Huffman encoding
.Concat(Encoding.ASCII.GetBytes(string8194))
.ToArray();

var ex = Assert.Throws<HPackDecodingException>(() => decoder.Decode(encoded, endHeaders: true, handler: _handler));
Assert.Equal(SR.Format(SR.net_http_headers_exceeded_length, MaxHeaderFieldSize + 1), ex.Message);
Assert.Equal(string8191, _handler.DecodedHeaders[string8191]);
Assert.Equal(string8193, _handler.DecodedHeaders[string8193]);
Assert.False(_handler.DecodedHeaders.ContainsKey(string8194));
}

[Fact]
public void DecodesStringLength_IndividualBytes()
{
Expand Down

0 comments on commit b389117

Please sign in to comment.