Skip to content

Commit 4e69dd4

Browse files
FlinkFlink
committed
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`)
1 parent bb1d257 commit 4e69dd4

File tree

9 files changed

+65
-59
lines changed

9 files changed

+65
-59
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,12 @@ env:
1212
jobs:
1313
build:
1414
runs-on: ubuntu-latest
15-
name: Ruby ${{ matrix.ruby }}
15+
name: Ruby ${{ matrix.ruby }} (Rack ~> ${{ matrix.rack }})
1616
services:
17+
redis:
18+
image: "redis"
19+
ports:
20+
- 6379:6379
1721
memcached:
1822
image: memcached:1.6.9
1923
ports:
@@ -22,19 +26,17 @@ jobs:
2226
strategy:
2327
fail-fast: false
2428
matrix:
25-
ruby: ["3.4", "3.3", "3.2", "3.1"]
26-
redis: ["5.x"]
29+
ruby: ["3.4", "3.3", "3.2"]
30+
rack: ["2.0", "3.0"]
31+
env:
32+
RACK_VERSION: ${{ matrix.rack }}
2733
steps:
2834
- uses: actions/checkout@v4
2935
- uses: ruby/setup-ruby@v1
3036
with:
3137
ruby-version: ${{ matrix.ruby }}
3238
bundler-cache: true
3339
rubygems: latest # `gem update --system` is run to update to the latest compatible RubyGems version.
34-
- name: Setup redis
35-
uses: shogo82148/actions-setup-redis@v1
36-
with:
37-
redis-version: ${{ matrix.redis }}
3840
- name: Tests
3941
run: bundle exec rspec
4042
- name: Rubocop

‎Gemfile‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ ruby '>= 2.7.0'
55

66
gemspec
77

8+
if ENV["CI"]
9+
gem "rack", "~> #{ENV['RACK_VERSION']}"
10+
end
11+
812
group :test do
913
gem 'codecov', require: false
1014
gem 'stackprof', require: false

‎lib/mini_profiler.rb‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ def call(env)
329329
)
330330
end
331331
elsif path == '/rack-mini-profiler/requests'
332-
status, headers, body = [200, { 'Content-Type' => 'text/html' }, [blank_page_html.dup]] # important to dup here!
332+
status, headers, body = [200, { Rack::CONTENT_TYPE => 'text/html' }, [blank_page_html.dup]] # important to dup here!
333333
else
334334
status, headers, body = @app.call(env)
335335
end
@@ -423,20 +423,20 @@ def action_parameters(env)
423423
def inject_profiler(env, status, headers, body)
424424
# mini profiler is meddling with stuff, we can not cache cause we will get incorrect data
425425
# Rack::ETag has already inserted some nonesense in the chain
426-
content_type = headers['Content-Type']
426+
content_type = headers[Rack::CONTENT_TYPE]
427427

428428
if config.disable_caching
429-
headers.delete('ETag')
430-
headers.delete('Date')
429+
headers.delete(Rack::ETAG)
430+
headers.delete('date') || headers.delete('Date')
431431
end
432432

433-
headers['X-MiniProfiler-Original-Cache-Control'] = headers['Cache-Control'] unless headers['Cache-Control'].nil?
434-
headers['Cache-Control'] = "#{"no-store, " if config.disable_caching}must-revalidate, private, max-age=0"
433+
headers['x-miniprofiler-original-cache-control'] = headers[Rack::CACHE_CONTROL] unless headers[Rack::CACHE_CONTROL].nil?
434+
headers[Rack::CACHE_CONTROL] = "#{"no-store, " if config.disable_caching}must-revalidate, private, max-age=0"
435435

436436
# inject header
437437
if headers.is_a? Hash
438-
headers['X-MiniProfiler-Ids'] = ids_comma_separated(env)
439-
headers['X-MiniProfiler-Flamegraph-Path'] = flamegraph_path(env) if current.page_struct[:has_flamegraph]
438+
headers['x-miniprofiler-ids'] = ids_comma_separated(env)
439+
headers['x-miniprofiler-flamegraph-path'] = flamegraph_path(env) if current.page_struct[:has_flamegraph]
440440
end
441441

442442
if current.inject_js && content_type =~ /text\/html/
@@ -589,7 +589,7 @@ def analyze_memory
589589
end
590590

591591
def text_result(body, status: 200, headers: nil)
592-
headers = (headers || {}).merge('Content-Type' => 'text/plain; charset=utf-8')
592+
headers = (headers || {}).merge(Rack::CONTENT_TYPE => 'text/plain; charset=utf-8')
593593
[status, headers, [body]]
594594
end
595595

‎lib/mini_profiler/gc_profiler.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ def profile_gc(app, env)
151151
body << "#{count} : #{string}\n"
152152
end
153153

154-
[200, { 'Content-Type' => 'text/plain' }, body]
154+
[200, { Rack::CONTENT_TYPE => 'text/plain' }, body]
155155
ensure
156156
prev_gc_state ? GC.disable : GC.enable
157157
end

‎spec/integration/middleware_spec.rb‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ def decompressed_response
2020
def app
2121
Rack::Builder.new do
2222
use Rack::MiniProfiler
23-
run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
23+
run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
2424
end
2525
end
2626
it 'is an empty page' do
@@ -35,7 +35,7 @@ def app
3535
def app
3636
Rack::Builder.new do
3737
use Rack::MiniProfiler
38-
run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
38+
run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
3939
end
4040
end
4141
it 'shows commands' do
@@ -75,7 +75,7 @@ def app
7575
def app
7676
Rack::Builder.new do
7777
use Rack::MiniProfiler
78-
run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
78+
run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
7979
end
8080
end
8181
it 'advanced tools are disabled' do
@@ -94,7 +94,7 @@ def app
9494
lambda do |_env|
9595
[
9696
201,
97-
{ 'Content-Type' => 'text/html', 'X-CUSTOM' => "1" },
97+
{ Rack::CONTENT_TYPE => 'text/html', 'X-CUSTOM' => "1" },
9898
[+'<html><body><h1>Hi</h1></body></html>'],
9999
]
100100
end
@@ -118,7 +118,7 @@ def app
118118
expect(last_response.body).to eq(
119119
'Please install the memory_profiler gem and require it: add gem \'memory_profiler\' to your Gemfile'
120120
)
121-
expect(last_response.headers['Content-Type']).to eq('text/plain; charset=utf-8')
121+
expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain; charset=utf-8')
122122
expect(last_response.headers['X-CUSTOM']).to eq('1')
123123
expect(last_response.status).to eq(500)
124124
end
@@ -130,7 +130,7 @@ def app
130130
expect(last_response.body).to eq(
131131
'Please install the stackprof gem and require it: add gem \'stackprof\' to your Gemfile'
132132
)
133-
expect(last_response.headers['Content-Type']).to eq('text/plain; charset=utf-8')
133+
expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain; charset=utf-8')
134134
expect(last_response.headers['X-CUSTOM']).to eq('1')
135135
expect(last_response.status).to eq(201)
136136
end
@@ -142,7 +142,7 @@ def app
142142
expect(last_response.body).to eq(
143143
'Please install the stackprof gem and require it: add gem \'stackprof\' to your Gemfile'
144144
)
145-
expect(last_response.headers['Content-Type']).to eq('text/plain; charset=utf-8')
145+
expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain; charset=utf-8')
146146
expect(last_response.headers['X-CUSTOM']).to eq('1')
147147
expect(last_response.status).to eq(201)
148148
end
@@ -154,7 +154,7 @@ def app
154154
Rack::Builder.new do
155155
use Rack::MiniProfiler
156156
use Rack::Deflater
157-
run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
157+
run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
158158
end
159159
end
160160

@@ -190,7 +190,7 @@ def app
190190
Rack::Builder.new do
191191
use Rack::Deflater
192192
use Rack::MiniProfiler
193-
run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
193+
run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
194194
end
195195
end
196196

@@ -223,7 +223,7 @@ def app
223223
def app
224224
Rack::Builder.new do
225225
use Rack::MiniProfiler
226-
run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
226+
run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, [+'<html><body><h1>Hi</h1></body></html>']] }
227227
end
228228
end
229229

@@ -249,7 +249,7 @@ def app
249249
use Rack::MiniProfiler
250250
run lambda { |env|
251251
env["action_dispatch.content_security_policy_nonce"] = "railsnonce"
252-
[200, { 'Content-Type' => 'text/html' }, [+'<html><body><h1>Hello world</h1></body></html>']]
252+
[200, { Rack::CONTENT_TYPE => 'text/html' }, [+'<html><body><h1>Hello world</h1></body></html>']]
253253
}
254254
end
255255
end
@@ -278,7 +278,7 @@ def app
278278

279279
(env, response_headers) = proc_arguments
280280
expect(env["REQUEST_METHOD"]).to eq("GET")
281-
expect(response_headers["Content-Type"]).to eq("text/html")
281+
expect(response_headers[Rack::CONTENT_TYPE]).to eq("text/html")
282282
end
283283
end
284284
end

‎spec/integration/mini_profiler_spec.rb‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,88 +10,88 @@ def app
1010
@app ||= Rack::Builder.new {
1111
use Rack::MiniProfiler
1212
map '/path2/a' do
13-
run lambda { |env| [200, { 'Content-Type' => 'text/html' }, +'<h1>path1</h1>'] }
13+
run lambda { |env| [200, { Rack::CONTENT_TYPE => 'text/html' }, +'<h1>path1</h1>'] }
1414
end
1515
map '/path1/a' do
16-
run lambda { |env| [200, { 'Content-Type' => 'text/html' }, +'<h1>path2</h1>'] }
16+
run lambda { |env| [200, { Rack::CONTENT_TYPE => 'text/html' }, +'<h1>path2</h1>'] }
1717
end
1818
map '/cached-resource' do
1919
run lambda { |env|
2020
ims = env['HTTP_IF_MODIFIED_SINCE'] || ""
2121
if ims.size > 0
22-
[304, { 'Content-Type' => 'application/json' }, '']
22+
[304, { Rack::CONTENT_TYPE => 'application/json' }, '']
2323
else
24-
[200, { 'Content-Type' => 'application/json', 'Cache-Control' => 'original-cache-control' }, '{"name": "Ryan"}']
24+
[200, { Rack::CONTENT_TYPE => 'application/json', Rack::CACHE_CONTROL => 'original-cache-control' }, '{"name": "Ryan"}']
2525
end
2626
}
2727
end
2828
map '/post' do
29-
run lambda { |env| [302, { 'Content-Type' => 'text/html' }, +'<h1>POST</h1>'] }
29+
run lambda { |env| [302, { Rack::CONTENT_TYPE => 'text/html' }, +'<h1>POST</h1>'] }
3030
end
3131
map '/html' do
32-
run lambda { |env| [200, { 'Content-Type' => 'text/html' }, +"<html><BODY><h1>Hi</h1></BODY>\n \t</html>"] }
32+
run lambda { |env| [200, { Rack::CONTENT_TYPE => 'text/html' }, +"<html><BODY><h1>Hi</h1></BODY>\n \t</html>"] }
3333
end
3434
map '/explicitly-allowed-html' do
3535
run lambda { |env|
3636
Rack::MiniProfiler.authorize_request
37-
[200, { 'Content-Type' => 'text/html' }, +"<html><BODY><h1>Hi</h1></BODY>\n \t</html>"]
37+
[200, { Rack::CONTENT_TYPE => 'text/html' }, +"<html><BODY><h1>Hi</h1></BODY>\n \t</html>"]
3838
}
3939
end
4040
map '/implicitbody' do
41-
run lambda { |env| [200, { 'Content-Type' => 'text/html' }, +"<html><h1>Hi</h1></html>"] }
41+
run lambda { |env| [200, { Rack::CONTENT_TYPE => 'text/html' }, +"<html><h1>Hi</h1></html>"] }
4242
end
4343
map '/implicitbodyhtml' do
44-
run lambda { |env| [200, { 'Content-Type' => 'text/html' }, +"<h1>Hi</h1>"] }
44+
run lambda { |env| [200, { Rack::CONTENT_TYPE => 'text/html' }, +"<h1>Hi</h1>"] }
4545
end
4646
map '/db' do
4747
run lambda { |env|
4848
::Rack::MiniProfiler.record_sql("I want to be, in a db", 10)
49-
[200, { 'Content-Type' => 'text/html' }, +'<h1>Hi+db</h1>']
49+
[200, { Rack::CONTENT_TYPE => 'text/html' }, +'<h1>Hi+db</h1>']
5050
}
5151
end
5252
map '/3ms' do
5353
run lambda { |env|
5454
sleep(0.003)
55-
[200, { 'Content-Type' => 'text/html' }, +'<h1>Hi</h1>']
55+
[200, { Rack::CONTENT_TYPE => 'text/html' }, +'<h1>Hi</h1>']
5656
}
5757
end
5858
map '/explicitly-allowed' do
5959
run lambda { |env|
6060
Rack::MiniProfiler.authorize_request
61-
[200, { 'Content-Type' => 'text/html' }, +'<h1>path1</h1>']
61+
[200, { Rack::CONTENT_TYPE => 'text/html' }, +'<h1>path1</h1>']
6262
}
6363
end
6464
map '/rails_engine' do
6565
run lambda { |env|
6666
env['SCRIPT_NAME'] = '/rails_engine' # Rails engines do that
67-
[200, { 'Content-Type' => 'text/html' }, +'<html><h1>Hi</h1></html>']
67+
[200, { Rack::CONTENT_TYPE => 'text/html' }, +'<html><h1>Hi</h1></html>']
6868
}
6969
end
7070
map '/under_passenger' do
7171
run lambda { |env|
72-
[200, { 'Content-Type' => 'text/html' }, +'<html><h1>and I ride and I ride</h1></html>']
72+
[200, { Rack::CONTENT_TYPE => 'text/html' }, +'<html><h1>and I ride and I ride</h1></html>']
7373
}
7474
end
7575
map '/create' do
7676
run lambda { |env|
77-
[201, { 'Content-Type' => 'text/html' }, +'<html><h1>success</h1></html>']
77+
[201, { Rack::CONTENT_TYPE => 'text/html' }, +'<html><h1>success</h1></html>']
7878
}
7979
end
8080
map '/notallowed' do
8181
run lambda { |env|
82-
[403, { 'Content-Type' => 'text/html' }, +'<html><h1>you are not allowed here</h1></html>']
82+
[403, { Rack::CONTENT_TYPE => 'text/html' }, +'<html><h1>you are not allowed here</h1></html>']
8383
}
8484
end
8585
map '/whoopsie-daisy' do
8686
run lambda { |env|
87-
[500, { 'Content-Type' => 'text/html' }, +'<html><h1>whoopsie daisy</h1></html>']
87+
[500, { Rack::CONTENT_TYPE => 'text/html' }, +'<html><h1>whoopsie daisy</h1></html>']
8888
}
8989
end
9090
map '/test-snapshots-custom-fields' do
9191
run lambda { |env|
9292
qp = Rack::Utils.parse_nested_query(env['QUERY_STRING'])
9393
qp.each { |k, v| Rack::MiniProfiler.add_snapshot_custom_field(k, v) }
94-
[200, { 'Content-Type' => 'text/html' }, +'<html><h1>custom fields</h1></html>']
94+
[200, { Rack::CONTENT_TYPE => 'text/html' }, +'<html><h1>custom fields</h1></html>']
9595
}
9696
end
9797
}.to_app
@@ -450,12 +450,12 @@ def load_prof(response)
450450
describe 'gc profiler' do
451451
it "should return a report" do
452452
get '/html?pp=profile-gc'
453-
expect(last_response['Content-Type']).to include('text/plain')
453+
expect(last_response[Rack::CONTENT_TYPE]).to include('text/plain')
454454
end
455455

456456
it "should return a report when an HTTP header is used" do
457457
get '/html', nil, { 'HTTP_X_RACK_MINI_PROFILER' => 'profile-gc' }
458-
expect(last_response['Content-Type']).to include('text/plain')
458+
expect(last_response[Rack::CONTENT_TYPE]).to include('text/plain')
459459
end
460460
end
461461

‎spec/integration/trace_exceptions_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def app
1414
@app ||= Rack::Builder.new {
1515
use Rack::MiniProfiler
1616
map '/no_exceptions' do
17-
run lambda { |_env| [200, { 'Content-Type' => 'text/html' }, '<h1>Success</h1>'] }
17+
run lambda { |_env| [200, { Rack::CONTENT_TYPE => 'text/html' }, '<h1>Success</h1>'] }
1818
end
1919
map '/raise_exceptions' do
2020
# This route raises 3 exceptions, catches them, and returns a successful response
@@ -34,7 +34,7 @@ def app
3434
rescue
3535
# Ignore the exception
3636
end
37-
[200, { 'Content-Type' => 'text/html' }, '<h1>Exception raised but success returned</h1>']
37+
[200, { Rack::CONTENT_TYPE => 'text/html' }, '<h1>Exception raised but success returned</h1>']
3838
}
3939
end
4040
}.to_app

‎spec/lib/client_settings_spec.rb‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,23 +44,23 @@
4444
@settings.disable_profiling = false
4545
hash = {}
4646
@settings.write!(hash)
47-
expect(hash["set-cookie"]).to include("path=/;")
47+
expect(hash[Rack::SET_COOKIE]).to include("path=/;")
4848
end
4949

5050
it 'should correctly set cookie with correct path' do
5151
Rack::MiniProfiler.config.cookie_path = '/test'
5252
@settings.disable_profiling = false
5353
hash = {}
5454
@settings.write!(hash)
55-
expect(hash["set-cookie"]).to include("path=/test;")
55+
expect(hash[Rack::SET_COOKIE]).to include("path=/test;")
5656
end
5757

5858
it 'writes auth token for authorized reqs' do
5959
Rack::MiniProfiler.config.authorization_mode = :allow_authorized
6060
Rack::MiniProfiler.authorize_request
6161
hash = {}
6262
@settings.write!(hash)
63-
expect(hash["set-cookie"]).to include(@store.allowed_tokens.join("|"))
63+
expect(hash[Rack::SET_COOKIE]).to include(@store.allowed_tokens.join("|"))
6464
end
6565

6666
it 'does nothing on short unauthed requests' do
@@ -80,7 +80,7 @@
8080
@settings.handle_cookie([200, hash, []])
8181
end
8282

83-
expect(hash["set-cookie"]).to include("max-age=0")
83+
expect(hash[Rack::SET_COOKIE]).to include("max-age=0")
8484
end
8585
end
8686

0 commit comments

Comments
 (0)