grape: Rack3 breaking change with query param nil

While updating rack-test locally to the latest version, rack got updated also to it’s latest version 3.0.3

Some tests are failing due to the following breaking change :

BREAKING CHANGE: Query parsing now treats parameters without = as having the empty string value instead of nil value, to conform to the URL spec.

What does it means ? Passing a query params like ?name will be considered by Rack (>=3) as { "name" => "" } instead { "name" => nil }.

Moslty, it affects all the optional parameters that you can pass param_name: nil.

Question? Since it’s a breaking change, should we update the gemspec to keep rack < 3. That’s one breaking change, but there might be others since other specs are falling.

Tests failing:

rspec ./spec/grape/validations/validators/regexp_spec.rb:114 # Grape::Validations::Validators::RegexpValidator accepts nil
rspec ./spec/grape/validations/validators/regexp_spec.rb:59 # Grape::Validations::Validators::RegexpValidator custom validation message accepts nil
rspec ./spec/grape/validations/validators/regexp_spec.rb:82 # Grape::Validations::Validators::RegexpValidator custom validation message regexp with array refuses nil items
rspec ./spec/grape/validations_spec.rb:36 # Grape::Validations params optional doesn't validate when param not present
rspec './spec/grape/validations/validators/default_spec.rb[1:12:2:1]' # Grape::Validations::Validators::DefaultValidator optional with nil as value structures types respects the default value
rspec './spec/grape/validations/validators/default_spec.rb[1:12:2:2]' # Grape::Validations::Validators::DefaultValidator optional with nil as value structures types respects the default value
rspec './spec/grape/validations/validators/default_spec.rb[1:12:1:15]' # Grape::Validations::Validators::DefaultValidator optional with nil as value primitive types respects the default value
rspec ./spec/grape/validations/validators/coerce_spec.rb:212 # Grape::Validations::Validators::CoerceValidator coerce coerces String
rspec './spec/grape/validations/validators/coerce_spec.rb[1:1:5:11:2:1]' # Grape::Validations::Validators::CoerceValidator coerce coerces nil values structures types respects the nil value
rspec './spec/grape/validations/validators/coerce_spec.rb[1:1:5:11:1:9]' # Grape::Validations::Validators::CoerceValidator coerce coerces nil values primitive types respects the nil value
rspec ./spec/grape/validations/validators/coerce_spec.rb:724 # Grape::Validations::Validators::CoerceValidator coerce using coerce_with Array type and coerce_with should coerce nil value to array

Othere tests failing with Rack > 3

rspec ./spec/grape/middleware/formatter_spec.rb:403 # Grape::Middleware::Formatter send file returns a file response
rspec ./spec/grape/endpoint_spec.rb:139 # Grape::Endpoint#headers includes request headers
rspec ./spec/grape/endpoint_spec.rb:195 # Grape::Endpoint#cookies sets and update browser cookies
rspec ./spec/grape/endpoint_spec.rb:206 # Grape::Endpoint#cookies deletes cookie
rspec ./spec/grape/endpoint_spec.rb:162 # Grape::Endpoint#cookies is callable from within a block
rspec ./spec/grape/endpoint_spec.rb:230 # Grape::Endpoint#cookies deletes cookies with path
rspec ./spec/grape/endpoint_spec.rb:427 # Grape::Endpoint#params from body parameters returns a 400 if given an invalid multipart body

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 31 (31 by maintainers)

Most upvoted comments

Add a +1 to https://github.com/ruby-grape/grape/issues/2349. I’ll do a release this week if another maintainer doesn’t beat me to it (please do!).

dblock on Aug 30, 2023

@kbarrette So now we pass specs with Rack 3. Should we close this?

dblock on Aug 21, 2023

There is a vulnerability in rack < 2 that some security monitoring software is currently bugging me about: https://github.com/rack/rack/issues/1732 Any advice on how we should move forward with grape and rack?

kbarrette on Jun 5, 2023

The correct place to fix this (if anything can be done) is in the standard. I’ve made an issue and linked it. I suggest if you feel passionate about it, to go and comment on it.

ioquatix on Jan 4, 2023

@ericproulx Feel free to PR what you think is right, including making a release. I agree that locking Rack down < 3 because of known issues is the right thing to do now.

@ioquatix I like the idea of extracting parsers out of Rack. Grape would like Rack not to parse data, really, and bring its own parser next to the JSON one or any other parser. I can see some issues with other Rack middleware and the assumptions being made that Rack always parses application/x-www-form-urlencoded . Either way those issues belong in Rack, should we open some? Maybe @ericproulx wants to help? 😃

dblock on Jan 3, 2023

Yes, IIUC, that’s the case. And it’s done that way to match the RFCs. However, my question is, why specify baz at all if you just want it to be nil?

Because I want to pass nil as opposed to the absence of parameter. Consider an API that stores a middle name as a string. To set my middle name I pass name=Middle. To unset it, I pass name. If I don’t pass any value I don’t want it changed. In no case I wanted it to be a blank string.

If I am not mistaken, with this change an application/json POST now behaves differently from an application/x-www-form-urlencoded POST.

dblock on Jan 3, 2023

I see, so is there any difference between api?foo=bar&baz vs api?foo=bar?

Rack3

foo=bar&baz => {"foo"=>"bar", "baz"=>""}
foo=bar => {"foo"=>"bar"}

Rack2

foo=bar&baz => {"foo"=>"bar", "baz"=>nil}
foo=bar => {"foo"=>"bar"}

The tricky part is this change only affect query parameters. A body payload would still have nil values but not the query params.

ericproulx on Dec 30, 2022