Skip to content

Commit dce59e8

Browse files
committed
Fix {Public,Debug}Exceptions body for HEAD
Rack::Lint already errors in this case, it just wasn't being covered. ``` Error: DebugExceptionsTest#test_returns_empty_body_on_HEAD_cascade_pass: Rack::Lint::LintError: Response body was given for HEAD request, but should be empty /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-3.1.15/lib/rack/lint.rb:773:in 'Rack::Lint::Wrapper#verify_content_length' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-3.1.15/lib/rack/lint.rb:899:in 'Rack::Lint::Wrapper#each' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-3.1.15/lib/rack/response.rb:345:in 'Rack::Response::Helpers#buffered_body!' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-3.1.15/lib/rack/mock_response.rb:65:in 'Rack::MockResponse#initialize' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-test-2.2.0/lib/rack/test.rb:362:in 'Class#new' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-test-2.2.0/lib/rack/test.rb:362:in 'Rack::Test::Session#process_request' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-test-2.2.0/lib/rack/test.rb:153:in 'Rack::Test::Session#request' lib/action_dispatch/testing/integration.rb:297:in 'ActionDispatch::Integration::Session#process' lib/action_dispatch/testing/integration.rb:49:in 'ActionDispatch::Integration::RequestHelpers#head' lib/action_dispatch/testing/integration.rb:388:in 'ActionDispatch::Integration::Runner#head' test/dispatch/debug_exceptions_test.rb:189:in 'block in <class:DebugExceptionsTest>' Error: ShowExceptionsTest#test_rescue_with_no_body_for_HEAD_requests: Rack::Lint::LintError: Response body was given for HEAD request, but should be empty /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-3.1.15/lib/rack/lint.rb:773:in 'Rack::Lint::Wrapper#verify_content_length' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-3.1.15/lib/rack/lint.rb:900:in 'Rack::Lint::Wrapper#each' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-3.1.15/lib/rack/response.rb:345:in 'Rack::Response::Helpers#buffered_body!' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-3.1.15/lib/rack/mock_response.rb:65:in 'Rack::MockResponse#initialize' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-test-2.2.0/lib/rack/test.rb:362:in 'Class#new' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-test-2.2.0/lib/rack/test.rb:362:in 'Rack::Test::Session#process_request' /home/hartley/.local/share/mise/installs/ruby/3.4.4/lib/ruby/gems/3.4.0/gems/rack-test-2.2.0/lib/rack/test.rb:153:in 'Rack::Test::Session#request' lib/action_dispatch/testing/integration.rb:297:in 'ActionDispatch::Integration::Session#process' lib/action_dispatch/testing/integration.rb:49:in 'ActionDispatch::Integration::RequestHelpers#head' lib/action_dispatch/testing/integration.rb:388:in 'ActionDispatch::Integration::Runner#head' test/dispatch/show_exceptions_test.rb:73:in 'block in <class:ShowExceptionsTest>' ```
1 parent fc55e5b commit dce59e8

File tree

5 files changed

+50
-2
lines changed

5 files changed

+50
-2
lines changed

actionpack/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
* Always return empty body for HEAD requests in `PublicExceptions` and
2+
`DebugExceptions`.
3+
4+
This is required by `Rack::Lint` (per RFC9110).
5+
6+
*Hartley McGuire*
7+
18
* Add comprehensive support for HTTP Cache-Control request directives according to RFC 9111.
29

310
Provides a `request.cache_control_directives` object that gives access to request cache directives:

actionpack/lib/action_dispatch/middleware/debug_exceptions.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ def render_exception(request, exception, wrapper)
6565
content_type = Mime[:text]
6666
end
6767

68-
if api_request?(content_type)
68+
if request.head?
69+
render(wrapper.status_code, "", content_type)
70+
elsif api_request?(content_type)
6971
render_for_api_request(content_type, wrapper)
7072
else
7173
render_for_browser_request(request, wrapper)

actionpack/lib/action_dispatch/middleware/public_exceptions.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,11 @@ def call(env)
2828
content_type = request.formats.first
2929
body = { status: status, error: Rack::Utils::HTTP_STATUS_CODES.fetch(status, Rack::Utils::HTTP_STATUS_CODES[500]) }
3030

31-
render(status, content_type, body)
31+
if env["action_dispatch.original_request_method"] == "HEAD"
32+
render_format(status, content_type, "")
33+
else
34+
render(status, content_type, body)
35+
end
3236
end
3337

3438
private

actionpack/test/dispatch/debug_exceptions_test.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,15 @@ def self.build_app(app, *args)
183183
assert boomer.closed, "Expected to close the response body"
184184
end
185185

186+
test "returns empty body on HEAD cascade pass" do
187+
@app = DevelopmentApp
188+
189+
head "/pass"
190+
191+
assert_response 404
192+
assert_equal "", body
193+
end
194+
186195
test "displays routes in a table when a RoutingError occurs" do
187196
@app = DevelopmentApp
188197
get "/pass", headers: { "action_dispatch.show_exceptions" => :all }

actionpack/test/dispatch/show_exceptions_test.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,32 @@ def setup
6969
assert_equal "", body
7070
end
7171

72+
test "rescue with no body for HEAD requests" do
73+
head "/", env: { "action_dispatch.show_exceptions" => :all }
74+
assert_response 500
75+
assert_equal "", body
76+
77+
head "/bad_params", env: { "action_dispatch.show_exceptions" => :all }
78+
assert_response 400
79+
assert_equal "", body
80+
81+
head "/not_found", env: { "action_dispatch.show_exceptions" => :all }
82+
assert_response 404
83+
assert_equal "", body
84+
85+
head "/method_not_allowed", env: { "action_dispatch.show_exceptions" => :all }
86+
assert_response 405
87+
assert_equal "", body
88+
89+
head "/unknown_http_method", env: { "action_dispatch.show_exceptions" => :all }
90+
assert_response 405
91+
assert_equal "", body
92+
93+
head "/invalid_mimetype", headers: { "Accept" => "text/html,*", "action_dispatch.show_exceptions" => :all }
94+
assert_response 406
95+
assert_equal "", body
96+
end
97+
7298
test "localize rescue error page" do
7399
old_locale, I18n.locale = I18n.locale, :da
74100

0 commit comments

Comments
 (0)