From 76c7870abeb82a5917dd27513416fd2b45f34bde Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 22 May 2026 12:11:34 -0700 Subject: [PATCH] Skip auto-inserting Rack::Lint on Rack 3+ Pitchfork's default app builder injects Rack::Lint into the middleware stack whenever RACK_ENV is "development" (which exe/pitchfork sets via ENV["RACK_ENV"] ||= "development" if unset). On Rack 3, the strict Rack::Lint::Wrapper::InputWrapper no longer exposes rewind, but Rails middleware (cookies, body parsers) and most third-party Rack middleware still call request.body.rewind unconditionally, so the moment a Rails dev process switches from Puma to Pitchfork every cookie- or body-bearing request 500s with undefined method 'rewind' for an instance of Rack::Lint::Wrapper::InputWrapper The dev-affordance value of the auto-insert no longer outweighs the trap on Rack 3+. Gate the insert on Rack::RELEASE < "3" so Rack 2 users keep the existing behavior and Rack 3 users stop tripping over the incompatibility. Adds an integration test using the existing fails-rack-lint.ru fixture to lock the new behavior in. Closes #177. --- lib/pitchfork.rb | 13 ++++++++++++- test/integration/test_rack_lint.rb | 31 ++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 test/integration/test_rack_lint.rb diff --git a/lib/pitchfork.rb b/lib/pitchfork.rb index f6b29ede..f161ff8d 100644 --- a/lib/pitchfork.rb +++ b/lib/pitchfork.rb @@ -97,7 +97,18 @@ def builder(ru, op) Rack::Builder.new do use(Rack::ContentLength) use(Pitchfork::Chunked) - use(Rack::Lint) if ENV["RACK_ENV"] == "development" + # Rack 3 dropped `rewind` from the strict `rack.input` surface, so + # `Rack::Lint::Wrapper::InputWrapper` no longer exposes it. Rails + # middleware (cookies, body parsers) and third-party Rack middleware + # still call `request.body.rewind` (or read the body twice), so + # inserting `Rack::Lint` here in development silently breaks every + # body-bearing request the moment a config switches a dev `web:` + # process to Pitchfork. The dev-affordance value of Lint is no longer + # worth the trap on Rack 3+, so only auto-insert under Rack 2. + # See https://github.com/Shopify/pitchfork/issues/177. + if ENV["RACK_ENV"] == "development" && (!defined?(Rack::RELEASE) || Rack::RELEASE < "3") + use(Rack::Lint) + end use(Rack::TempfileReaper) run inner_app end.to_app diff --git a/test/integration/test_rack_lint.rb b/test/integration/test_rack_lint.rb new file mode 100644 index 00000000..7c3f4b88 --- /dev/null +++ b/test/integration/test_rack_lint.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true +require 'integration_test_helper' + +class RackLintTest < Pitchfork::IntegrationTest + # `fails-rack-lint.ru` returns a header named "status", which Rack::Lint + # rejects with a 500. On Rack 3+, Pitchfork no longer auto-inserts Rack::Lint + # (it drops `rewind` from `rack.input` in a way that breaks Rails middleware, + # see https://github.com/Shopify/pitchfork/issues/177), so the response should + # come back from the app unchanged. + def test_no_auto_rack_lint_on_rack_3 + skip("Only relevant on Rack 3+") unless defined?(Rack::RELEASE) && Rack::RELEASE >= "3" + + addr, port = unused_port + + # lint: true keeps the default RACK_ENV (which the Pitchfork CLI sets to + # "development"). On Rack 2 this would inject Rack::Lint and the request + # below would 500; on Rack 3+ the gate skips the insert and the app's body + # comes through. + pid = spawn_server(app: File.join(ROOT, "test/integration/fails-rack-lint.ru"), lint: true, config: <<~CONFIG) + listen "#{addr}:#{port}" + CONFIG + + assert_healthy("http://#{addr}:#{port}") + + response = Net::HTTP.get_response(URI("http://#{addr}:#{port}")) + assert_equal 200, response.code.to_i + assert_equal "Rack::Lint wasn't there if you see this", response.body.strip + + assert_clean_shutdown(pid) + end +end