Skip to content

Commit 0258951

Browse files
BrunoBlanesManickaP
BrunoBlanes
and
ManickaP
committed
Fix HTTP3 header decoder buffer allocation (dotnet#78862)
* 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>
1 parent 3de70ef commit 0258951

File tree

5 files changed

+220
-12
lines changed

5 files changed

+220
-12
lines changed

‎src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack/HPackDecoder.cs

+6-5
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,11 @@ private void DecodeInternal(ReadOnlySpan<byte> data, IHttpStreamHeadersHandler h
187187
// will no longer be valid.
188188
if (_headerNameRange != null)
189189
{
190-
EnsureStringCapacity(ref _headerNameOctets);
190+
EnsureStringCapacity(ref _headerNameOctets, _headerNameLength);
191191
_headerName = _headerNameOctets;
192192

193193
ReadOnlySpan<byte> headerBytes = data.Slice(_headerNameRange.GetValueOrDefault().start, _headerNameRange.GetValueOrDefault().length);
194194
headerBytes.CopyTo(_headerName);
195-
_headerNameLength = headerBytes.Length;
196195
_headerNameRange = null;
197196
}
198197
}
@@ -427,6 +426,7 @@ private void ParseHeaderName(ReadOnlySpan<byte> data, ref int currentIndex, IHtt
427426
{
428427
// Fast path. Store the range rather than copying.
429428
_headerNameRange = (start: currentIndex, count);
429+
_headerNameLength = _stringLength;
430430
currentIndex += count;
431431

432432
_state = State.HeaderValueLength;
@@ -621,11 +621,12 @@ int Decode(ref byte[] dst)
621621
_state = nextState;
622622
}
623623

624-
private void EnsureStringCapacity(ref byte[] dst)
624+
private void EnsureStringCapacity(ref byte[] dst, int stringLength = -1)
625625
{
626-
if (dst.Length < _stringLength)
626+
stringLength = stringLength >= 0 ? stringLength : _stringLength;
627+
if (dst.Length < stringLength)
627628
{
628-
dst = new byte[Math.Max(_stringLength, dst.Length * 2)];
629+
dst = new byte[Math.Max(stringLength, dst.Length * 2)];
629630
}
630631
}
631632

‎src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/QPack/QPackDecoder.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -243,12 +243,11 @@ private void DecodeInternal(ReadOnlySpan<byte> data, IHttpStreamHeadersHandler h
243243
// will no longer be valid.
244244
if (_headerNameRange != null)
245245
{
246-
EnsureStringCapacity(ref _headerNameOctets, _stringLength, existingLength: 0);
246+
EnsureStringCapacity(ref _headerNameOctets, _headerNameLength, existingLength: 0);
247247
_headerName = _headerNameOctets;
248248

249249
ReadOnlySpan<byte> headerBytes = data.Slice(_headerNameRange.GetValueOrDefault().start, _headerNameRange.GetValueOrDefault().length);
250250
headerBytes.CopyTo(_headerName);
251-
_headerNameLength = headerBytes.Length;
252251
_headerNameRange = null;
253252
}
254253
}
@@ -294,6 +293,7 @@ private void ParseHeaderName(ReadOnlySpan<byte> data, ref int currentIndex, IHtt
294293
{
295294
// Fast path. Store the range rather than copying.
296295
_headerNameRange = (start: currentIndex, count);
296+
_headerNameLength = _stringLength;
297297
currentIndex += count;
298298

299299
_state = State.HeaderValueLength;

‎src/libraries/Common/tests/Tests/System/Net/aspnetcore/Http2/HPackDecoderTest.cs

+106
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,13 @@ public class HPackDecoderTests
4646

4747
private const string _headerNameString = "new-header";
4848

49+
// On purpose longer than 4096 (DefaultStringOctetsSize from HPackDecoder) to trigger https://github.com/dotnet/runtime/issues/78516
50+
private static readonly string _literalHeaderNameString = string.Concat(Enumerable.Range(0, 4100).Select(c => (char)('a' + (c % 26))));
51+
4952
private static readonly byte[] _headerNameBytes = Encoding.ASCII.GetBytes(_headerNameString);
5053

54+
private static readonly byte[] _literalHeaderNameBytes = Encoding.ASCII.GetBytes(_literalHeaderNameString);
55+
5156
// n e w - h e a d e r *
5257
// 10101000 10111110 00010110 10011100 10100011 10010000 10110110 01111111
5358
private static readonly byte[] _headerNameHuffmanBytes = new byte[] { 0xa8, 0xbe, 0x16, 0x9c, 0xa3, 0x90, 0xb6, 0x7f };
@@ -64,6 +69,12 @@ public class HPackDecoderTests
6469
.Concat(_headerNameBytes)
6570
.ToArray();
6671

72+
// size = 4096 ==> 0x7f, 0x81, 0x1f (7+) prefixed integer
73+
// size = 4100 ==> 0x7f, 0x85, 0x1f (7+) prefixed integer
74+
private static readonly byte[] _literalHeaderName = new byte[] { 0x7f, 0x85, 0x1f } // 4100
75+
.Concat(_literalHeaderNameBytes)
76+
.ToArray();
77+
6778
private static readonly byte[] _headerNameHuffman = new byte[] { (byte)(0x80 | _headerNameHuffmanBytes.Length) }
6879
.Concat(_headerNameHuffmanBytes)
6980
.ToArray();
@@ -392,6 +403,101 @@ public void DecodesLiteralHeaderFieldNeverIndexed_IndexedName_OutOfRange_Error()
392403
Assert.Empty(_handler.DecodedHeaders);
393404
}
394405

406+
[Fact]
407+
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_SingleBuffer()
408+
{
409+
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
410+
.Concat(_literalHeaderName)
411+
.Concat(_headerValue)
412+
.ToArray();
413+
414+
_decoder.Decode(encoded, endHeaders: true, handler: _handler);
415+
416+
Assert.Equal(1, _handler.DecodedHeaders.Count);
417+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
418+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
419+
}
420+
421+
[Fact]
422+
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_NameLengthBrokenIntoSeparateBuffers()
423+
{
424+
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
425+
.Concat(_literalHeaderName)
426+
.Concat(_headerValue)
427+
.ToArray();
428+
429+
_decoder.Decode(encoded[..1], endHeaders: false, handler: _handler);
430+
_decoder.Decode(encoded[1..], endHeaders: true, handler: _handler);
431+
432+
Assert.Equal(1, _handler.DecodedHeaders.Count);
433+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
434+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
435+
}
436+
437+
[Fact]
438+
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_NameBrokenIntoSeparateBuffers()
439+
{
440+
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
441+
.Concat(_literalHeaderName)
442+
.Concat(_headerValue)
443+
.ToArray();
444+
445+
_decoder.Decode(encoded[..(_literalHeaderNameString.Length / 2)], endHeaders: false, handler: _handler);
446+
_decoder.Decode(encoded[(_literalHeaderNameString.Length / 2)..], endHeaders: true, handler: _handler);
447+
448+
Assert.Equal(1, _handler.DecodedHeaders.Count);
449+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
450+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
451+
}
452+
453+
[Fact]
454+
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_NameAndValueBrokenIntoSeparateBuffers()
455+
{
456+
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
457+
.Concat(_literalHeaderName)
458+
.Concat(_headerValue)
459+
.ToArray();
460+
461+
_decoder.Decode(encoded[..^_headerValue.Length], endHeaders: false, handler: _handler);
462+
_decoder.Decode(encoded[^_headerValue.Length..], endHeaders: true, handler: _handler);
463+
464+
Assert.Equal(1, _handler.DecodedHeaders.Count);
465+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
466+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
467+
}
468+
469+
[Fact]
470+
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_ValueLengthBrokenIntoSeparateBuffers()
471+
{
472+
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
473+
.Concat(_literalHeaderName)
474+
.Concat(_headerValue)
475+
.ToArray();
476+
477+
_decoder.Decode(encoded[..^(_headerValue.Length - 1)], endHeaders: false, handler: _handler);
478+
_decoder.Decode(encoded[^(_headerValue.Length - 1)..], endHeaders: true, handler: _handler);
479+
480+
Assert.Equal(1, _handler.DecodedHeaders.Count);
481+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
482+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
483+
}
484+
485+
[Fact]
486+
public void DecodesLiteralHeaderFieldNeverIndexed_NewName_ValueBrokenIntoSeparateBuffers()
487+
{
488+
byte[] encoded = _literalHeaderFieldWithoutIndexingNewName
489+
.Concat(_literalHeaderName)
490+
.Concat(_headerValue)
491+
.ToArray();
492+
493+
_decoder.Decode(encoded[..^(_headerValueString.Length / 2)], endHeaders: false, handler: _handler);
494+
_decoder.Decode(encoded[^(_headerValueString.Length / 2)..], endHeaders: true, handler: _handler);
495+
496+
Assert.Equal(1, _handler.DecodedHeaders.Count);
497+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderNameString));
498+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderNameString]);
499+
}
500+
395501
[Fact]
396502
public void DecodesDynamicTableSizeUpdate()
397503
{

‎src/libraries/Common/tests/Tests/System/Net/aspnetcore/Http3/QPackDecoderTest.cs

+100-5
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ public class QPackDecoderTests
2525
// 4.5.4 - Literal Header Field With Name Reference - Static Table - Index 44 (content-type)
2626
private static readonly byte[] _literalHeaderFieldWithNameReferenceStatic = new byte[] { 0x5f, 0x1d };
2727

28-
// 4.5.6 - Literal Field Line With Literal Name - (translate)
29-
private static readonly byte[] _literalFieldLineWithLiteralName = new byte[] { 0x37, 0x02, 0x74, 0x72, 0x61, 0x6e, 0x73, 0x6c, 0x61, 0x74, 0x65 };
28+
// 4.5.6 - Literal Field Line With Literal Name - (literal-header-field)
29+
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 };
3030

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

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

100-
TestDecodeWithoutIndexing(encoded, _translateString, _headerValueString);
100+
TestDecodeWithoutIndexing(encoded, _literalHeaderFieldString, _headerValueString);
101101
}
102102

103103
[Fact]
@@ -140,7 +140,7 @@ public void DecodesLiteralFieldLineWithLiteralName_HuffmanEncodedValue()
140140
.Concat(_headerValueHuffman)
141141
.ToArray();
142142

143-
TestDecodeWithoutIndexing(encoded, _translateString, _headerValueString);
143+
TestDecodeWithoutIndexing(encoded, _literalHeaderFieldString, _headerValueString);
144144
}
145145

146146
[Fact]
@@ -173,6 +173,101 @@ public void DecodesLiteralFieldLineWithLiteralName_LargeValues()
173173
});
174174
}
175175

176+
[Fact]
177+
public void LiteralFieldWithoutNameReference_SingleBuffer()
178+
{
179+
byte[] encoded = _literalFieldLineWithLiteralName
180+
.Concat(_headerValue)
181+
.ToArray();
182+
183+
_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
184+
_decoder.Decode(encoded, endHeaders: true, handler: _handler);
185+
186+
Assert.Equal(1, _handler.DecodedHeaders.Count);
187+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
188+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
189+
}
190+
191+
[Fact]
192+
public void LiteralFieldWithoutNameReference_NameLengthBrokenIntoSeparateBuffers()
193+
{
194+
byte[] encoded = _literalFieldLineWithLiteralName
195+
.Concat(_headerValue)
196+
.ToArray();
197+
198+
_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
199+
_decoder.Decode(encoded[..1], endHeaders: false, handler: _handler);
200+
_decoder.Decode(encoded[1..], endHeaders: true, handler: _handler);
201+
202+
Assert.Equal(1, _handler.DecodedHeaders.Count);
203+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
204+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
205+
}
206+
207+
[Fact]
208+
public void LiteralFieldWithoutNameReference_NameBrokenIntoSeparateBuffers()
209+
{
210+
byte[] encoded = _literalFieldLineWithLiteralName
211+
.Concat(_headerValue)
212+
.ToArray();
213+
214+
_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
215+
_decoder.Decode(encoded[..(_literalHeaderFieldString.Length / 2)], endHeaders: false, handler: _handler);
216+
_decoder.Decode(encoded[(_literalHeaderFieldString.Length / 2)..], endHeaders: true, handler: _handler);
217+
218+
Assert.Equal(1, _handler.DecodedHeaders.Count);
219+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
220+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
221+
}
222+
223+
[Fact]
224+
public void LiteralFieldWithoutNameReference_NameAndValueBrokenIntoSeparateBuffers()
225+
{
226+
byte[] encoded = _literalFieldLineWithLiteralName
227+
.Concat(_headerValue)
228+
.ToArray();
229+
230+
_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
231+
_decoder.Decode(encoded[..^_headerValue.Length], endHeaders: false, handler: _handler);
232+
_decoder.Decode(encoded[^_headerValue.Length..], endHeaders: true, handler: _handler);
233+
234+
Assert.Equal(1, _handler.DecodedHeaders.Count);
235+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
236+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
237+
}
238+
239+
[Fact]
240+
public void LiteralFieldWithoutNameReference_ValueLengthBrokenIntoSeparateBuffers()
241+
{
242+
byte[] encoded = _literalFieldLineWithLiteralName
243+
.Concat(_headerValue)
244+
.ToArray();
245+
246+
_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
247+
_decoder.Decode(encoded[..^(_headerValue.Length - 1)], endHeaders: false, handler: _handler);
248+
_decoder.Decode(encoded[^(_headerValue.Length - 1)..], endHeaders: true, handler: _handler);
249+
250+
Assert.Equal(1, _handler.DecodedHeaders.Count);
251+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
252+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
253+
}
254+
255+
[Fact]
256+
public void LiteralFieldWithoutNameReference_ValueBrokenIntoSeparateBuffers()
257+
{
258+
byte[] encoded = _literalFieldLineWithLiteralName
259+
.Concat(_headerValue)
260+
.ToArray();
261+
262+
_decoder.Decode(new byte[] { 0x00, 0x00 }, endHeaders: false, handler: _handler);
263+
_decoder.Decode(encoded[..^(_headerValueString.Length / 2)], endHeaders: false, handler: _handler);
264+
_decoder.Decode(encoded[^(_headerValueString.Length / 2)..], endHeaders: true, handler: _handler);
265+
266+
Assert.Equal(1, _handler.DecodedHeaders.Count);
267+
Assert.True(_handler.DecodedHeaders.ContainsKey(_literalHeaderFieldString));
268+
Assert.Equal(_headerValueString, _handler.DecodedHeaders[_literalHeaderFieldString]);
269+
}
270+
176271
public static readonly TheoryData<byte[]> _incompleteHeaderBlockData = new TheoryData<byte[]>
177272
{
178273
// Incomplete header

‎src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj

+6
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,8 @@
322322
Link="HPack\HPackIntegerTest.cs" />
323323
<Compile Include="$(CommonPath)..\tests\Tests\System\Net\aspnetcore\Http2\HuffmanDecodingTests.cs"
324324
Link="HPack\HuffmanDecodingTests.cs" />
325+
<Compile Include="$(CommonPath)..\tests\Tests\System\Net\aspnetcore\Http3\QPackDecoderTest.cs"
326+
Link="QPack\QPackDecoderTest.cs" />
325327
<Compile Include="HttpContentTest.cs" />
326328
<Compile Include="HttpRuleParserTest.cs" />
327329
<Compile Include="MockContent.cs" />
@@ -393,8 +395,12 @@
393395
Link="Common\System\Net\Http\aspnetcore\Http3\QPack\HeaderField.cs" />
394396
<Compile Include="$(CommonPath)System\Net\Http\aspnetcore\Http3\QPack\QPackEncoder.cs"
395397
Link="Common\System\Net\Http\aspnetcore\Http3\QPack\QPackEncoder.cs" />
398+
<Compile Include="$(CommonPath)System\Net\Http\aspnetcore\Http3\QPack\QPackDecoder.cs"
399+
Link="Common\System\Net\Http\aspnetcore\Http3\QPack\QPackDecoder.cs" />
396400
<Compile Include="$(CommonPath)System\Net\Http\aspnetcore\Http3\QPack\QPackEncodingException.cs"
397401
Link="Common\System\Net\Http\aspnetcore\Http3\QPack\QPackEncodingException.cs" />
402+
<Compile Include="$(CommonPath)System\Net\Http\aspnetcore\Http3\QPack\QPackDecodingException.cs"
403+
Link="Common\System\Net\Http\aspnetcore\Http3\QPack\QPackDecodingException.cs" />
398404
<Compile Include="$(CommonPath)System\Text\ValueStringBuilder.cs"
399405
Link="Common\System\Text\ValueStringBuilder.cs" />
400406
<Compile Include="$(CommonPath)System\Text\ValueStringBuilder.AppendSpanFormattable.cs"

0 commit comments

Comments
 (0)