hackney
hackney copied to clipboard
Flatten request body to prevent hackney adding unnecessary headers
Fixes #5.
Hackney sees a body of <<>> as empty and doesn't set any extra headers for it. But it sees [<<>>] as a non-empty body that happens to have a length of zero, so it gets a content type of application/octet-stream and a content length of 0 (edit: this is intended for POST/PUT requests only). This PR flattens the body into a single binary so that hackney can detect empty bodies correctly.
Unfortunately this isn't practical to unit test, but I can confirm that it fixes the 500 errors I saw in #5.
Actually, this could possibly be considered a bug in hackney itself. I'll send a PR upstream and see how it turns out.
edit: https://github.com/benoitc/hackney/pull/750. It is a bug in hackney imo, albeit an edge case. But this PR is still worth merging to work better with current releases of hackney.
This negatively impacts performance and makes use of the iolist type all overhead rather than a performance optimisation, so it's not a suitable fix I'm afraid. It would need to be only done for bodiless methods.
When we pass in an iolist hackney does the iolist_to_binary conversion itself, so I believe the net performance is the same. I'll confirm that and cite the specific code involved when I'm at my home computer later.
With an iolist request body we get to this case statement in hackney_request.erl which handles iolists with:
_ when is_list(Body0) -> % iolist case
Body1 = iolist_to_binary(Body0),
S = erlang:byte_size(Body1),
CT = hackney_headers_new:get_value(
<<"content-type">>, Headers, <<"application/octet-stream">>
),
{S, CT, Body1};
By calling iolist_to_binary we instead take the binary case, which is identical but without the conversion. Either way iolist_to_binary is only called once. And in the empty body case we instead hit this empty body check earlier on and skip the iolist_to_binary call and the unnecessary header work we're doing now. So to my understanding performance should be as good or better in all cases with this PR.
Hackney performing unnecessary work is not an argument in favour of us also performing unnecessary work.
And in the empty body case we instead hit this empty body check earlier on and skip the iolist_to_binary call and the unnecessary header work we're doing now. So to my understanding performance should be as good or better in all cases with this PR.
The code you have linked also matches the empty iolist that we send.