ninenines / cowlib

Support library for manipulating Web protocols.
ISC License
281 stars 172 forks source link

HPACK encoder must resize table when max_size changes, before encoding #101

Closed essen closed 4 years ago

essen commented 4 years ago

cow_hpack:encode/2 is missing a table_resize call when the size has changed. It must resize the table BEFORE encoding, otherwise the decoder won't be able to decode the packet.

blaxmirror commented 4 years ago

call table_update_size/2 in cow_hpack:encode/2,3 will be enough, right?

essen commented 4 years ago

Not sure, a test would help confirm one way or another.

blaxmirror commented 4 years ago

I wrote a simple case to test it (along with the fix of the other issue that I just submitted), seems okay to me.

table_update_encode_max_size_0_test() ->
    %% Encoding starts with default max size
    EncState0 = init(),
    %% Decoding starts with max size as 0
    DecState0 = init(0),

    %% First request
    Headers1 = [
        {<<":method">>, <<"GET">>},
        {<<":scheme">>, <<"http">>},
        {<<":path">>, <<"/">>},
        {<<":authority">>, <<"www.example.com">>}
    ],
    {Encoded1, EncState1} = encode(Headers1, EncState0),
    {Headers1, DecState1} = decode(iolist_to_binary(Encoded1), DecState0),
    #state{size=57, dyn_table=[{57,{<<":authority">>, <<"www.example.com">>}}]} = EncState1,
    #state{size=0, dyn_table=[]} = DecState1,

    %% Settings received after the first request
    EncState2 = set_max_size(0, EncState1),
    #state{configured_max_size=0, max_size=4096,
           size=57, dyn_table=[{57,{<<":authority">>, <<"www.example.com">>}}]} = EncState2,

    %% Second request
    Headers2 = [
        {<<":method">>, <<"GET">>},
        {<<":scheme">>, <<"http">>},
        {<<":path">>, <<"/">>},
        {<<":authority">>, <<"www.example.com">>},
        {<<"cache-control">>, <<"no-cache">>}
    ],
    {Encoded2, EncState3} = encode(Headers2, EncState2),
    {Headers2, DecState2} = decode(iolist_to_binary(Encoded2), DecState1),
    #state{configured_max_size=0, max_size=0, size=0, dyn_table=[]} = EncState3,
    #state{size=0, dyn_table=[]} = DecState2,
    ok.
essen commented 4 years ago

Closing, thanks!