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

[optimistic-upgrade] Recommend GET for future Upgrade Tokens #2827

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

bemasc
Copy link
Contributor

@bemasc bemasc commented Jul 11, 2024

Fixes #2738

@bemasc bemasc added the optimistic-upgrade draft-ietf-httpbis-optimistic-upgrade label Jul 11, 2024
@royfielding
Copy link
Member

I do not agree with this suggestion.

Upgrade is intended to support a not-yet-defined future. Limits placed upon it must be evaluated in terms of what will be excluded from future use, not what is currently being used in practice.

If you want to limit what can be sent in a body, do that instead. The entire premise of this document is that a client will send something incredibly stupid and uncontrolled in a request body while asking for an upgrade. Just forbid that and be done.

In any case, Upgrade with GET, HEAD, and OPTIONS has no such issues. Upgrade should also be possible to use with QUERY with any compliant server. If you can find a server that fails to process a valid request body and then proceeds to treat it as a pipelined request, that is a vulnerability in their code.

@bemasc bemasc force-pushed the bemasc-deprecate-http branch 2 times, most recently from c3d4c21 to aed69da Compare July 12, 2024 13:51
@bemasc bemasc changed the base branch from bemasc-deprecate-http to main July 12, 2024 14:17
@bemasc bemasc marked this pull request as ready for review July 12, 2024 14:18
@bemasc
Copy link
Contributor Author

bemasc commented Jul 12, 2024

This PR originally assumed the deprecation of "TLS" in #2818. I've removed that deprecation there, so other HTTP methods are necessarily allowed.

Due to that change, I've rewritten the recommendation here, removed the PR stacking, and removed the "draft PR" flag.

@bemasc bemasc merged commit 4661890 into main Oct 21, 2024
2 checks passed
@bemasc bemasc deleted the bemasc-prefer-get branch October 21, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimistic-upgrade draft-ietf-httpbis-optimistic-upgrade
Development

Successfully merging this pull request may close these issues.

Consider restrictions on bodies in Upgrade requests
2 participants