benoitc / hackney

simple HTTP client in Erlang
Other
1.34k stars 427 forks source link

Long redirect urls with follow_redirect causes a crash #617

Open mattmatters opened 4 years ago

mattmatters commented 4 years ago

Hiya, looks like the header parsing does not finish parsing the Location header when the url is incredibly long. This is in version 1.15.2.

This might also occur when there are other headers, but the easiest way to reproduce was to make an async request with options [stream_to: pid, async: :once, follow_redirect: true] and have the server return a 302 redirect with a long url.

Error (in elixir)

{:unknown_error, {:case_clause, {:more, {:client, {1585, 770788, 824965}, {:metrics_ng, :metrics_dummy}, :hackney_tcp, 'localhost', 4000, "localhost:4000", [stream_to: #PID<0.1046.0>, async: :once, follow_redirect: true], #Port<0.35>, {:default, #Reference<0.1445723912.3210215425.82106>, {'localhost', 4000, :hackney_tcp}, #PID<0.866.0>, :hackney_tcp}, #Reference<0.1445723912.3210215425.82106>, true, :hackney_pool, 5000, true, 5, false, 5, nil, nil, {:hparser, :response, 4096, 10, 0, :on_header, "location: http://localhost:1234?something=asdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasd", {1, 1}, "", [], 8172, "", "", "text/html; charset=utf-8", "", :waiting}, {0, {:dict, 0, 16, 16, 8, 80, 48, {[], [], [], [], [], [], [], [], [], [], [], [], ...}, {{[], [], [], [], [], [], [], [], [], [], ...}}}}, :connected, :on_header, nil, :normal, false, :once, false, :undefined, #PID<0.1046.0>, &:hackney_request.send/2, :waiting, nil, 4096, "", [{"date", "Wed, 01 Apr2020 19:53:09 GMT"}, {"content-type", "text/html; charset=utf-8"}, {"content-length", "8172"}, {"cache-control", "max-age=0, private, must-revalidate"}], {1, 1}, 8172, nil, nil, "GET", "/_/explode", "text/html; charset=utf-8"}, ""}}}

Cheers!

benoitc commented 4 years ago

There is a limit of 4096 bytes set to the header line to prevent DoS that may explain the error. It’s not configurable yet. I will add this option. For my curiosity’ is there any reason to use such ling urls?

mattmatters commented 4 years ago

Thanks @benoitc! I'm downloading data from an external service. The service used to do a redirect and set authentication stuff as cookies while redirecting. Now it instead does a redirect to a url with the authentication information directly in the url.

So the reasoning is that an external service has set up their authentication system really weirdly. Unfortunately it's been a really blocker for us recently.

Thanks for the prompt reply and looking into it :)

bismark commented 4 years ago

Looking at this error a bit more, it doesn't appear to be directly related to hparser.max_line_length.

The case here https://github.com/benoitc/hackney/blob/eefe744282c80524bb670a11e6bf5f1493936d41/src/hackney_stream.erl#L176 is not handling the {more, Client, <<>>} case returned when hackney_http:parse_header requires more data from the socket here: https://github.com/benoitc/hackney/blob/eefe744282c80524bb670a11e6bf5f1493936d41/src/hackney_http.erl#L284