Skip to content

Commit a995dd3

Browse files
committed
Review changes, ManickaP dotnet#1
- renaming LUT - swapping leaf flag meaning (MSB) - typos
1 parent 28e9288 commit a995dd3

File tree

1 file changed

+27
-26
lines changed
  • src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack

1 file changed

+27
-26
lines changed

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

+27-26
Original file line numberDiff line numberDiff line change
@@ -292,33 +292,34 @@ private static ushort[] GenerateDecodingLookupTree()
292292
// -----------------------------------------------------------------
293293
// 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0
294294
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
295-
// | 0 | next_lookup_table_index | not_used |
295+
// | 1 | next_lookup_table_index | not_used |
296296
// +---+---------------------------+-------------------------------+
297297
// or
298298
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
299-
// | 1 | number_of_used_bits | octet |
299+
// | 0 | number_of_used_bits | octet |
300300
// +---+---------------------------+-------------------------------+
301301

302-
// Bit 15 set indicates a leaf value of decoding tree.
303-
// For example value 0x8241 means that we have reached end of huffman code
304-
// with result byte 0x41 'A' and from lookup bits count only rightmost 2 bites was used
302+
// Bit 15 unset indicates a leaf value of decoding tree.
303+
// For example value 0x0241 means that we have reached end of huffman code
304+
// with result byte 0x41 'A' and from lookup bits only rightmost 2 bits were used
305305
// and rest of bits are part of next huffman code.
306306

307-
// Bit 15 unset indicates that code is not yet decoded and next lookup table index shall be used
307+
// Bit 15 set indicates that code is not yet decoded and next lookup table index shall be used
308308
// for next n bits of huffman code.
309309
// 0 in 'next lookup table index' is considered as decoding error - invalid huffman code
310310

311311
// Because HPack uses static huffman code defined in RFC https://httpwg.org/specs/rfc7541.html#huffman.code
312-
// it is guaranteed that for this huffman code generated decoding lookup tree MUST consist of 15 lookup tables
313-
var lut = new ushort[15 * 256];
312+
// it is guaranteed that for this huffman code generated decoding lookup tree MUST consist of exactly 15 lookup tables
313+
var decodingTree = new ushort[15 * 256];
314314

315315
int allocatedLookupTableIndex = 0;
316316
// Create traverse path for all 0..256 octets, 256 is EOS, see: http://httpwg.org/specs/rfc7541.html#rfc.section.5.2
317317
for (int octet = 0; octet <= 256; octet++)
318318
{
319-
(uint code, int bitsLeft) = Encode(octet);
319+
(uint code, int bitLength) = Encode(octet);
320320

321321
int lookupTableIndex = 0;
322+
int bitsLeft = bitLength;
322323
while (bitsLeft > 0)
323324
{
324325
// read next 8 bits from huffman code
@@ -349,39 +350,39 @@ private static ushort[] GenerateDecodingLookupTree()
349350

350351
// Invalid huffman code - EOS
351352
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
352-
// | 0 | 0 0 0 0 0 0 0 | 1 1 1 1 1 1 1 1 |
353+
// | 1 | 0 0 0 0 0 0 0 | 1 1 1 1 1 1 1 1 |
353354
// +---+---------------------------+-------------------------------+
354-
lut[(lookupTableIndex << 8) + (indexInLookupTable | suffix)] = 0x00ff;
355+
decodingTree[(lookupTableIndex << 8) + (indexInLookupTable | suffix)] = 0x80ff;
355356
}
356357
else
357358
{
358359
// Leaf lookup value
359360
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
360-
// | 1 | number_of_used_bits | code |
361+
// | 0 | number_of_used_bits | code |
361362
// +---+---------------------------+-------------------------------+
362-
lut[(lookupTableIndex << 8) + (indexInLookupTable | suffix)] = (ushort)(((0x80 | bitsLeft) << 8) | octet);
363+
decodingTree[(lookupTableIndex << 8) + (indexInLookupTable | suffix)] = (ushort)((bitsLeft << 8) | octet);
363364
}
364365
}
365366
}
366367
else
367368
{
368369
// More than 8 bits left in huffman code means that we need to traverse to another lookup table for next 8 bits
369-
ushort lookupValue = lut[(lookupTableIndex << 8) + indexInLookupTable];
370+
ushort lookupValue = decodingTree[(lookupTableIndex << 8) + indexInLookupTable];
370371

371372
// Because next_lookup_table_index can not be 0, as 0 is index of root table, default value of array element
372373
// means that we have not initialized it yet => lookup table MUST be allocated and its index assigned to that lookup value
373374
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
374-
// | 0 | next_lookup_table_index | not_used |
375+
// | 1 | next_lookup_table_index | not_used |
375376
// +---+---------------------------+-------------------------------+
376377
if (lookupValue == default(ushort))
377378
{
378379
++allocatedLookupTableIndex;
379-
lut[(lookupTableIndex << 8) + indexInLookupTable] = (ushort)(allocatedLookupTableIndex << 8);
380+
decodingTree[(lookupTableIndex << 8) + indexInLookupTable] = (ushort)((0x80 | allocatedLookupTableIndex) << 8);
380381
lookupTableIndex = allocatedLookupTableIndex;
381382
}
382383
else
383384
{
384-
lookupTableIndex = lookupValue >> 8;
385+
lookupTableIndex = (lookupValue & 0x7f00) >> 8;
385386
}
386387
}
387388

@@ -390,7 +391,7 @@ private static ushort[] GenerateDecodingLookupTree()
390391
}
391392
}
392393

393-
return lut;
394+
return decodingTree;
394395
}
395396

396397
/// <summary>
@@ -444,11 +445,11 @@ public static int Decode(ReadOnlySpan<byte> src, ref byte[] dstArray)
444445

445446
ushort lookupValue = decodingTree[(lookupTableIndex << 8) + lookupIndex];
446447

447-
if (lookupValue >= 0x80_00)
448+
if (lookupValue < 0x80_00)
448449
{
449450
// Octet found.
450451
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
451-
// | 1 | number_of_used_bits | octet |
452+
// | 0 | number_of_used_bits | octet |
452453
// +---+---------------------------+-------------------------------+
453454
if (j >= dst.Length)
454455
{
@@ -459,15 +460,15 @@ public static int Decode(ReadOnlySpan<byte> src, ref byte[] dstArray)
459460

460461
// Start lookup of next symbol
461462
lookupTableIndex = 0;
462-
bitsInAcc -= (lookupValue & 0x7f00) >> 8;
463+
bitsInAcc -= lookupValue >> 8;
463464
}
464465
else
465466
{
466467
// Traverse to next lookup table.
467468
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
468-
// | 0 | next_lookup_table_index | not_used |
469+
// | 1 | next_lookup_table_index | not_used |
469470
// +---+---------------------------+-------------------------------+
470-
lookupTableIndex = lookupValue >> 8;
471+
lookupTableIndex = (lookupValue & 0x7f00) >> 8;
471472
if (lookupTableIndex == 0)
472473
{
473474
// No valid symbol could be decoded or EOS was decoded
@@ -504,13 +505,13 @@ public static int Decode(ReadOnlySpan<byte> src, ref byte[] dstArray)
504505

505506
ushort lookupValue = decodingTree[(lookupTableIndex << 8) + lookupIndex];
506507

507-
if (lookupValue >= 0x80_00)
508+
if (lookupValue < 0x80_00)
508509
{
509510
// Octet found.
510511
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
511-
// | 1 | number_of_used_bits | octet |
512+
// | 0 | number_of_used_bits | octet |
512513
// +---+---------------------------+-------------------------------+
513-
bitsInAcc -= (lookupValue & 0x7f00) >> 8;
514+
bitsInAcc -= lookupValue >> 8;
514515

515516
if (bitsInAcc < 0)
516517
{

0 commit comments

Comments
 (0)