Grape allows invalid headers to be set.
Follow up from https://github.com/socketry/protocol-rack/issues/2#issuecomment-1577206428.
The following implementation allows non-string header values.
https://github.com/ruby-grape/grape/blob/d1dfdccca242e00d4d88064a013f92e89a76dbd2/lib/grape/dsl/headers.rb#L13
Technically, all headers should be strings, according to the rack spec.
header "x-foo", 123
I'm not sure if we should change this. I see several options:
- Emit a warning if it's not a string.
- Convert values to a string.
- Do nothing.
- Later on, map
@headerkey values to string values, (or array of string values, allowed by Rack 3+).
For Grape users, is the real issue is that it works in Grape with Rack 2.x and not 3.x causing a NoMethodError: undefined method split' for 2:Integer`? If so, I think we should do nothing and document that upgrading to Rack 3 causes this. We could error early with "rack 3 doesn't support non-string values", too.
Other breaking changes with Rack 3, #2298.
No, it's nothing to do with Rack 3, it's always been invalid, even Rack 2 spec does not allow non-string header values.
But it works for users today, except when otherwise (described in https://github.com/socketry/protocol-rack/issues/2). I think Grape should adhere to spec. I like the option of doing .to_s on header values, because it's backwards-compatible, but we should look at whether any specs break with that.
If you put Rack::Lint in front of the apps (e.g. during testing/CI) it will fail.
I think you should encourage people to use Rack::Lint behind Grape in CI. It will report the problem.