From dd35c3ef97ff36d9d25a5f17d787d2c4818a6c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=CE=BBstor?= Date: Fri, 27 Feb 2026 12:36:25 +0100 Subject: [PATCH] fix: Fixes off-by-one error in HPACK --- src/hackney_hpack.erl | 4 +- test/hackney_hpack_tests.erl | 105 +++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 test/hackney_hpack_tests.erl diff --git a/src/hackney_hpack.erl b/src/hackney_hpack.erl index e4138c60..abfab93b 100644 --- a/src/hackney_hpack.erl +++ b/src/hackney_hpack.erl @@ -566,7 +566,7 @@ find_in_dynamic_field(Header, #state{field_index = FieldIndex, insert_count = In {ok, AbsIndex} when InsertCount - AbsIndex < Count -> %% Convert absolute index to relative index %% Relative index = insert_count - abs_index + STATIC_TABLE_SIZE - {ok, ?STATIC_TABLE_SIZE + (InsertCount - AbsIndex)}; + {ok, ?STATIC_TABLE_SIZE + (InsertCount - AbsIndex) + 1}; _ -> error end. @@ -575,7 +575,7 @@ find_in_dynamic_field(Header, #state{field_index = FieldIndex, insert_count = In find_in_dynamic_name(Name, #state{name_index = NameIndex, insert_count = InsertCount, count = Count}) -> case maps:find(Name, NameIndex) of {ok, AbsIndex} when InsertCount - AbsIndex < Count -> - {ok, ?STATIC_TABLE_SIZE + (InsertCount - AbsIndex)}; + {ok, ?STATIC_TABLE_SIZE + (InsertCount - AbsIndex) + 1}; _ -> error end. diff --git a/test/hackney_hpack_tests.erl b/test/hackney_hpack_tests.erl new file mode 100644 index 00000000..0ec7b6aa --- /dev/null +++ b/test/hackney_hpack_tests.erl @@ -0,0 +1,105 @@ +-module(hackney_hpack_tests). +-include_lib("eunit/include/eunit.hrl"). + +-define(STATIC_TABLE_SIZE, 61). + +encode_decode_roundtrip_test_() -> + {"HPACK encode/decode round-trip", [ + {"single encode/decode cycle preserves headers", + fun encode_decode_single/0}, + {"second encode uses dynamic table and still decodes correctly", + fun encode_decode_reuses_dynamic_table/0}, + {"multiple sequential requests round-trip correctly", + fun encode_decode_multiple_sequential/0}, + {"dynamic table indices start at STATIC_TABLE_SIZE + 1", + fun dynamic_table_index_starts_at_62/0} + ]}. + +encode_decode_single() -> + Headers = [ + {<<":method">>, <<"GET">>}, + {<<":path">>, <<"/">>}, + {<<":scheme">>, <<"https">>}, + {<<":authority">>, <<"example.com">>} + ], + {Encoded, EncState} = hackney_hpack:encode(Headers), + EncodedBin = iolist_to_binary(Encoded), + {Decoded, _DecState} = hackney_hpack:decode(EncodedBin), + ?assertEqual(Headers, Decoded), + %% Verify encoder state is usable for next request + ?assertNotEqual(hackney_hpack:init(), EncState). + +encode_decode_reuses_dynamic_table() -> + Headers = [ + {<<":method">>, <<"GET">>}, + {<<":path">>, <<"/api/test">>}, + {<<":scheme">>, <<"https">>}, + {<<":authority">>, <<"example.com">>}, + {<<"accept">>, <<"application/json">>} + ], + %% First encode populates the dynamic table + {Encoded1, EncState1} = hackney_hpack:encode(Headers), + Encoded1Bin = iolist_to_binary(Encoded1), + {Decoded1, DecState1} = hackney_hpack:decode(Encoded1Bin), + ?assertEqual(Headers, Decoded1), + + %% Second encode should use indexed references from dynamic table + {Encoded2, _EncState2} = hackney_hpack:encode(Headers, EncState1), + Encoded2Bin = iolist_to_binary(Encoded2), + + %% Second encoding should be smaller (using indexed references) + ?assert(byte_size(Encoded2Bin) < byte_size(Encoded1Bin)), + + %% Must still decode to the same headers + {Decoded2, _DecState2} = hackney_hpack:decode(Encoded2Bin, DecState1), + ?assertEqual(Headers, Decoded2). + +encode_decode_multiple_sequential() -> + EncState0 = hackney_hpack:init(), + DecState0 = hackney_hpack:init(), + Requests = [ + [{<<":method">>, <<"GET">>}, + {<<":path">>, <<"/">>}, + {<<":scheme">>, <<"https">>}, + {<<":authority">>, <<"example.com">>}], + [{<<":method">>, <<"GET">>}, + {<<":path">>, <<"/page2">>}, + {<<":scheme">>, <<"https">>}, + {<<":authority">>, <<"example.com">>}], + [{<<":method">>, <<"POST">>}, + {<<":path">>, <<"/api/data">>}, + {<<":scheme">>, <<"https">>}, + {<<":authority">>, <<"example.com">>}, + {<<"content-type">>, <<"application/json">>}], + [{<<":method">>, <<"GET">>}, + {<<":path">>, <<"/">>}, + {<<":scheme">>, <<"https">>}, + {<<":authority">>, <<"example.com">>}] + ], + lists:foldl(fun(Headers, {ES, DS}) -> + {Encoded, ES2} = hackney_hpack:encode(Headers, ES), + EncodedBin = iolist_to_binary(Encoded), + {Decoded, DS2} = hackney_hpack:decode(EncodedBin, DS), + ?assertEqual(Headers, Decoded), + {ES2, DS2} + end, {EncState0, DecState0}, Requests). + +dynamic_table_index_starts_at_62() -> + Headers = [{<<"x-custom">>, <<"value">>}], + {Encoded1, EncState1} = hackney_hpack:encode(Headers), + Encoded1Bin = iolist_to_binary(Encoded1), + {_, DecState1} = hackney_hpack:decode(Encoded1Bin), + + %% Second encode should produce an indexed header field representation + %% The index must be >= 62 (STATIC_TABLE_SIZE + 1) + {Encoded2, _} = hackney_hpack:encode(Headers, EncState1), + Encoded2Bin = iolist_to_binary(Encoded2), + + %% Indexed header field starts with 1-bit prefix: 1xxxxxxx + %% Extract the index from the 7-bit prefix integer + <<1:1, IndexBits:7, _/binary>> = Encoded2Bin, + ?assert(IndexBits >= ?STATIC_TABLE_SIZE + 1), + + %% Verify it decodes correctly + {Decoded2, _} = hackney_hpack:decode(Encoded2Bin, DecState1), + ?assertEqual(Headers, Decoded2).