Allow blocking on fastify path params #5889

IlyasShabi · 2025-06-12T15:10:39Z

What does this PR do?

Allow blocking on fastify path params

Motivation

Plugin Checklist

Additional Notes

github-actions · 2025-06-12T15:11:23Z

Overall package size

Self size: 9.67 MB
Deduped: 106.17 MB
No deduping: 106.69 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 8.5.2 | 19.33 MB | 19.34 MB | | @datadog/native-iast-taint-tracking | 4.0.0 | 11.72 MB | 11.73 MB | | @datadog/pprof | 5.8.2 | 9.56 MB | 9.93 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.3 | 2.95 MB | 5.6 MB | | @datadog/wasm-js-rewriter | 4.0.1 | 2.85 MB | 3.58 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.14.0 | 120.58 kB | 841.68 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.2 | 53.63 kB | 53.63 kB | | istanbul-lib-coverage | 3.2.2 | 34.37 kB | 34.37 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | dc-polyfill | 0.1.9 | 25.11 kB | 25.11 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.2 | 23.54 kB | 23.54 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | mutexify | 1.4.0 | 5.71 kB | 8.74 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.4 | 3.96 kB | 3.96 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

codecov · 2025-06-12T15:11:41Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.75%. Comparing base (0b3ab0c) to head (528f1c3).
Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5889      +/-   ##
==========================================
- Coverage   81.62%   80.75%   -0.88%     
==========================================
  Files         455      462       +7     
  Lines       19589    19907     +318     
==========================================
+ Hits        15990    16075      +85     
- Misses       3599     3832     +233

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

datadog-datadog-prod-us1 · 2025-06-12T15:19:58Z

Datadog Report

Branch report: ishabi/fastify-path-params
Commit report: 81438b8
Test service: dd-trace-js-integration-tests

✅ 0 Failed, 1257 Passed, 0 Skipped, 20m 44.5s Total Time

pr-commenter · 2025-06-12T15:20:24Z

Benchmarks

Benchmark execution time: 2025-06-16 08:40:22

Comparing candidate commit 528f1c3 in PR branch ishabi/fastify-path-params with baseline commit 0b3ab0c in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1274 metrics, 49 unstable metrics.

packages/dd-trace/test/appsec/index.fastify.plugin.spec.js

uurien · 2025-06-13T06:34:54Z

packages/dd-trace/test/appsec/index.fastify.plugin.spec.js

+      })
+
+      app.register(async function (nested) {
+        nested.get('/:nestedDuplicatedParameter', async (request, reply) => {


The reason why we have this nestedDuplicatedParameter in express test, is that we have two parameters with the same name in different routes.
Here, we don't have any other parameter called nestedDuplicatedParameter, so the property name is wrong, and should be something like secondParam, or in the prefix, it should be also nestedDuplicatedParameter.
Maybe this example does make no sense in fastify.

uurien · 2025-06-13T06:35:43Z

packages/dd-trace/test/appsec/index.fastify.plugin.spec.js

+
+      app.addHook('preHandler', paramHookSpy)
+
+      app.get('/callback-path-param/:callbackedParameter', (request, reply) => {


Same for this test, in express tests, :callbackedParameter is called as it is, because we are listening for app.param('callbackedParameter'...). Does it make sense here?

Parameter name doesn't make sense here because in Fastify we have a global hook that runs for all routes using app.addHook('preHandler', paramHookSpy)

BridgeAR · 2025-06-14T10:56:17Z

packages/dd-trace/test/appsec/index.fastify.plugin.spec.js

+        try {
+          await axios.get('/multiple-path-params/testattack/testattack')
+
+          return Promise.reject(new Error('Request should not return 200'))
+        } catch (e) {
+          assert.equal(e.response.status, 403)
+          assert.deepEqual(e.response.data, JSON.parse(json))
+        }


Instead of using try / catch, we could switch to the native assert and use assert.rejects. That provides more information and is IMO easier to reason about.

BridgeAR · 2025-06-14T10:57:56Z

packages/datadog-instrumentations/src/fastify.js

@@ -133,6 +134,17 @@ function preValidation (request, reply, done) {
      if (abortController.signal.aborted) return
    }

+    if (pathParamsReadCh.hasSubscribers && request.params) {
+      const abortController = new AbortController()


Creating new AbortControllers is quite expensive. Could we just reuse an already defined controller?
I believe it's possible to unify the up to three controllers to a single one.

simon-id · 2025-06-15T12:14:34Z

packages/datadog-instrumentations/src/fastify.js

+    if (pathParamsReadCh.hasSubscribers && request.params) {
+      const abortController = new AbortController()
+
+      pathParamsReadCh.publish({
+        req,
+        res,
+        abortController,
+        params: request.params
+      })
+    }


uhm where is the blocking ?

simon-id · 2025-06-16T09:27:32Z

packages/datadog-instrumentations/src/fastify.js

+      pathParamsReadCh.publish({
+        req,
+        res,
+        abortController,


if you do this for the 3 different calls, you don't create it until it's needed, but you also cache it for the others. Wdyt ?

Suggested change

abortController,

abortController: abortController ?? abortController = new AbortController(),

simon-id · 2025-06-16T09:30:58Z

packages/dd-trace/test/appsec/index.fastify.plugin.spec.js

@@ -264,7 +264,11 @@ withVersions('fastify', 'fastify', version => {
  })

  describe('Suspicious request blocking - path parameters', () => {
-    let server, paramHookSpy, axios
+    // Skip Fastify v1 - preValidation hook is not supported
+    if (semver.lt(semver.coerce(version), '2.0.0')) {


if we don't really support blocking before 2.0.0 we should consider appsec only support fastify starting from v2 and add that to the relevant doc

IlyasShabi added 2 commits June 11, 2025 11:32

fastify path params support

25600e0

Merge branch 'master' into ishabi/fastify-path-params

13783bd

IlyasShabi added semver-minor appsec labels Jun 12, 2025

IlyasShabi changed the title ~~Ishabi/fastify path params~~ Allow blocking on fastify path params Jun 12, 2025

Merge branch 'master' into ishabi/fastify-path-params

f072775

IlyasShabi force-pushed the ishabi/fastify-path-params branch from af324cd to f072775 Compare June 12, 2025 15:54

IlyasShabi marked this pull request as ready for review June 12, 2025 16:21

IlyasShabi requested review from a team as code owners June 12, 2025 16:21

uurien reviewed Jun 13, 2025

View reviewed changes

IlyasShabi added 2 commits June 13, 2025 10:23

change params names

50afa93

linter

906f727

IlyasShabi requested a review from uurien June 13, 2025 09:04

BridgeAR reviewed Jun 14, 2025

View reviewed changes

simon-id reviewed Jun 15, 2025

View reviewed changes

assert blocking

528f1c3

simon-id reviewed Jun 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow blocking on fastify path params #5889

Allow blocking on fastify path params #5889

IlyasShabi commented Jun 12, 2025

Uh oh!

github-actions bot commented Jun 12, 2025 •

edited

Loading

Uh oh!

codecov bot commented Jun 12, 2025 •

edited

Loading

Uh oh!

datadog-datadog-prod-us1 bot commented Jun 12, 2025 •

edited

Loading

Uh oh!

pr-commenter bot commented Jun 12, 2025 •

edited

Loading

Uh oh!

Uh oh!

uurien Jun 13, 2025

Uh oh!

uurien Jun 13, 2025

Uh oh!

IlyasShabi Jun 13, 2025

Uh oh!

BridgeAR Jun 14, 2025

Uh oh!

BridgeAR Jun 14, 2025

Uh oh!

simon-id Jun 15, 2025

Uh oh!

simon-id Jun 16, 2025

Uh oh!

simon-id Jun 16, 2025

Uh oh!

Uh oh!


		app.addHook('preHandler', paramHookSpy)

		app.get('/callback-path-param/:callbackedParameter', (request, reply) => {

	abortController,
	abortController: abortController ?? abortController = new AbortController(),

Allow blocking on fastify path params #5889

Are you sure you want to change the base?

Allow blocking on fastify path params #5889

Conversation

IlyasShabi commented Jun 12, 2025

What does this PR do?

Motivation

Plugin Checklist

Additional Notes

Uh oh!

github-actions bot commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overall package size

Uh oh!

codecov bot commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

datadog-datadog-prod-us1 bot commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Datadog Report

Uh oh!

pr-commenter bot commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jun 12, 2025 •

edited

Loading

codecov bot commented Jun 12, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Jun 12, 2025 •

edited

Loading

pr-commenter bot commented Jun 12, 2025 •

edited

Loading