Commit aff2b6e4 authored by Stan Hu's avatar Stan Hu

Switch use of Rack::Request to ActionDispatch::Request

As mentioned in
https://gitlab.com/gitlab-org/gitlab-ee/issues/9035#note_129093444,
Rails 5 switched ActionDispatch::Request so that it no longer inherits
Rack::Request directly. A middleware that uses Rack::Request to
read the environment may see stale request parameters if
another middleware modifies the environment via ActionDispatch::Request.
To be safe, we should be using ActionDispatch::Request everywhere.
parent b83be503
...@@ -512,7 +512,7 @@ module API ...@@ -512,7 +512,7 @@ module API
# `request`. We workaround this by defining methods that returns the right # `request`. We workaround this by defining methods that returns the right
# values. # values.
def define_params_for_grape_middleware def define_params_for_grape_middleware
self.define_singleton_method(:request) { Rack::Request.new(env) } self.define_singleton_method(:request) { ActionDispatch::Request.new(env) }
self.define_singleton_method(:params) { request.params.symbolize_keys } self.define_singleton_method(:params) { request.params.symbolize_keys }
end end
......
...@@ -8,7 +8,7 @@ module Gitlab ...@@ -8,7 +8,7 @@ module Gitlab
end end
def call(env) def call(env)
request = Rack::Request.new(env) request = ActionDispatch::Request.new(env)
route = Gitlab::EtagCaching::Router.match(request.path_info) route = Gitlab::EtagCaching::Router.match(request.path_info)
return @app.call(env) unless route return @app.call(env) unless route
......
...@@ -24,7 +24,7 @@ module Gitlab ...@@ -24,7 +24,7 @@ module Gitlab
def call(env) def call(env)
return @app.call(env) unless env['PATH_INFO'] == HEALTH_PATH return @app.call(env) unless env['PATH_INFO'] == HEALTH_PATH
request = Rack::Request.new(env) request = ActionDispatch::Request.new(env)
return OK_RESPONSE if client_ip_whitelisted?(request) return OK_RESPONSE if client_ip_whitelisted?(request)
......
...@@ -60,7 +60,7 @@ module Gitlab ...@@ -60,7 +60,7 @@ module Gitlab
end end
def request def request
@env['rack.request'] ||= Rack::Request.new(@env) @env['actionpack.request'] ||= ActionDispatch::Request.new(@env)
end end
def last_visited_url def last_visited_url
......
...@@ -13,7 +13,7 @@ module Gitlab ...@@ -13,7 +13,7 @@ module Gitlab
end end
def call(env) def call(env)
req = Rack::Request.new(env) req = ActionDispatch::Request.new(env)
Gitlab::SafeRequestStore[:client_ip] = req.ip Gitlab::SafeRequestStore[:client_ip] = req.ip
......
...@@ -9,7 +9,7 @@ describe Gitlab::Auth::UserAuthFinders do ...@@ -9,7 +9,7 @@ describe Gitlab::Auth::UserAuthFinders do
'rack.input' => '' 'rack.input' => ''
} }
end end
let(:request) { Rack::Request.new(env) } let(:request) { ActionDispatch::Request.new(env) }
def set_param(key, value) def set_param(key, value)
request.update_param(key, value) request.update_param(key, value)
......
...@@ -15,7 +15,7 @@ describe Gitlab::RequestContext do ...@@ -15,7 +15,7 @@ describe Gitlab::RequestContext do
let(:ip) { '192.168.1.11' } let(:ip) { '192.168.1.11' }
before do before do
allow_any_instance_of(Rack::Request).to receive(:ip).and_return(ip) allow_any_instance_of(ActionDispatch::Request).to receive(:ip).and_return(ip)
described_class.new(app).call(env) described_class.new(app).call(env)
end end
......
...@@ -25,6 +25,8 @@ describe OmniAuth::Strategies::Jwt do ...@@ -25,6 +25,8 @@ describe OmniAuth::Strategies::Jwt do
subject.options[:secret] = secret subject.options[:secret] = secret
subject.options[:algorithm] = algorithm subject.options[:algorithm] = algorithm
# We use Rack::Request instead of ActionDispatch::Request because
# Rack::Test::Methods enables testing of this module.
expect_next_instance_of(Rack::Request) do |rack_request| expect_next_instance_of(Rack::Request) do |rack_request|
expect(rack_request).to receive(:params).and_return('jwt' => payload) expect(rack_request).to receive(:params).and_return('jwt' => payload)
end end
......
...@@ -387,7 +387,7 @@ describe 'Git HTTP requests' do ...@@ -387,7 +387,7 @@ describe 'Git HTTP requests' do
it "responds with status 401" do it "responds with status 401" do
expect(Rack::Attack::Allow2Ban).to receive(:filter).and_return(true) expect(Rack::Attack::Allow2Ban).to receive(:filter).and_return(true)
allow_any_instance_of(Rack::Request).to receive(:ip).and_return('1.2.3.4') allow_any_instance_of(ActionDispatch::Request).to receive(:ip).and_return('1.2.3.4')
clone_get(path, env) clone_get(path, env)
...@@ -548,7 +548,7 @@ describe 'Git HTTP requests' do ...@@ -548,7 +548,7 @@ describe 'Git HTTP requests' do
maxretry = options[:maxretry] - 1 maxretry = options[:maxretry] - 1
ip = '1.2.3.4' ip = '1.2.3.4'
allow_any_instance_of(Rack::Request).to receive(:ip).and_return(ip) allow_any_instance_of(ActionDispatch::Request).to receive(:ip).and_return(ip)
Rack::Attack::Allow2Ban.reset(ip, options) Rack::Attack::Allow2Ban.reset(ip, options)
maxretry.times.each do maxretry.times.each do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment