Skip to content

fix(http1): avoid adding close to switching protocols responses - #4248

Open
akasakariko wants to merge 1 commit into
hyperium:masterfrom
akasakariko:fix/http1-upgrade-connection-close
Open

akasakariko wants to merge 1 commit into
hyperium:masterfrom
akasakariko:fix/http1-upgrade-connection-close

Conversation

@akasakariko

Copy link
Copy Markdown
Contributor

Fixes #4244

Summary

Do not append Connection: close to an HTTP/1.1 101 Switching Protocols response when HTTP keep-alive is disabled
The connection is handed off to the upgraded protocol rather than closed by HTTP

  • Add is_switching_protocols predicate to the internal Http1Transaction trait
  • Avoid inserting close into the HTTP/1.1 response headers when replying with status 101
  • Preserve normal request-side and response-side close handling for ordinary final responses
  • Extend the existing upgrades_new test with Connection: close, Upgrade to assert connection: upgrade without close while retaining buffered data and upgraded stream checks

Local validation

  • cargo test --features full --test server upgrades_new -- --exact fails on baseline and passes with this fix

  • cargo test --features full (328 passed, 0 failed, 10 ignored)

  • cargo +1.99.1 clippy --features full -- -D warnings passed without warnings

  • rustfmt --check --edition 2021 $(git ls-files '*.rs') and git diff --check passed

  • RUSTFLAGS='-D dead_code -D unused_imports' cargo hack --no-dev-deps check --feature-powerset --depth 2 --skip ffi,tracing (all 33 passed)

  • RUSTFLAGS='--cfg hyper_unstable_tracing -D dead_code -D unused_imports' cargo hack --no-dev-deps check --feature-powerset --depth 2 --features tracing --skip ffi (all 33 passed)

  • RUSTFLAGS='--cfg hyper_unstable_ffi' cargo test --features client,http1,http2,ffi --lib (104 passed, 6 ignored)

  • cargo semver-checks check-release --baseline-rev b07337878fe57b34b2065335ae8cae9ce5a780a0 --only-explicit-features --features full (223 passed, 31 skipped)

  • Rust 1.63 consumer check with MSRV-resolved vendored dependencies passed

  • Issue gist reproducer and raw-probe test passed with connection: upgrade and successful ping/pong

  • The description, docs, and comments (words meant for humans) are written by a human, not by an LLM.

Do not append Connection: close to an HTTP/1.1 101 response when HTTP keep-alive is disabled
The connection is handed off to the upgraded protocol rather than closed

Extend the existing upgrade test to cover a request with Connection: close, Upgrade
Verify the response headers while retaining the buffered-byte and upgraded-I/O checks

Fixes hyperium#4244

This branch has not been deployed

No deployments
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.

can't upgrade request that came in with Connection: close, Upgrade

1 participant