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

Use Rack::QueryParser with nil missing value. #47652

Closed
wants to merge 3 commits into from

Conversation

ioquatix
Copy link
Contributor

@ioquatix ioquatix commented Mar 13, 2023

Rack 3 follows the WhatWG standard for the query parser. This deviates from
Rack 2 as ?a as a query string is was previously interpreted as
{"a" => nil} but is now interpreted as {"a" => ""}. Rails depends on
the previous behaviour for compatibility, so we introduced hooks into Rack
to restore compatibility.

This depends on rack/rack#2052 being merged and released as part of Rack 3.0.

cc @matthewd @tenderlove

@ioquatix
Copy link
Contributor Author

I've released rack v3.0.6 with rack/rack#2052 which should be sufficient to make this PR pass.

Rack 3 follows the WhatWG standard for the query parser. This deviates from
Rack 2 as `?a` as a query string is was previously interpreted as
`{"a" => nil}` but is now interpreted as `{"a" => ""}`. Rails depends on
the previous behaviour for compatibility, so we introduced hooks into Rack
to restore compatibility.
@ioquatix
Copy link
Contributor Author

@matthewd this seems to fix the tests + the rack keyed cache PR...

@ioquatix
Copy link
Contributor Author

This will only work once rack/rack#2054 is merged and back ported to 3-0-stable and released.

@dentarg dentarg mentioned this pull request Mar 16, 2023
7 tasks
@ioquatix
Copy link
Contributor Author

We have decided to revert this behaviour in Rack. Thus this PR is no longer necessary. The backport was released in Rack v3.0.7.

@ioquatix ioquatix closed this Mar 16, 2023
dentarg added a commit to dentarg/sinatra that referenced this pull request May 15, 2023
dentarg added a commit to dentarg/sinatra that referenced this pull request May 15, 2023
dentarg added a commit to dentarg/sinatra that referenced this pull request May 16, 2023
dentarg added a commit to dentarg/sinatra that referenced this pull request Aug 7, 2023
geoffharcourt pushed a commit to commonlit/sinatra that referenced this pull request Nov 6, 2023
dentarg added a commit to dentarg/sinatra that referenced this pull request Dec 23, 2023
dentarg added a commit to dentarg/sinatra that referenced this pull request Jan 2, 2024
dentarg added a commit to dentarg/sinatra that referenced this pull request Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant