Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NO UPSTREAM] Malformed 400 reset #1

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

franfastly
Copy link
Owner

No description provided.

franfastly and others added 6 commits February 22, 2024 20:06
`h2` returns `RST_STREAM` frames with the `PROTOCOL_ERROR` bit set as a
response to many types of errors in client requests. Many of those cases,
when handled by an HTTP/1 server such as the one used in `hyper`, would
result in an HTTP 400 Bad Request response returned to the client rather
than a TCP reset (the HTTP/1 equivalent of a `RST_STREAM`). As a
consequence, a client will observe differences in behaviour between
HTTP/1 and HTTP/2: a malformed request will result in a 400 response when
HTTP/1 is used whereas the same request will result in a reset stream
with `h2`.

Make `h2` reply a `HEADERS`+`400`+`END_STREAM` frame followed by a
`RST_STREAM`+`PROTOCOL_ERROR` frame to all the `malformed!()` macro
invocations in `Peer::convert_poll_message()` in `src/server.rs`.

The `Reset` variant in the `h2::proto::error::Error` Enum now contains an
`Option<http::StatusCode>` value that is set by the `malformed!()` macro
with a `Some(400)`. That value is propagated all the way until
`h2::proto::streams::send::Send::send_reset()` where, if not `None`, a
`h2::frame::headers::Headers` frame with the status code and the
`END_STREAM` flag set will now be sent to the client before the
`RST_STREAM` frame.

Some of the parameters passed to `send_reset()` have been grouped into a
new `SendResetContext` Struct in order to avoid a
`clippy::too_many_arguments` lint.

Tests where malformed requests were sent have been updated to check that
an additional  `HEADERS`+`400`+`END_STREAM` frame is now received before
the `RST_STREAM + PROTOCOL_ERROR` frame.

This change has been validated with  other clients like `curl`, a custom
ad-hoc client written with `python3+httpx` and `varnishtest`.
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.

1 participant