AppSec: Fix `undefined method request_method=` #4870

marcotc · 2025-08-27T01:37:39Z

What does this PR do?

Motivation:

Change log entry

Additional Notes:

How to test the change?

github-actions · 2025-08-27T01:37:49Z

👋 Hey @marcotc, please fill "Change log entry" section in the pull request description.

If changes need to be present in CHANGELOG.md you can state it this way

**Change log entry**

Yes. A brief summary to be placed into the CHANGELOG.md

(possible answers Yes/Yep/Yeah)

Or you can opt out like that

**Change log entry**

None.

(possible answers No/Nope/None)

^{Visited at: 2025-08-27 01:37:49 UTC}

datadog-official · 2025-08-27T01:55:07Z

⚠️ Tests

⚠️ Warnings

❄️ 5 New flaky tests detected

Rails integration tests for an application with a basic route GET request with an event-triggering request in headers behaves like a trace with AppSec api security tags with api security enabled is expected not to be empty

from rspec (Datadog)

expected \`{}.empty?\` to be falsey, got true

Failure/Error: expect(api_security_tags).to_not be_empty
  expected \`{}.empty?\` to be falsey, got true
Shared Example Group: "a trace with AppSec api security tags" called from ./spec/datadog/appsec/contrib/rails/integration_test_spec.rb:261
./spec/datadog/appsec/contrib/support/integration/shared_examples.rb:135:in \`block (3 levels) in <top (required)>'
./spec/datadog/tracing/contrib/rails/support/base.rb:65:in \`block (3 levels) in <top (required)>'
/usr/local/bundle/gems/climate_control-1.2.0/lib/climate_control.rb:24:in \`block in modify'
/usr/local/bundle/gems/climate_control-1.2.0/lib/climate_control.rb:15:in \`modify'
./spec/datadog/tracing/contrib/rails/support/base.rb:64:in \`block (2 levels) in <top (required)>'
...

Rails integration tests for an application with a basic route GET request with an event-triggering request in IP behaves like a trace with AppSec api security tags with api security enabled is expected not to be empty

from rspec (Datadog)

expected \`{}.empty?\` to be falsey, got true

Failure/Error: expect(api_security_tags).to_not be_empty
  expected \`{}.empty?\` to be falsey, got true
Shared Example Group: "a trace with AppSec api security tags" called from ./spec/datadog/appsec/contrib/rails/integration_test_spec.rb:329
./spec/datadog/appsec/contrib/support/integration/shared_examples.rb:135:in \`block (3 levels) in <top (required)>'
./spec/datadog/tracing/contrib/rails/support/base.rb:65:in \`block (3 levels) in <top (required)>'
/usr/local/bundle/gems/climate_control-1.2.0/lib/climate_control.rb:24:in \`block in modify'
/usr/local/bundle/gems/climate_control-1.2.0/lib/climate_control.rb:15:in \`modify'
./spec/datadog/tracing/contrib/rails/support/base.rb:64:in \`block (2 levels) in <top (required)>'
...

Rails integration tests for an application with a basic route GET request with an event-triggering request in query string and a blocking rule behaves like a trace with AppSec api security tags with api security enabled is expected not to be empty

from rspec (Datadog)

expected \`{}.empty?\` to be falsey, got true

Failure/Error: expect(api_security_tags).to_not be_empty
  expected \`{}.empty?\` to be falsey, got true
Shared Example Group: "a trace with AppSec api security tags" called from ./spec/datadog/appsec/contrib/rails/integration_test_spec.rb:284
./spec/datadog/appsec/contrib/support/integration/shared_examples.rb:135:in \`block (3 levels) in <top (required)>'
./spec/datadog/tracing/contrib/rails/support/base.rb:65:in \`block (3 levels) in <top (required)>'
/usr/local/bundle/gems/climate_control-1.2.0/lib/climate_control.rb:24:in \`block in modify'
/usr/local/bundle/gems/climate_control-1.2.0/lib/climate_control.rb:15:in \`synchronize'
/usr/local/bundle/gems/climate_control-1.2.0/lib/climate_control.rb:15:in \`modify'
...

View all

ℹ️ Info

🧪 All tests passed

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 69b1fa1 | Docs | Was this helpful? Give us feedback!}

pr-commenter · 2025-08-27T02:07:41Z

Benchmarks

Benchmark execution time: 2025-08-27 02:07:16

Comparing candidate commit 69b1fa1 in PR branch fix-appsec-rack with baseline commit 319db66 in branch master.

Found 2 performance improvements and 2 performance regressions! Performance is the same for 40 metrics, 2 unstable metrics.

scenario:line instrumentation - targeted

🟥 throughput [-11517.037op/s; -10907.233op/s] or [-6.883%; -6.518%]

scenario:method instrumentation

🟥 throughput [-10556.629op/s; -10069.172op/s] or [-5.941%; -5.666%]

scenario:profiling - stack collector (ruby frames - native filenames enabled)

🟩 throughput [+492.710op/s; +494.619op/s] or [+16.903%; +16.968%]

scenario:tracing - Tracing.log_correlation

🟩 throughput [+8986.753op/s; +9280.936op/s] or [+8.883%; +9.174%]

Strech · 2025-09-01T09:50:35Z

lib/datadog/appsec/api_security/route_extractor.rb

-            pattern = request.env[RAILS_ROUTES_KEY].router
-              .recognize(request) { |route, _| break route.path.spec.to_s }
+            route_set = request.env[RAILS_ROUTES_KEY]
+            rails_request = route_set.request_class.new(request.env)


Hey @marcotc, thanks for taking care of that. We don't have grape contrib, but there was a plan to start supporting it as main API framework.

The init of the router will generate new instance with unprocessed env, could you please explain what is the actual issue here of using stored router?

The main issue is that recognize(request) expects a wrapped ActionPack request, not a raw Rack request, and it throws the error undefined method request_method= when running.

Thankfully this is easy to reproduce in tests (which are being added).

Tbh, I only really have to fix the Rails issue today, but I noticed that the Grape and Sinatra paths were not tested in integration tests (which the the only way to trigger the Rails failure today), so I started adding them in this PR as well.

Still WIP though!

🤔 Hmmm, we have tested it with Rails and we decide to scope out Sinatra to system tests, but both working. In which case it do you get that error?

If it happens solely in tests like an old "integration" test we poses, than I would challenge the quality of the test.

We also have a test app where I run it across different Rails app versions.

BTW, here are integrations tests that will use route extraction (but not on all Rails versions)

If you don't mind could you share the error case you are fixing? I would love to help on this.

I think I get the issue now, problem starts not because sampling window, but because the HEAD request. It will take a path into the router which will assign the method into it to be able to find it across the available routes.

Rails will try to modify the request in order to find it.

As a suggestion, to avoid this expensive path, could we wrap the request only for this specific scenario of HEAD request then?

AppSec: Fix undefined method request_method=

69b1fa1

github-actions bot added the appsec Application Security monitoring product label Aug 27, 2025

Strech reviewed Sep 1, 2025

View reviewed changes

Strech mentioned this pull request Sep 10, 2025

Fix error on HEAD requests route extraction #4887

Merged

Strech closed this in #4887 Sep 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

AppSec: Fix `undefined method request_method=` #4870

AppSec: Fix `undefined method request_method=` #4870

Uh oh!

marcotc commented Aug 27, 2025

Uh oh!

github-actions bot commented Aug 27, 2025

Uh oh!

datadog-official bot commented Aug 27, 2025 •

edited

Loading

Uh oh!

pr-commenter bot commented Aug 27, 2025

Uh oh!

Strech Sep 1, 2025

Uh oh!

marcotc Sep 2, 2025

Uh oh!

Strech Sep 3, 2025

Uh oh!

Strech Sep 3, 2025 •

edited

Loading

Uh oh!

Strech Sep 4, 2025

Uh oh!

Uh oh!

AppSec: Fix undefined method request_method= #4870

AppSec: Fix undefined method request_method= #4870

Uh oh!

Conversation

marcotc commented Aug 27, 2025

Uh oh!

github-actions bot commented Aug 27, 2025

Uh oh!

datadog-official bot commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Warnings

ℹ️ Info

Uh oh!

pr-commenter bot commented Aug 27, 2025

Benchmarks

scenario:line instrumentation - targeted

scenario:method instrumentation

scenario:profiling - stack collector (ruby frames - native filenames enabled)

scenario:tracing - Tracing.log_correlation

Uh oh!

Strech Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

marcotc Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Strech Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Strech Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Strech Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AppSec: Fix `undefined method request_method=` #4870

AppSec: Fix `undefined method request_method=` #4870

datadog-official bot commented Aug 27, 2025 •

edited

Loading

Strech Sep 3, 2025 •

edited

Loading