Skip to content

Commit 2fc6c6c

Browse files
FlinkFlink
authored
Ensure compatibility with Rack 2.x and 3.x (#653)
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 2fc6c6c

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)