Fix {Public,Debug}Exceptions rendering body for HEAD #55149

skipkayhil · 2025-06-04T03:17:36Z

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>'

Checklist

Before submitting the PR make sure the following are checked:

This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
Tests are added or updated if you fix a bug or add a feature.
CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

Edouard-chin · 2025-06-04T15:54:59Z

actionpack/lib/action_dispatch/middleware/show_exceptions.rb

        response = @exceptions_app.call(request.env)
-        response[1][Constants::X_CASCADE] == "pass" ? pass_response(status) : response
+
+        if response[1][Constants::X_CASCADE] == "pass" || head_request


I think this should belong in the PublicException app, otherwise we'd be discarding custom headers sent by a custom PublicException app (We'd just send the X_CASCADE header).

Another small benefit if we move this in the PublicException app is that we won't need to render the template since we don't need it.

The downside is that application will have to implement this in their PublicException app if they use a custom one.

I agree, that's a much better change. I missed that ShowExceptions sets action_dispatch.original_request_method which is what was needed to make that work!

Edouard-chin · 2025-06-04T18:17:39Z

👍 !

(You can rebase from main if you want CI to be green)

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>' ```

byroot · 2025-06-05T20:16:58Z

actionpack/lib/action_dispatch/middleware/public_exceptions.rb

@@ -28,7 +28,11 @@ def call(env)
      content_type = request.formats.first
      body = { status: status, error: Rack::Utils::HTTP_STATUS_CODES.fetch(status, Rack::Utils::HTTP_STATUS_CODES[500]) }

-      render(status, content_type, body)
+      if env["action_dispatch.original_request_method"] == "HEAD"


Suggested change

if env["action_dispatch.original_request_method"] == "HEAD"

if request.raw_request_method == "HEAD

Not a huge deal, but since we instantiated a request, we could use the slightly nicer API.

Nevermind, Running your tests I see why you used that now. It's to avoid method rewrites.

Fix {Public,Debug}Exceptions rendering body for HEAD Co-authored-by: Hartley McGuire <skipkayhil@gmail.com>

Backport #55149 to 8-0-stable

Backport #55149 to 7-2-stable

rails-bot bot added the actionpack label Jun 4, 2025

Edouard-chin reviewed Jun 4, 2025

View reviewed changes

skipkayhil force-pushed the hm-lint-head-body branch from 4659801 to e006dac Compare June 4, 2025 17:25

skipkayhil changed the title ~~Fix {Show,Debug}Exceptions rendering body for HEAD~~ Fix {Public,Debug}Exceptions rendering body for HEAD Jun 4, 2025

skipkayhil force-pushed the hm-lint-head-body branch from e006dac to dce59e8 Compare June 4, 2025 19:43

byroot reviewed Jun 5, 2025

View reviewed changes

byroot merged commit 4b7fb1d into rails:main Jun 5, 2025
3 checks passed

skipkayhil added a commit to skipkayhil/rails that referenced this pull request Jun 5, 2025

Merge pull request rails#55149 from skipkayhil/hm-lint-head-body

4fa08c9

Fix {Public,Debug}Exceptions rendering body for HEAD Co-authored-by: Hartley McGuire <skipkayhil@gmail.com>

skipkayhil added a commit to skipkayhil/rails that referenced this pull request Jun 5, 2025

Merge pull request rails#55149 from skipkayhil/hm-lint-head-body

253bb29

Fix {Public,Debug}Exceptions rendering body for HEAD Co-authored-by: Hartley McGuire <skipkayhil@gmail.com>

This was referenced Jun 5, 2025

Backport #55149 to 8-0-stable #55162

Merged

Backport #55149 to 7-2-stable #55163

Merged

eileencodes added a commit that referenced this pull request Jun 9, 2025

Merge pull request #55162 from skipkayhil/hm-55149-8-0

b0f3652

Backport #55149 to 8-0-stable

eileencodes added a commit that referenced this pull request Jun 9, 2025

Merge pull request #55163 from skipkayhil/hm-55149-72

8b639e4

Backport #55149 to 7-2-stable

skipkayhil deleted the hm-lint-head-body branch June 9, 2025 15:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix {Public,Debug}Exceptions rendering body for HEAD #55149

Fix {Public,Debug}Exceptions rendering body for HEAD #55149

Uh oh!

skipkayhil commented Jun 4, 2025

Uh oh!

Edouard-chin Jun 4, 2025 •

edited

Loading

Uh oh!

skipkayhil Jun 4, 2025

Uh oh!

Edouard-chin commented Jun 4, 2025

Uh oh!

byroot Jun 5, 2025

Uh oh!

byroot Jun 5, 2025

Uh oh!

Uh oh!

Uh oh!

	if env["action_dispatch.original_request_method"] == "HEAD"
	if request.raw_request_method == "HEAD

Fix {Public,Debug}Exceptions rendering body for HEAD #55149

Fix {Public,Debug}Exceptions rendering body for HEAD #55149

Uh oh!

Conversation

skipkayhil commented Jun 4, 2025

Checklist

Uh oh!

Edouard-chin Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skipkayhil Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

Edouard-chin commented Jun 4, 2025

Uh oh!

byroot Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

byroot Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Edouard-chin Jun 4, 2025 •

edited

Loading