Skip to content

Commit

Permalink
Rack spec requires mutable headers
Browse files Browse the repository at this point in the history
The rack spec requires the header object to be an unfrozen hash.
https://github.com/rack/rack/blob/c8e98221830a0b972b1dc19f3b6784f65197f444/SPEC.rdoc?plain=1#L240

Rack::ETag was buffering and making a copy of the response,
so the freeze was not effective anyway.

Plus we are freezing the hash too early, preventing middlewares
from modifying it. It causes crash with gems like rack-livereload.
I started having crashes on some pages (like the internal
http://localhost:3000/rails/info/routes) because of rack-livereload
hitting the frozen hash after the rack 3 upgrade.

Also we're not consistent with the protection. We're not preventing
users from adding cookies. The cookie jar is already flushed,
therefore it doesn't try to change the headers and never triggers the
frozen hash error.
  • Loading branch information
JoeDupuis committed Jun 11, 2023
1 parent bb8f1cf commit 45c90b7
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 31 deletions.
1 change: 0 additions & 1 deletion actionpack/lib/action_dispatch/http/response.rb
Expand Up @@ -462,7 +462,6 @@ def before_sending
# our last chance.
commit! unless committed?

@headers.freeze
@request.commit_cookie_jar! unless committed?
end

Expand Down
30 changes: 0 additions & 30 deletions actionpack/test/dispatch/live_response_test.rb
Expand Up @@ -86,36 +86,6 @@ def test_content_length_is_removed
@response.stream.write "omg"
assert_nil @response.headers["Content-Length"]
end

def test_headers_cannot_be_written_after_web_server_reads
@response.stream.write "omg"
latch = Concurrent::CountDownLatch.new

t = Thread.new {
@response.each do
latch.count_down
end
}

latch.wait
assert_predicate @response.headers, :frozen?
assert_raises(FrozenError) do
@response.headers["Content-Length"] = "zomg"
end

@response.stream.close
t.join
end

def test_headers_cannot_be_written_after_close
@response.stream.close
# we can add data until it's actually written, which happens on `each`
@response.each { |x| }

assert_raises(FrozenError) do
@response.headers["Content-Length"] = "zomg"
end
end
end
end
end

0 comments on commit 45c90b7

Please sign in to comment.