From e97f27078a8b9e4e6a90b8894f736f8df44acebc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= Date: Tue, 26 Sep 2023 14:47:19 +0200 Subject: [PATCH] Fix Huffman decoding on big-endian systems MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On PPC systems, HTTP2 & HPACK testcases were failing because AWS.HTTP2.HPACK.Huffman.Decode was raising Protocol_Errors over inconsistent padding. The encoded bytestrings were correct; things were diverging in AWS.HTTP2.HPACK.Decode, where bytes were misinterpreted when read as RFC_Byte, due to that record's representation clause being target-dependent. For example, if we break in that function right as it processes the first byte of an encoded string, and that first byte is decimal integer 130… 256 Byte := Get_Byte; (gdb) next 258 if Bit.B0 = 1 then (gdb) p/t byte $8 = 10000010 In RFC_Byte's "Bₙ at 0 range i .. j" component clauses, by default, i and j are interpreted differently depending on which bit ordering the target uses. Thus on a little-endian platform like x86_64-linux: (gdb) p/t bit $9 = ( b7 => 0, b6 => 1, b5 => 0, b4 => 0, b3 => 0, b2 => 0, b1 => 0, b0 => 1, b23 => 10, b22 => 0, b21 => 0, b20 => 10, b30 => 100, b41 => 10, b40 => 1000 ) Whereas on a big-endian platform such as ppc64-linux: (gdb) p/t bit $9 = ( b7 => 1, b6 => 0, b5 => 0, b4 => 0, b3 => 0, b2 => 0, b1 => 1, b0 => 0, b23 => 10, b22 => 0, b21 => 0, b20 => 10, b30 => 10, b41 => 1000, b40 => 10 ) Fix this by picking an explicit Bit_Order for RFC_Byte, so that in those range clauses, i and j are interpreted consistently across all platforms (i.e. pick Low_Order_First so that "0" refers to the MSb, "7" to the LSb). Also fix AWS.HTTP2.HPACK.Huffman.Decode, which presents a similar problem: a Stream_Element is aliased as an array of bits, but the indexing order of that array will depend on endianness. Use a similar solution to 2023-03-21 "Fix creation of Huffman decoding tree for big-endian platforms" (7eca5a10c) That is, remove the aliasing array type, and rely on platform-independent shifts. TN: eng/toolchain/aws#4 --- regtests/0341_hpack/test.opt | 2 -- regtests/0344_http2_hello/test.opt | 2 -- regtests/0346_http2_post_attachments/test.opt | 2 -- regtests/0347_http2_post_connection/test.opt | 2 -- regtests/0348_http2_client_status/test.opt | 2 -- src/http2/aws-http2-hpack-huffman.adb | 5 ++--- src/http2/aws-http2-hpack.adb | 6 ++++++ 7 files changed, 8 insertions(+), 13 deletions(-) delete mode 100644 regtests/0341_hpack/test.opt delete mode 100644 regtests/0344_http2_hello/test.opt delete mode 100644 regtests/0346_http2_post_attachments/test.opt delete mode 100644 regtests/0347_http2_post_connection/test.opt delete mode 100644 regtests/0348_http2_client_status/test.opt diff --git a/regtests/0341_hpack/test.opt b/regtests/0341_hpack/test.opt deleted file mode 100644 index 5cf5570fb..000000000 --- a/regtests/0341_hpack/test.opt +++ /dev/null @@ -1,2 +0,0 @@ -ppc-linux XFAIL Server crash; see eng/toolchain/aws#4. -ppc64-linux XFAIL Server crash; see eng/toolchain/aws#4. diff --git a/regtests/0344_http2_hello/test.opt b/regtests/0344_http2_hello/test.opt deleted file mode 100644 index 5cf5570fb..000000000 --- a/regtests/0344_http2_hello/test.opt +++ /dev/null @@ -1,2 +0,0 @@ -ppc-linux XFAIL Server crash; see eng/toolchain/aws#4. -ppc64-linux XFAIL Server crash; see eng/toolchain/aws#4. diff --git a/regtests/0346_http2_post_attachments/test.opt b/regtests/0346_http2_post_attachments/test.opt deleted file mode 100644 index 5cf5570fb..000000000 --- a/regtests/0346_http2_post_attachments/test.opt +++ /dev/null @@ -1,2 +0,0 @@ -ppc-linux XFAIL Server crash; see eng/toolchain/aws#4. -ppc64-linux XFAIL Server crash; see eng/toolchain/aws#4. diff --git a/regtests/0347_http2_post_connection/test.opt b/regtests/0347_http2_post_connection/test.opt deleted file mode 100644 index 5cf5570fb..000000000 --- a/regtests/0347_http2_post_connection/test.opt +++ /dev/null @@ -1,2 +0,0 @@ -ppc-linux XFAIL Server crash; see eng/toolchain/aws#4. -ppc64-linux XFAIL Server crash; see eng/toolchain/aws#4. diff --git a/regtests/0348_http2_client_status/test.opt b/regtests/0348_http2_client_status/test.opt deleted file mode 100644 index 5cf5570fb..000000000 --- a/regtests/0348_http2_client_status/test.opt +++ /dev/null @@ -1,2 +0,0 @@ -ppc-linux XFAIL Server crash; see eng/toolchain/aws#4. -ppc64-linux XFAIL Server crash; see eng/toolchain/aws#4. diff --git a/src/http2/aws-http2-hpack-huffman.adb b/src/http2/aws-http2-hpack-huffman.adb index 508bca2a8..e0565a867 100644 --- a/src/http2/aws-http2-hpack-huffman.adb +++ b/src/http2/aws-http2-hpack-huffman.adb @@ -380,7 +380,6 @@ package body AWS.HTTP2.HPACK.Huffman is EOS : constant Unsigned_32 := 16#fffffffa#; - type Byte_Bits is array (0 .. 7) of Bit with Pack; Iter : Node_Access := Root; Padding : Natural := 0; Pad_0 : Boolean := False; @@ -390,7 +389,6 @@ package body AWS.HTTP2.HPACK.Huffman is for K in Str'Range loop declare E : constant Stream_Element := Str (K); - Bits : Byte_Bits with Size => 8, Address => E'Address; C : Character; begin -- Keep last four bytes to check for EOS @@ -406,7 +404,8 @@ package body AWS.HTTP2.HPACK.Huffman is for B in reverse 0 .. 7 loop declare - Bit : constant Huffman.Bit := Bits (B); + Bit : constant Huffman.Bit := + Huffman.Bit (Shift_Right (Unsigned_8 (E), B) and 1); begin if Decode_Bit (Iter, Bit, C) then I := I + 1; diff --git a/src/http2/aws-http2-hpack.adb b/src/http2/aws-http2-hpack.adb index 023bc712d..053d90cdd 100644 --- a/src/http2/aws-http2-hpack.adb +++ b/src/http2/aws-http2-hpack.adb @@ -28,6 +28,7 @@ ------------------------------------------------------------------------------ with Interfaces; +with System; with AWS.HTTP2.Connection; with AWS.HTTP2.HPACK.Huffman; @@ -93,6 +94,11 @@ package body AWS.HTTP2.HPACK is B41 at 0 range 0 .. 3; end record; + -- Pick an explicit bit indexing order for this record, otherwise + -- the position & ranges in the component clauses will be + -- interpreted differently depending on endianness. + for RFC_Byte'Bit_Order use System.Low_Order_First; + B_II : constant Bit2 := 2#01#; -- Incremental Indexing