Skip to content

H2 upgrade tests and mitigations#11563

Closed
icing wants to merge 10 commits intocurl:masterfrom
icing:h2-upgrade-conn-reuse
Closed

H2 upgrade tests and mitigations#11563
icing wants to merge 10 commits intocurl:masterfrom
icing:h2-upgrade-conn-reuse

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

icing added 2 commits August 1, 2023 11:33
- 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.

2 participants