From 4e69dd46cc3f42ca3795bf487437111236fbba23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= Date: Wed, 30 Jul 2025 16:24:31 +0200 Subject: [PATCH] Ensure compatibility with Rack 2.x and 3.x Some parts of the code are changing the headers from the Rack request, but are using capitalized headers when doing so. If the underlying object is not a bare hash but either a `Rack::Utils::HeaderHash` or a `Rack::Headers` object, then everything will work as expected. But sometimes the headers object can be a bare hash (that can be the case when Rails is serving static files) and if Rack 3 is in use, things can break. This patch ensures the headers names are compliant with both versions of Rack. (mainly by using things like `Rack::CONTENT_TYPE` instead of writing directly `Content-Type`/`content-type`) --- .github/workflows/ci.yml | 16 +++++---- Gemfile | 4 +++ lib/mini_profiler.rb | 18 +++++----- lib/mini_profiler/gc_profiler.rb | 2 +- spec/integration/middleware_spec.rb | 24 +++++++------- spec/integration/mini_profiler_spec.rb | 40 +++++++++++------------ spec/integration/trace_exceptions_spec.rb | 4 +-- spec/lib/client_settings_spec.rb | 8 ++--- spec/lib/profiler_spec.rb | 8 ++--- 9 files changed, 65 insertions(+), 59 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b4056dd..66a71607 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,8 +12,12 @@ env: jobs: build: runs-on: ubuntu-latest - name: Ruby ${{ matrix.ruby }} + name: Ruby ${{ matrix.ruby }} (Rack ~> ${{ matrix.rack }}) services: + redis: + image: "redis" + ports: + - 6379:6379 memcached: image: memcached:1.6.9 ports: @@ -22,8 +26,10 @@ jobs: strategy: fail-fast: false matrix: - ruby: ["3.4", "3.3", "3.2", "3.1"] - redis: ["5.x"] + ruby: ["3.4", "3.3", "3.2"] + rack: ["2.0", "3.0"] + env: + RACK_VERSION: ${{ matrix.rack }} steps: - uses: actions/checkout@v4 - uses: ruby/setup-ruby@v1 @@ -31,10 +37,6 @@ jobs: ruby-version: ${{ matrix.ruby }} bundler-cache: true rubygems: latest # `gem update --system` is run to update to the latest compatible RubyGems version. - - name: Setup redis - uses: shogo82148/actions-setup-redis@v1 - with: - redis-version: ${{ matrix.redis }} - name: Tests run: bundle exec rspec - name: Rubocop diff --git a/Gemfile b/Gemfile index f26e9fff..8e7a41a3 100644 --- a/Gemfile +++ b/Gemfile @@ -5,6 +5,10 @@ ruby '>= 2.7.0' gemspec +if ENV["CI"] + gem "rack", "~> #{ENV['RACK_VERSION']}" +end + group :test do gem 'codecov', require: false gem 'stackprof', require: false diff --git a/lib/mini_profiler.rb b/lib/mini_profiler.rb index a21a2568..3eb012bc 100644 --- a/lib/mini_profiler.rb +++ b/lib/mini_profiler.rb @@ -329,7 +329,7 @@ def call(env) ) end elsif path == '/rack-mini-profiler/requests' - status, headers, body = [200, { 'Content-Type' => 'text/html' }, [blank_page_html.dup]] # important to dup here! + status, headers, body = [200, { Rack::CONTENT_TYPE => 'text/html' }, [blank_page_html.dup]] # important to dup here! else status, headers, body = @app.call(env) end @@ -423,20 +423,20 @@ def action_parameters(env) def inject_profiler(env, status, headers, body) # mini profiler is meddling with stuff, we can not cache cause we will get incorrect data # Rack::ETag has already inserted some nonesense in the chain - content_type = headers['Content-Type'] + content_type = headers[Rack::CONTENT_TYPE] if config.disable_caching - headers.delete('ETag') - headers.delete('Date') + headers.delete(Rack::ETAG) + headers.delete('date') || headers.delete('Date') end - headers['X-MiniProfiler-Original-Cache-Control'] = headers['Cache-Control'] unless headers['Cache-Control'].nil? - headers['Cache-Control'] = "#{"no-store, " if config.disable_caching}must-revalidate, private, max-age=0" + headers['x-miniprofiler-original-cache-control'] = headers[Rack::CACHE_CONTROL] unless headers[Rack::CACHE_CONTROL].nil? + headers[Rack::CACHE_CONTROL] = "#{"no-store, " if config.disable_caching}must-revalidate, private, max-age=0" # inject header if headers.is_a? Hash - headers['X-MiniProfiler-Ids'] = ids_comma_separated(env) - headers['X-MiniProfiler-Flamegraph-Path'] = flamegraph_path(env) if current.page_struct[:has_flamegraph] + headers['x-miniprofiler-ids'] = ids_comma_separated(env) + headers['x-miniprofiler-flamegraph-path'] = flamegraph_path(env) if current.page_struct[:has_flamegraph] end if current.inject_js && content_type =~ /text\/html/ @@ -589,7 +589,7 @@ def analyze_memory end def text_result(body, status: 200, headers: nil) - headers = (headers || {}).merge('Content-Type' => 'text/plain; charset=utf-8') + headers = (headers || {}).merge(Rack::CONTENT_TYPE => 'text/plain; charset=utf-8') [status, headers, [body]] end diff --git a/lib/mini_profiler/gc_profiler.rb b/lib/mini_profiler/gc_profiler.rb index 8dcb3122..a578238b 100644 --- a/lib/mini_profiler/gc_profiler.rb +++ b/lib/mini_profiler/gc_profiler.rb @@ -151,7 +151,7 @@ def profile_gc(app, env) body << "#{count} : #{string}\n" end - [200, { 'Content-Type' => 'text/plain' }, body] + [200, { Rack::CONTENT_TYPE => 'text/plain' }, body] ensure prev_gc_state ? GC.disable : GC.enable end diff --git a/spec/integration/middleware_spec.rb b/spec/integration/middleware_spec.rb index 65ba066f..14fb1df2 100644 --- a/spec/integration/middleware_spec.rb +++ b/spec/integration/middleware_spec.rb @@ -20,7 +20,7 @@ def decompressed_response def app Rack::Builder.new do use Rack::MiniProfiler - run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'

Hi

']] } + run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'

Hi

']] } end end it 'is an empty page' do @@ -35,7 +35,7 @@ def app def app Rack::Builder.new do use Rack::MiniProfiler - run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'

Hi

']] } + run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'

Hi

']] } end end it 'shows commands' do @@ -75,7 +75,7 @@ def app def app Rack::Builder.new do use Rack::MiniProfiler - run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'

Hi

']] } + run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'

Hi

']] } end end it 'advanced tools are disabled' do @@ -94,7 +94,7 @@ def app lambda do |_env| [ 201, - { 'Content-Type' => 'text/html', 'X-CUSTOM' => "1" }, + { Rack::CONTENT_TYPE => 'text/html', 'X-CUSTOM' => "1" }, [+'

Hi

'], ] end @@ -118,7 +118,7 @@ def app expect(last_response.body).to eq( 'Please install the memory_profiler gem and require it: add gem \'memory_profiler\' to your Gemfile' ) - expect(last_response.headers['Content-Type']).to eq('text/plain; charset=utf-8') + expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain; charset=utf-8') expect(last_response.headers['X-CUSTOM']).to eq('1') expect(last_response.status).to eq(500) end @@ -130,7 +130,7 @@ def app expect(last_response.body).to eq( 'Please install the stackprof gem and require it: add gem \'stackprof\' to your Gemfile' ) - expect(last_response.headers['Content-Type']).to eq('text/plain; charset=utf-8') + expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain; charset=utf-8') expect(last_response.headers['X-CUSTOM']).to eq('1') expect(last_response.status).to eq(201) end @@ -142,7 +142,7 @@ def app expect(last_response.body).to eq( 'Please install the stackprof gem and require it: add gem \'stackprof\' to your Gemfile' ) - expect(last_response.headers['Content-Type']).to eq('text/plain; charset=utf-8') + expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain; charset=utf-8') expect(last_response.headers['X-CUSTOM']).to eq('1') expect(last_response.status).to eq(201) end @@ -154,7 +154,7 @@ def app Rack::Builder.new do use Rack::MiniProfiler use Rack::Deflater - run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'

Hi

']] } + run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'

Hi

']] } end end @@ -190,7 +190,7 @@ def app Rack::Builder.new do use Rack::Deflater use Rack::MiniProfiler - run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'

Hi

']] } + run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'

Hi

']] } end end @@ -223,7 +223,7 @@ def app def app Rack::Builder.new do use Rack::MiniProfiler - run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'

Hi

']] } + run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'

Hi

']] } end end @@ -249,7 +249,7 @@ def app use Rack::MiniProfiler run lambda { |env| env["action_dispatch.content_security_policy_nonce"] = "railsnonce" - [200, { 'Content-Type' => 'text/html' }, [+'

Hello world

']] + [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'

Hello world

']] } end end @@ -278,7 +278,7 @@ def app (env, response_headers) = proc_arguments expect(env["REQUEST_METHOD"]).to eq("GET") - expect(response_headers["Content-Type"]).to eq("text/html") + expect(response_headers[Rack::CONTENT_TYPE]).to eq("text/html") end end end diff --git a/spec/integration/mini_profiler_spec.rb b/spec/integration/mini_profiler_spec.rb index 0fca154d..6e53afcc 100644 --- a/spec/integration/mini_profiler_spec.rb +++ b/spec/integration/mini_profiler_spec.rb @@ -10,88 +10,88 @@ def app @app ||= Rack::Builder.new { use Rack::MiniProfiler map '/path2/a' do - run lambda { |env| [200, { 'Content-Type' => 'text/html' }, +'

path1

'] } + run lambda { |env| [200, { Rack::CONTENT_TYPE => 'text/html' }, +'

path1

'] } end map '/path1/a' do - run lambda { |env| [200, { 'Content-Type' => 'text/html' }, +'

path2

'] } + run lambda { |env| [200, { Rack::CONTENT_TYPE => 'text/html' }, +'

path2

'] } end map '/cached-resource' do run lambda { |env| ims = env['HTTP_IF_MODIFIED_SINCE'] || "" if ims.size > 0 - [304, { 'Content-Type' => 'application/json' }, ''] + [304, { Rack::CONTENT_TYPE => 'application/json' }, ''] else - [200, { 'Content-Type' => 'application/json', 'Cache-Control' => 'original-cache-control' }, '{"name": "Ryan"}'] + [200, { Rack::CONTENT_TYPE => 'application/json', Rack::CACHE_CONTROL => 'original-cache-control' }, '{"name": "Ryan"}'] end } end map '/post' do - run lambda { |env| [302, { 'Content-Type' => 'text/html' }, +'

POST

'] } + run lambda { |env| [302, { Rack::CONTENT_TYPE => 'text/html' }, +'

POST

'] } end map '/html' do - run lambda { |env| [200, { 'Content-Type' => 'text/html' }, +"

Hi

\n \t"] } + run lambda { |env| [200, { Rack::CONTENT_TYPE => 'text/html' }, +"

Hi

\n \t"] } end map '/explicitly-allowed-html' do run lambda { |env| Rack::MiniProfiler.authorize_request - [200, { 'Content-Type' => 'text/html' }, +"

Hi

\n \t"] + [200, { Rack::CONTENT_TYPE => 'text/html' }, +"

Hi

\n \t"] } end map '/implicitbody' do - run lambda { |env| [200, { 'Content-Type' => 'text/html' }, +"

Hi

"] } + run lambda { |env| [200, { Rack::CONTENT_TYPE => 'text/html' }, +"

Hi

"] } end map '/implicitbodyhtml' do - run lambda { |env| [200, { 'Content-Type' => 'text/html' }, +"

Hi

"] } + run lambda { |env| [200, { Rack::CONTENT_TYPE => 'text/html' }, +"

Hi

"] } end map '/db' do run lambda { |env| ::Rack::MiniProfiler.record_sql("I want to be, in a db", 10) - [200, { 'Content-Type' => 'text/html' }, +'

Hi+db

'] + [200, { Rack::CONTENT_TYPE => 'text/html' }, +'

Hi+db

'] } end map '/3ms' do run lambda { |env| sleep(0.003) - [200, { 'Content-Type' => 'text/html' }, +'

Hi

'] + [200, { Rack::CONTENT_TYPE => 'text/html' }, +'

Hi

'] } end map '/explicitly-allowed' do run lambda { |env| Rack::MiniProfiler.authorize_request - [200, { 'Content-Type' => 'text/html' }, +'

path1

'] + [200, { Rack::CONTENT_TYPE => 'text/html' }, +'

path1

'] } end map '/rails_engine' do run lambda { |env| env['SCRIPT_NAME'] = '/rails_engine' # Rails engines do that - [200, { 'Content-Type' => 'text/html' }, +'

Hi

'] + [200, { Rack::CONTENT_TYPE => 'text/html' }, +'

Hi

'] } end map '/under_passenger' do run lambda { |env| - [200, { 'Content-Type' => 'text/html' }, +'

and I ride and I ride

'] + [200, { Rack::CONTENT_TYPE => 'text/html' }, +'

and I ride and I ride

'] } end map '/create' do run lambda { |env| - [201, { 'Content-Type' => 'text/html' }, +'

success

'] + [201, { Rack::CONTENT_TYPE => 'text/html' }, +'

success

'] } end map '/notallowed' do run lambda { |env| - [403, { 'Content-Type' => 'text/html' }, +'

you are not allowed here

'] + [403, { Rack::CONTENT_TYPE => 'text/html' }, +'

you are not allowed here

'] } end map '/whoopsie-daisy' do run lambda { |env| - [500, { 'Content-Type' => 'text/html' }, +'

whoopsie daisy

'] + [500, { Rack::CONTENT_TYPE => 'text/html' }, +'

whoopsie daisy

'] } end map '/test-snapshots-custom-fields' do run lambda { |env| qp = Rack::Utils.parse_nested_query(env['QUERY_STRING']) qp.each { |k, v| Rack::MiniProfiler.add_snapshot_custom_field(k, v) } - [200, { 'Content-Type' => 'text/html' }, +'

custom fields

'] + [200, { Rack::CONTENT_TYPE => 'text/html' }, +'

custom fields

'] } end }.to_app @@ -450,12 +450,12 @@ def load_prof(response) describe 'gc profiler' do it "should return a report" do get '/html?pp=profile-gc' - expect(last_response['Content-Type']).to include('text/plain') + expect(last_response[Rack::CONTENT_TYPE]).to include('text/plain') end it "should return a report when an HTTP header is used" do get '/html', nil, { 'HTTP_X_RACK_MINI_PROFILER' => 'profile-gc' } - expect(last_response['Content-Type']).to include('text/plain') + expect(last_response[Rack::CONTENT_TYPE]).to include('text/plain') end end diff --git a/spec/integration/trace_exceptions_spec.rb b/spec/integration/trace_exceptions_spec.rb index ff77acb1..6ecc1945 100644 --- a/spec/integration/trace_exceptions_spec.rb +++ b/spec/integration/trace_exceptions_spec.rb @@ -14,7 +14,7 @@ def app @app ||= Rack::Builder.new { use Rack::MiniProfiler map '/no_exceptions' do - run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, '

Success

'] } + run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, '

Success

'] } end map '/raise_exceptions' do # This route raises 3 exceptions, catches them, and returns a successful response @@ -34,7 +34,7 @@ def app rescue # Ignore the exception end - [200, { 'Content-Type' => 'text/html' }, '

Exception raised but success returned

'] + [200, { Rack::CONTENT_TYPE => 'text/html' }, '

Exception raised but success returned

'] } end }.to_app diff --git a/spec/lib/client_settings_spec.rb b/spec/lib/client_settings_spec.rb index bd8d003e..37b883f8 100644 --- a/spec/lib/client_settings_spec.rb +++ b/spec/lib/client_settings_spec.rb @@ -44,7 +44,7 @@ @settings.disable_profiling = false hash = {} @settings.write!(hash) - expect(hash["set-cookie"]).to include("path=/;") + expect(hash[Rack::SET_COOKIE]).to include("path=/;") end it 'should correctly set cookie with correct path' do @@ -52,7 +52,7 @@ @settings.disable_profiling = false hash = {} @settings.write!(hash) - expect(hash["set-cookie"]).to include("path=/test;") + expect(hash[Rack::SET_COOKIE]).to include("path=/test;") end it 'writes auth token for authorized reqs' do @@ -60,7 +60,7 @@ Rack::MiniProfiler.authorize_request hash = {} @settings.write!(hash) - expect(hash["set-cookie"]).to include(@store.allowed_tokens.join("|")) + expect(hash[Rack::SET_COOKIE]).to include(@store.allowed_tokens.join("|")) end it 'does nothing on short unauthed requests' do @@ -80,7 +80,7 @@ @settings.handle_cookie([200, hash, []]) end - expect(hash["set-cookie"]).to include("max-age=0") + expect(hash[Rack::SET_COOKIE]).to include("max-age=0") end end diff --git a/spec/lib/profiler_spec.rb b/spec/lib/profiler_spec.rb index 4ac7ab3d..8f3d75c4 100644 --- a/spec/lib/profiler_spec.rb +++ b/spec/lib/profiler_spec.rb @@ -208,9 +208,9 @@ def self.bar(baz, boo) it "returns error response when stackprof isn't installed" do response = profiler.call({ "PATH_INFO" => "/", "QUERY_STRING" => "pp=flamegraph" }) - expect(response).to eq([ + expect(response).to match([ 200, - { "Content-Type" => "text/plain; charset=utf-8", "set-cookie" => "__profilin=p%3Dt; path=/; httponly; samesite=lax" }, + { Rack::CONTENT_TYPE => "text/plain; charset=utf-8", Rack::SET_COOKIE => %r{^__profilin=p%3Dt; path=/; httponly; samesite=lax$}i }, ["Please install the stackprof gem and require it: add gem 'stackprof' to your Gemfile"], ]) end @@ -221,9 +221,9 @@ def self.bar(baz, boo) response = profiler.call({ "PATH_INFO" => "/", "QUERY_STRING" => "pp=profile-memory" }) - expect(response).to eq([ + expect(response).to match([ 500, - { "Content-Type" => "text/plain; charset=utf-8", "set-cookie" => "__profilin=p%3Dt; path=/; httponly; samesite=lax" }, + { Rack::CONTENT_TYPE => "text/plain; charset=utf-8", Rack::SET_COOKIE => %r{^__profilin=p%3Dt; path=/; httponly; samesite=lax$}i }, ["Please install the memory_profiler gem and require it: add gem 'memory_profiler' to your Gemfile"], ])