Skip to content

Commit

Permalink
Fix HTTP3 header decoder buffer allocation (dotnet#78862)
Browse files Browse the repository at this point in the history 
* Add test for literal field without name reference

* Fix header name buffer allocation

* Add more tests

* Unified QPackDecoderTest test files

* Fix variable name

* Fixed HPackDecoder and ported QPack tests

* Feedback

---------

Co-authored-by: ManickaP <mapichov@microsoft.com>
  • Loading branch information
@BrunoBlanes @ManickaP
BrunoBlanes and ManickaP committed May 9, 2023
1 parent 86ce6a1 commit 94eb040
Show file tree
Hide file tree
Showing 5 changed files with 220 additions and 12 deletions.
11 changes: 6 additions & 5 deletions src/libraries/Common/src/System/Net/Http/aspnetcore/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, IHttpStreamHeadersHandler h
// 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 @@ -621,11 +621,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, dst.Length * 2)];
dst = new byte[Math.Max(stringLength, dst.Length * 2)];
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/QPack/QPackDecoder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,11 @@ private void DecodeInternal(ReadOnlySpan<byte> data, IHttpStreamHeadersHandler h
// will no longer be valid.
if (_headerNameRange != null)
{
EnsureStringCapacity(ref _headerNameOctets, _stringLength, existingLength: 0);
EnsureStringCapacity(ref _headerNameOctets, _headerNameLength, existingLength: 0);
_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 @@ -294,6 +293,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
106 changes: 106 additions & 0 deletions src/libraries/Common/tests/Tests/System/Net/aspnetcore/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.Equal(1, _handler.DecodedHeaders.Count);
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.Equal(1, _handler.DecodedHeaders.Count);
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.Equal(1, _handler.DecodedHeaders.Count);
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.Equal(1, _handler.DecodedHeaders.Count);
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.Equal(1, _handler.DecodedHeaders.Count);
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.Equal(1, _handler.DecodedHeaders.Count);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
}

[Fact]
public void DecodesDynamicTableSizeUpdate()
{
Expand Down
105 changes: 100 additions & 5 deletions src/libraries/Common/tests/Tests/System/Net/aspnetcore/Http3/QPackDecoderTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ public class QPackDecoderTests
// 4.5.4 - Literal Header Field With Name Reference - Static Table - Index 44 (content-type)
private static readonly byte[] _literalHeaderFieldWithNameReferenceStatic = new byte[] { 0x5f, 0x1d };

// 4.5.6 - Literal Field Line With Literal Name - (translate)
private static readonly byte[] _literalFieldLineWithLiteralName = new byte[] { 0x37, 0x02, 0x74, 0x72, 0x61, 0x6e, 0x73, 0x6c, 0x61, 0x74, 0x65 };
// 4.5.6 - Literal Field Line With Literal Name - (literal-header-field)
private static readonly byte[] _literalFieldLineWithLiteralName = new byte[] { 0x37, 0x0d, 0x6c, 0x69, 0x74, 0x65, 0x72, 0x61, 0x6c, 0x2d, 0x68, 0x65, 0x61, 0x64, 0x65, 0x72, 0x2d, 0x66, 0x69, 0x65, 0x6c, 0x64 };

private const string _contentTypeString = "content-type";
private const string _translateString = "translate";
private const string _literalHeaderFieldString = "literal-header-field";

// n e w - h e a d e r *
// 10101000 10111110 00010110 10011100 10100011 10010000 10110110 01111111
Expand Down Expand Up @@ -97,7 +97,7 @@ public void DecodesLiteralFieldLineWithLiteralName_Value()
.Concat(_headerValue)
.ToArray();

TestDecodeWithoutIndexing(encoded, _translateString, _headerValueString);
TestDecodeWithoutIndexing(encoded, _literalHeaderFieldString, _headerValueString);
}

[Fact]
Expand Down Expand Up @@ -140,7 +140,7 @@ public void DecodesLiteralFieldLineWithLiteralName_HuffmanEncodedValue()
.Concat(_headerValueHuffman)
.ToArray();

TestDecodeWithoutIndexing(encoded, _translateString, _headerValueString);
TestDecodeWithoutIndexing(encoded, _literalHeaderFieldString, _headerValueString);
}

[Fact]
Expand Down Expand Up @@ -173,6 +173,101 @@ public void DecodesLiteralFieldLineWithLiteralName_LargeValues()
});
}

[Fact]
public void LiteralFieldWithoutNameReference_SingleBuffer()
{
byte[] encoded = _literalFieldLineWithLiteralName
.Concat(_headerValue)
.ToArray();

_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
_decoder.Decode(encoded, endHeaders: true, handler: _handler);

Assert.Equal(1, _handler.DecodedHeaders.Count);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
}

[Fact]
public void LiteralFieldWithoutNameReference_NameLengthBrokenIntoSeparateBuffers()
{
byte[] encoded = _literalFieldLineWithLiteralName
.Concat(_headerValue)
.ToArray();

_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
_decoder.Decode(encoded[..1], endHeaders: false, handler: _handler);
_decoder.Decode(encoded[1..], endHeaders: true, handler: _handler);

Assert.Equal(1, _handler.DecodedHeaders.Count);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
}

[Fact]
public void LiteralFieldWithoutNameReference_NameBrokenIntoSeparateBuffers()
{
byte[] encoded = _literalFieldLineWithLiteralName
.Concat(_headerValue)
.ToArray();

_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
_decoder.Decode(encoded[..(_literalHeaderFieldString.Length / 2)], endHeaders: false, handler: _handler);
_decoder.Decode(encoded[(_literalHeaderFieldString.Length / 2)..], endHeaders: true, handler: _handler);

Assert.Equal(1, _handler.DecodedHeaders.Count);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
}

[Fact]
public void LiteralFieldWithoutNameReference_NameAndValueBrokenIntoSeparateBuffers()
{
byte[] encoded = _literalFieldLineWithLiteralName
.Concat(_headerValue)
.ToArray();

_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
_decoder.Decode(encoded[..^_headerValue.Length], endHeaders: false, handler: _handler);
_decoder.Decode(encoded[^_headerValue.Length..], endHeaders: true, handler: _handler);

Assert.Equal(1, _handler.DecodedHeaders.Count);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
}

[Fact]
public void LiteralFieldWithoutNameReference_ValueLengthBrokenIntoSeparateBuffers()
{
byte[] encoded = _literalFieldLineWithLiteralName
.Concat(_headerValue)
.ToArray();

_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
_decoder.Decode(encoded[..^(_headerValue.Length - 1)], endHeaders: false, handler: _handler);
_decoder.Decode(encoded[^(_headerValue.Length - 1)..], endHeaders: true, handler: _handler);

Assert.Equal(1, _handler.DecodedHeaders.Count);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
}

[Fact]
public void LiteralFieldWithoutNameReference_ValueBrokenIntoSeparateBuffers()
{
byte[] encoded = _literalFieldLineWithLiteralName
.Concat(_headerValue)
.ToArray();

_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
_decoder.Decode(encoded[..^(_headerValueString.Length / 2)], endHeaders: false, handler: _handler);
_decoder.Decode(encoded[^(_headerValueString.Length / 2)..], endHeaders: true, handler: _handler);

Assert.Equal(1, _handler.DecodedHeaders.Count);
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
}

public static readonly TheoryData<byte[]> _incompleteHeaderBlockData = new TheoryData<byte[]>
{
// Incomplete header
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@
Link="HPack\HPackIntegerTest.cs" />
<Compile Include="$(CommonPath)..\tests\Tests\System\Net\aspnetcore\Http2\HuffmanDecodingTests.cs"
Link="HPack\HuffmanDecodingTests.cs" />
<Compile Include="$(CommonPath)..\tests\Tests\System\Net\aspnetcore\Http3\QPackDecoderTest.cs"
Link="QPack\QPackDecoderTest.cs" />
<Compile Include="HttpContentTest.cs" />
<Compile Include="HttpRuleParserTest.cs" />
<Compile Include="MockContent.cs" />
Expand Down Expand Up @@ -393,8 +395,12 @@
Link="Common\System\Net\Http\aspnetcore\Http3\QPack\HeaderField.cs" />
<Compile Include="$(CommonPath)System\Net\Http\aspnetcore\Http3\QPack\QPackEncoder.cs"
Link="Common\System\Net\Http\aspnetcore\Http3\QPack\QPackEncoder.cs" />
<Compile Include="$(CommonPath)System\Net\Http\aspnetcore\Http3\QPack\QPackDecoder.cs"
Link="Common\System\Net\Http\aspnetcore\Http3\QPack\QPackDecoder.cs" />
<Compile Include="$(CommonPath)System\Net\Http\aspnetcore\Http3\QPack\QPackEncodingException.cs"
Link="Common\System\Net\Http\aspnetcore\Http3\QPack\QPackEncodingException.cs" />
<Compile Include="$(CommonPath)System\Net\Http\aspnetcore\Http3\QPack\QPackDecodingException.cs"
Link="Common\System\Net\Http\aspnetcore\Http3\QPack\QPackDecodingException.cs" />
<Compile Include="$(CommonPath)System\Text\ValueStringBuilder.cs"
Link="Common\System\Text\ValueStringBuilder.cs" />
<Compile Include="$(CommonPath)System\Text\ValueStringBuilder.AppendSpanFormattable.cs"
Expand Down

0 comments on commit 94eb040

Please sign in to comment.