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

H2 upgrade tests and mitigations #11563

Closed
wants to merge 10 commits into from
Closed

Conversation

icing
Copy link
Contributor

@icing icing commented Aug 1, 2023

While issue of premature connection sharing with ongoing HTTP/2 Upgrades (https://curl.se/mail/lib-2023-07/0045.html) was fixed in d4618a3, this PR adds a test case for the original problem and some additional mitigation.

  • added in test_02_25 with special client to reproduce, based on sample code by Richard W.M. Jones
  • added check in http2.c for being called with an unknown transfer, giving CURLE_HTTP2 instead of a crash
  • added DEBUGBUILD library logging with transfer and connection ids for easier analysis

- check in h2 filter recv that stream actually exists
  and return error if not
- add test for parallel, extreme h2 upgrades that fail if
  connections get reused before fully switched
- add h2 upgrade upload test just for completeness
- added in test_02_25 with special client
- added xfer-conn-id logging for DEBUG builds in standard
  library debug function
@github-actions github-actions bot added the tests label Aug 1, 2023
@icing
Copy link
Contributor Author

icing commented Aug 2, 2023

Ok, bearssl and libressl fail the new test with timeouts. Investigating...

@icing
Copy link
Contributor Author

icing commented Aug 3, 2023

I believe this is ready for merge now.

@bagder bagder closed this in 944e219 Aug 3, 2023
ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
- check in h2 filter recv that stream actually exists
  and return error if not
- add test for parallel, extreme h2 upgrades that fail if
  connections get reused before fully switched
- add h2 upgrade upload test just for completeness

Closes curl#11563
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants