added UseLibHeaders option to allow disabling of lib headers

[yuchunc]

#529
I am not familiar with erlang, and I can't, for the life of me, seem to figure out how to get erlang test suite to run. I fiddle with the code under my /deps, and this is working for me.

Please let me know if anything need to be updated.

Thank you!

[hazardfn]

@yuchunc I can't speak for @benoitc and this is his project, I personally see a potential problem regarding multi-part-streams partially because I don't know what the code you are bypassing does, here are a list of things I see:

• You have bypassed handle_body for multi part streams, which sounds like a massive side-effect for just disabling standard headers. It may not be important but I suspect it is without looking deeper.
• I would have the proplist value be a boolean and be called disable_default_headers (true/false)
• I would use proplist:get_value/3 and explicitly write a default of false
• I would use a function guard instead of having maybe_add_host like below
• A big one: lack of tests.

add_host(Headers, Netloc, DisableHeaders) when DisableHeaders ->
  Headers;
add_host(Headers0, Netloc, _DisableHeaders) ->
  {_, Headers1} = hackney_headers_new:store_new(<<"Host">>, Netloc, Headers0),
  Headers1.

[hazardfn]

Here is the method you are bypassing when headers are disabled. Not sure what it does or what horrific things may happen to multipart streams if it isn't run. This you should definitely check 😈

[hazardfn]

@benoitc for some context, here is the original post and reasoning behind why this may be a requirement (despite my personal belief the person who wrote this third-party API should have their computer(s) revoked): https://elixirforum.com/t/options-for-httpoison-and-hackney-to-not-append-library-headers/16519/4

[benoitc]

@hazardfn it would have been interesting which headers are triggering an error als.

@yuchunc Patch looks good but I think I would prefer to have it named skip_default_headers (false by default). Also I think that in such case the request should probably be validated before being sent. to make sure it comply to http 1.1, this would fix possible errors when sending a body or else as spotted by @hazardfn . Can you make such changes?

[hazardfn]

@benoitc that's up to @yuchunc, I just offered to give it a general review as he requested some input due to him being more comfortable with Elixir than Erlang 👍

[yuchunc]

@benoitc, @hazardfn Thank you both for reviewing this PR.
To be perfectly honest, I am not familiar with http 1.1 compliant, but I can send sometime looking in to it. I'll find sometime to make the changes.

I also do agree with @hazardfn, this api is pretty stupid . ☹️