Skip to content

Commit

Permalink
Don't use a mutable constant as Rack response
Browse files Browse the repository at this point in the history
A common gotcha in Rack API is to return a constant as a response.
Another middleware higher in the stack may mutate the header hash
to add some user specific things (.e.g `Set-Cookie`) and this state
then leak across requests.

In the case of `rack-utf8-sanitizer`, the risk is limited because
it's likely to be among the very first middlewares, but it's still
best not to do this.
  • Loading branch information
byroot authored and whitequark committed Apr 12, 2024
1 parent fa9924c commit f2bcf86
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
3 changes: 1 addition & 2 deletions lib/rack/utf8_sanitizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
module Rack
class UTF8Sanitizer
StringIO = ::StringIO
BAD_REQUEST = [400, { "Content-Type" => "text/plain" }, ["Bad Request"]]
NULL_BYTE_REGEX = /\x00/.freeze

class NullByteInString < StandardError; end
Expand All @@ -28,7 +27,7 @@ def call(env)
begin
env = sanitize(env)
rescue EOFError
return BAD_REQUEST
return [400, { "Content-Type" => "text/plain" }, ["Bad Request"]]
end
@app.call(env)
end
Expand Down
10 changes: 10 additions & 0 deletions test/test_utf8_sanitizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,16 @@ def read
@response_env.should == [400, {"Content-Type"=>"text/plain"}, ["Bad Request"]]
end

it "Bad Request response can safety be mutated" do
@rack_input = BrokenIO.new
response_env = @app.(request_env)
response_env.should == [400, {"Content-Type"=>"text/plain"}, ["Bad Request"]]
response_env[1]["Set-Cookie"] = "you_are_admin"

response_env = @app.(request_env)
response_env[1]["Set-Cookie"].should == nil
end

it "sanitizes StringIO rack.input" do
input = "foo=bla&quux=bar"
@rack_input = StringIO.new input
Expand Down

0 comments on commit f2bcf86

Please sign in to comment.