Skip to content

Conversation

@noahlevenson
Copy link

This PR introduces a number of changes which fix HTTP client bugs, implement correct SSL/TLS semantics, and make our two underlying HTTP client libraries -- gun and httpc -- match one another's behavior.

Changes:

  • gun now automatically follows 3xx redirects, matching httpc behavior
  • new http_follow_redirects option toggles autoredirect behavior, works for both gun and httpc
  • new gun_max_redirects option mitigates redirect attacks on our bespoke gun redirect follower (httpc handles redirects out of the box and does not expose a twiddleable param)
  • deriving transport (TLS over TCP vs. plaintext TCP) from port and scheme is fixed
  • URLs without explicit ports now work the way you expect
  • TLS over arbitrary ports now works

Now, you should be able to do something like this:

hb_http_client:req(#{path => "/", method => <<"GET">>, peer => "http://nytimes.com", headers => #{}, body => <<>>}, #{http_client => gun}).

And it should just work. That request actually follows 2 redirects -- first, to the TLS encrypted https:// URL, and then to the www subdomain. So just for fun, if you twiddle the gun_max_redirects option, you can observe the client only follow the first redirect and return the 301:

hb_http_client:req(#{path => "/", method => <<"GET">>, peer => "http://nytimes.com", headers => #{}, body => <<>>}, #{http_client => gun, gun_max_redirects => 1}).

req(Args, ReestablishedConnection, Opts) ->
case hb_opts:get(http_client, gun, Opts) of
gun -> gun_req(Args, ReestablishedConnection, Opts);
gun ->
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reference for #481

Reply ->
Reply
end;
Reply = {_Ok, StatusCode, RedirectRes, _} ->
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reference for #481

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants