Repository navigation
Send the final response to a HEAD request after an informational response - #748
Merged
Merged
Conversation
…onse A response to a HEAD request is sent without a body by turning the body disposition of its headers into :no_body. This happened for informational responses too, so an interim response to a HEAD request (the 100 Continue sent when the plug reads a body sent with `expect: 100-continue`, or anything sent with inform/3) was sent as if it were the complete response. Over HTTP/1 the socket was left in the :sent write state and the final response raised a FunctionClauseError, which went out as a 500. Over HTTP/2 the interim HEADERS frame carried END_STREAM, which makes it malformed (RFC9113§8.1), and the final response failed against the closed stream in the same way. An informational response never has a body whatever the request method, so leave its disposition alone and apply the HEAD rule only to the final response.
Owner
|
Ooof. good catch! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An informational response to a HEAD request broke the final response.
Bandit.Adapter.send_headers/4turns the body disposition into:no_bodywhenever the response has no body to send, which is always the case for HEAD, and it did so for:informtoo. The interim response was therefore sent as if it were the complete response::sentwrite state, the final response raised aFunctionClauseErrorinsend_headers/4, and the client received a 500;Both the automatic
100 Continuesent when a plug reads a body that came withexpect: 100-continueand anything sent withPlug.Conn.inform/3take this path. The same requests with GET work.This change leaves an
:informdisposition alone and applies the no-body rule only to the final response. An informational response never has a body whatever the request method (RFC9110§15.2).Four tests, next to the existing informational and
100 Continuetests intest/bandit/http1/protocol_test.exsandtest/bandit/http2/plug_test.exs: HEAD withinform/3and HEAD withexpect: 100-continueon each protocol, asserting the interim response followed by the plug's status with its headers and no body.Verified with
mix format --check-formatted,mix compile --warnings-as-errors,mix credo --strict,mix dialyzer, andmix teston Elixir 1.20.4 / OTP 29 (1078 passed, 1 skipped, 8 excluded; the 3 failures are inHibernationTestand also fail on main under OTP 29).