Skip to content

Commit 492622c

Browse files
authored
Merge pull request #4887 from DataDog/appsec-58855-fix-head-requests-route-extraction
Fix error on HEAD requests route extraction
2 parents b054770 + a21436e commit 492622c

File tree

3 files changed

+50
-3
lines changed

3 files changed

+50
-3
lines changed

lib/datadog/appsec/api_security/route_extractor.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ module RouteExtractor
3939
# WARNING: This method works only *after* the request has been routed.
4040
#
4141
# WARNING: In Rails > 7.1 when a route was not found,
42-
# action_dispatch.route_uri_pattern will not be set.
42+
# `action_dispatch.route_uri_pattern` will not be set.
4343
# In Rails < 7.1 it also will not be set even if a route was found,
44-
# but in this case action_dispatch.request.path_parameters won't be empty.
44+
# but in this case `action_dispatch.request.path_parameters` won't be empty.
4545
def self.route_pattern(request)
4646
if request.env.key?(GRAPE_ROUTE_KEY)
4747
pattern = request.env[GRAPE_ROUTE_KEY][:route_info]&.pattern&.origin
@@ -52,6 +52,10 @@ def self.route_pattern(request)
5252
elsif request.env.key?(RAILS_ROUTE_KEY)
5353
request.env[RAILS_ROUTE_KEY].delete_suffix(RAILS_FORMAT_SUFFIX)
5454
elsif request.env.key?(RAILS_ROUTES_KEY) && !request.env.fetch(RAILS_PATH_PARAMS_KEY, {}).empty?
55+
# NOTE: Rails mutate HEAD request in order to understand that route is supported.
56+
# It will assing GET request method and run the route recognition.
57+
request = request.env[RAILS_ROUTES_KEY].request_class.new(request.env) if request.head?
58+
5559
pattern = request.env[RAILS_ROUTES_KEY].router
5660
.recognize(request) { |route, _| break route.path.spec.to_s }
5761

sig/datadog/appsec/api_security.rbs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ module Datadog
66
def script_name: () -> String?
77
def request_method: () -> String
88
def path: () -> String
9+
def head?: () -> bool
910
end
1011

1112
interface _Response

spec/datadog/appsec/api_security/route_extractor_spec.rb

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@
9292
it { expect(described_class.route_pattern(request)).to eq('/api/v1/users/:id/posts/:post_id') }
9393
end
9494

95-
context 'when route_uri_pattern is not set, but request path parameters are present' do
95+
context 'when route_uri_pattern is not set and request path_parameters is empty' do
9696
before do
9797
allow(request).to receive(:env).and_return({
9898
'action_dispatch.routes' => route_set,
@@ -107,6 +107,48 @@
107107
it { expect(described_class.route_pattern(request)).to eq('/users/1') }
108108
end
109109

110+
context 'when route_uri_pattern is not set and request path_parameters is present' do
111+
let(:env) do
112+
{
113+
'action_dispatch.routes' => route_set,
114+
'action_dispatch.request.path_parameters' => {
115+
'controller' => 'users', 'action' => 'show', 'id' => '1'
116+
}
117+
}
118+
end
119+
120+
context 'when request is HEAD' do
121+
before do
122+
allow(request).to receive(:env).and_return(env)
123+
allow(action_dispatch_request).to receive(:env).and_return(env)
124+
end
125+
126+
let(:router) { double('ActionDispatch::Routing::RouteSet::Router') }
127+
let(:route_set) { double('ActionDispatch::Routing::RouteSet', router: router, request_class: action_dispatch_request_class) }
128+
let(:request) { double('Rack::Request', env: {}, script_name: '', path: '/users/1', head?: true) }
129+
let(:action_dispatch_request_class) { double('class ActionDispatch::Request', new: action_dispatch_request) }
130+
let(:action_dispatch_request) { double('ActionDispatch::Request', env: {}, script_name: '', path: '/users/1') }
131+
132+
it 'uses action dispatch request for route recognition' do
133+
expect(router).to receive(:recognize).with(action_dispatch_request).and_return('/users/1')
134+
expect(described_class.route_pattern(request)).to eq('/users/1')
135+
end
136+
end
137+
138+
context 'when request is not HEAD' do
139+
before { allow(request).to receive(:env).and_return(env) }
140+
141+
let(:router) { double('ActionDispatch::Routing::RouteSet::Router') }
142+
let(:route_set) { double('ActionDispatch::Routing::RouteSet', router: router) }
143+
let(:request) { double('Rack::Request', env: {}, script_name: '', path: '/users/1', head?: false) }
144+
145+
it 'uses action dispatch request for route recognition' do
146+
expect(router).to receive(:recognize).with(request).and_return('/users/1')
147+
expect(described_class.route_pattern(request)).to eq('/users/1')
148+
end
149+
end
150+
end
151+
110152
context 'when Rails router cannot recognize request' do
111153
before do
112154
allow(request).to receive(:env).and_return({'action_dispatch.routes' => route_set})

0 commit comments

Comments
 (0)