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