-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Make query parameters without = have nil values #2059
Conversation
)" This reverts commit 5f90c33.
This reverts commit d25fedd.
This reverts commit 9f059d1.
This was Rack's historical behavior. While it doesn't match URL spec section 5.1.3.3, keeping the historical behavior avoids all of the complexity required to support the URL spec standard by default, but also support frameworks that want to be backwards compatible. This keeps as much of the specs added by the recently reverted commits that make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I think what we've identified here is that the way I think we should consider those assumptions carefully and attempt to build something more robust going forward. Now we know a lot more about the different assumptions and ways we can address them, I think we should consider this for Rack 4. |
💯💯💯 |
* Revert "Prefer to use `query_parser` itself as the cache key. (rack#2058)" This reverts commit 5f90c33. * Revert "Fix handling of cached values in `Rack::Request`. (rack#2054)" This reverts commit d25fedd. * Revert "Add `QueryParser#missing_value` for handling missing values + tests. (rack#2052)" This reverts commit 59d9ba9. * Revert "Split form/query parsing into two steps (rack#2038)" This reverts commit 9f059d1. * Make query parameters without = have nil values This was Rack's historical behavior. While it doesn't match URL spec section 5.1.3.3, keeping the historical behavior avoids all of the complexity required to support the URL spec standard by default, but also support frameworks that want to be backwards compatible. This keeps as much of the specs added by the recently reverted commits that make sense. # Conflicts: # lib/rack/multipart.rb # lib/rack/request.rb # test/spec_request.rb
* Revert "Prefer to use `query_parser` itself as the cache key. (#2058)" This reverts commit 5f90c33. * Revert "Fix handling of cached values in `Rack::Request`. (#2054)" This reverts commit d25fedd. * Revert "Add `QueryParser#missing_value` for handling missing values + tests. (#2052)" This reverts commit 59d9ba9. * Revert "Split form/query parsing into two steps (#2038)" This reverts commit 9f059d1. * Make query parameters without = have nil values This was Rack's historical behavior. While it doesn't match URL spec section 5.1.3.3, keeping the historical behavior avoids all of the complexity required to support the URL spec standard by default, but also support frameworks that want to be backwards compatible. This keeps as much of the specs added by the recently reverted commits that make sense. # Conflicts: # lib/rack/multipart.rb # lib/rack/request.rb # test/spec_request.rb
Rack v3.0.7 rolled back this change with rack/rack#2059 Other references: - rails/rails#47652 - rack/rack#2052 - rack/rack#2038
Rack v3.0.7 rolled back this change with rack/rack#2059 Other references: - rails/rails#47652 - rack/rack#2052 - rack/rack#2038
Rack v3.0.7 rolled back this change with rack/rack#2059 Other references: - rails/rails#47652 - rack/rack#2052 - rack/rack#2038
Rack v3.0.7 rolled back this change with rack/rack#2059 Other references: - rails/rails#47652 - rack/rack#2052 - rack/rack#2038
Rack v3.0.7 rolled back this change with rack/rack#2059 Other references: - rails/rails#47652 - rack/rack#2052 - rack/rack#2038
Rack v3.0.7 rolled back this change with rack/rack#2059 Other references: - rails/rails#47652 - rack/rack#2052 - rack/rack#2038
Rack v3.0.7 rolled back this change with rack/rack#2059 Other references: - rails/rails#47652 - rack/rack#2052 - rack/rack#2038
Rack v3.0.7 rolled back this change with rack/rack#2059 Other references: - rails/rails#47652 - rack/rack#2052 - rack/rack#2038
This was Rack's historical behavior. While it doesn't match
URL spec section 5.1.3.3, keeping the historical behavior avoids
all of the complexity required to support the URL spec standard
by default, but also support frameworks that want to be backwards
compatible.
This reverts the four recent commits that added a lot of additional
complexity just to get the backwards compatible behavior. While
I think the URL spec behavior is better, I think it's better to have
the backwards compatible behavior without the complexity than
the URL spec behavior with all of the complexity required to also
allow frameworks to get the backwards compatible behavior.
This keeps as much of the specs added by the recently reverted
commits that make sense.