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

Fix query params being clobbered by Clearance::BackDoor #1041

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

fcheung
Copy link
Contributor

@fcheung fcheung commented Nov 14, 2024

In rack 3.1.x Setting Rack::RACK_REQUEST_QUERY_STRING causes rack to think
that the query string has already been parsed
(see https://github.com/rack/rack/blob/v3.1.7/lib/rack/request.rb#L487)

This was introduced in #2703 but wasn't actually necessary - the warning mentioned
in that PR is only triggered if only Rack::RACK_REQUEST_QUERY_STRING is updated,
but the correct behaviour is to only set Rack::QUERY_STRING, not to set both

Fixes #1040

In rack 3.1.x Setting Rack::RACK_REQUEST_QUERY_STRING causes rack to think
that the query string has already been parsed
(see https://github.com/rack/rack/blob/v3.1.7/lib/rack/request.rb#L487)

This was introduced in #2703 but wasn't actually necessary - the warning mentioned
in that PR is only triggered if only Rack::RACK_REQUEST_QUERY_STRING is updated,
but the correct behaviour is to only set Rack::QUERY_STRING, not to set both

Fixes thoughtbot#1040
@sej3506
Copy link
Contributor

sej3506 commented Nov 14, 2024

Awesome! Thank you! It was fun watching your process solving this in person at RubyConf. 😁

@sej3506 sej3506 merged commit 6cc1919 into thoughtbot:main Nov 14, 2024
10 checks passed
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.

Query params removed after update from version 2.8.0 to 2.9.1
2 participants