Cover the HTTP/3 side of the header fix, and widen a test's margins - #948
Merged
Merged
Conversation
normalize_headers/1 is shared by the HTTP/2 and HTTP/3 request paths, so stripping the connection-specific headers of RFC 9113 8.2.2 fixed HTTP/3 too, where RFC 9114 4.2 bans the same fields. Only HTTP/2 was covered, so narrowing that filter would have broken HTTP/3 quietly. The HTTP/3 test server reports the headers it received on a new /headers route, and its route list gains the /reset entry it was missing. The checkout-idle tests probed within 100ms of a 300ms deadline, which is thin for the FreeBSD and macOS runners. The timer is 1s now and the probe sits half a second from either side of it. The assertions are unchanged.
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
1 participant
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.
Follow-up to #934 and #935, both merged as-is. Tests only, no behaviour change.
hackney_conn:normalize_headers/1is shared by the HTTP/2 request path and all four HTTP/3 send paths, so #935 fixed HTTP/3 as well, where RFC 9114 4.2 bans the same fields RFC 9113 8.2.2 does. Only HTTP/2 was covered, so a later change narrowing that filter to HTTP/2 would have broken HTTP/3 without failing anything.hackney_http3_connection_headers_testsmirrors the three HTTP/2 cases; all three fail against the pre-#935 filter with{error, {invalid_field, <<"connection">>, <<>>}}.The HTTP/3 test server had no way to show a test which headers reached it, so it answers
GET /headerswith them, one per line. Its route list also gains the/resetentry it was missing.The checkout-idle tests from #934 probed within 100ms of a 300ms deadline. A stall before the probe closes the conn and fails the test for a reason unrelated to the bug it guards, which is a poor trade on the FreeBSD and macOS runners. The timer is 1s now, with the probe half a second clear of the deadline on either side; the assertions are untouched. The module takes about 4.5s instead of under a second.
Both modules ran 8 times clean, and 8 more with
ERL_FLAGS="+S 1:1". Full suite: 1136 tests, xref and dialyzer clean.