Use Rack's own headers classes where appropriate. #47091
Conversation
Rack 3 response headers must be a mutable hash with lower-case keys. Rack provides `Rack::Headers` as a compatibility layer for existing systems which don't conform to this requirement. Prefer `Rack::Utils::HeaderHash` on Rack 2, and `Rack::Headers` on Rack 3. Remove some of the response test cases which test `nil` header keys as these are considered invalid, and will fail with `Rack::Headers`.
ioquatix
force-pushed
the
rack-3-case-insensitive-headers
branch
from
4aecac8
to
0b4b4c6
Compare
|
@rafaelfranca this is also another relatively straight forward compatibility update with minimal changes to public interface. It actually corrects some cases because before using a hash, header keys were case sensitive, but using the headers class from Rack avoids this problem. |
|
Thanks so much for the quick turn around!!! |
| if @response.sending? || @response.sent? | ||
| raise ActionDispatch::IllegalStateError, "header already sent" | ||
| end |
There was a problem hiding this comment.
Are we still protecting against setting headers after the response has started sending?
There was a problem hiding this comment.
To the extent that it's reasonable and possible, yes, however Rack 3 requires a mutable hash for the headers. We might need to revisit this and clarify the exact behaviour.
0b4b4c6#diff-1e126b979451002d618e43e3517230f7b925cc0af5e693bb525db18259df4b85R455
There was a problem hiding this comment.
By the way, there were plenty of ways to bypass []= in the previous implementation... so I'm a little concerned the previous implementation was insufficient. e.g. Hash#update...
Rack 3 response headers must be a mutable hash with lower-case keys. Rack provides
Rack::Headersas a compatibility layer for existing systems which don't conform to this requirement. PreferRack::Utils::HeaderHashon Rack 2, andRack::Headerson Rack 3.Remove some of the response test cases which test
nilheader keys as these are considered invalid, and will fail withRack::Headers.