use route path instead of url

DataDog · Oct 7, 2024 · 7d0ace0 · 7d0ace0
1 parent 200a177
commit 7d0ace0
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 8 deletions.
diff --git a/packages/dd-trace/src/appsec/api_security_sampler.js b/packages/dd-trace/src/appsec/api_security_sampler.js
@@ -53,7 +53,7 @@ function isSampled (req, res) {
 }
 
 function computeKey (req, res) {
-  const route = req.url
+  const route = req.route.path
   const method = req.method.toLowerCase()
   const statusCode = res.statusCode
   const str = route + statusCode + method

diff --git a/packages/dd-trace/src/appsec/index.js b/packages/dd-trace/src/appsec/index.js
@@ -132,7 +132,7 @@ function incomingHttpEndTranslator ({ req, res }) {
     persistent[addresses.HTTP_INCOMING_QUERY] = req.query
   }
 
-  if (apiSecuritySampler.sampleRequest(req, res)) {
+  if (req.route && typeof req.route.path === 'string' && apiSecuritySampler.sampleRequest(req, res)) {
     persistent[addresses.WAF_CONTEXT_PROCESSOR] = { 'extract-schema': true }
   }
 

diff --git a/packages/dd-trace/test/appsec/api_security_sampler.spec.js b/packages/dd-trace/test/appsec/api_security_sampler.spec.js
@@ -4,7 +4,7 @@ const { performance } = require('node:perf_hooks')
 const proxyquire = require('proxyquire')
 
 describe('API Security Sampler', () => {
-  const req = { url: '/test', method: 'GET' }
+  const req = { route: { path: '/test' }, method: 'GET' }
   const res = { statusCode: 200 }
   let apiSecuritySampler, performanceNowStub, webStub, sampler, span
 
@@ -77,11 +77,10 @@ describe('API Security Sampler', () => {
   it('should remove oldest entry when max size is exceeded', () => {
     const method = req.method
     for (let i = 0; i < 4097; i++) {
-      expect(apiSecuritySampler.sampleRequest({ method, url: `/test${i}` }, res)).to.be.true
+      expect(apiSecuritySampler.sampleRequest({ method, route: { path: `/test${i}` } }, res)).to.be.true
     }
-
-    expect(apiSecuritySampler.isSampled({ method, url: '/test0' }, res)).to.be.false
-    expect(apiSecuritySampler.isSampled({ method, url: '/test4096' }, res)).to.be.true
+    expect(apiSecuritySampler.isSampled({ method, route: { path: '/test0' } }, res)).to.be.false
+    expect(apiSecuritySampler.isSampled({ method, route: { path: '/test4096' } }, res)).to.be.true
   })
 
   it('should set enabled to false and clear the cache', () => {
@@ -91,4 +90,34 @@ describe('API Security Sampler', () => {
 
     expect(apiSecuritySampler.sampleRequest(req, res)).to.be.false
   })
+
+  it('should create different keys for different URLs', () => {
+    const req1 = { route: { path: '/test1' }, method: 'GET' }
+    const req2 = { route: { path: '/test2' }, method: 'GET' }
+
+    expect(apiSecuritySampler.sampleRequest(req1, res)).to.be.true
+    expect(apiSecuritySampler.sampleRequest(req2, res)).to.be.true
+    expect(apiSecuritySampler.isSampled(req1, res)).to.be.true
+    expect(apiSecuritySampler.isSampled(req2, res)).to.be.true
+  })
+
+  it('should create different keys for different methods', () => {
+    const getReq = { route: { path: '/test1' }, method: 'GET' }
+    const postReq = { route: { path: '/test1' }, method: 'POST' }
+
+    expect(apiSecuritySampler.sampleRequest(getReq, res)).to.be.true
+    expect(apiSecuritySampler.sampleRequest(postReq, res)).to.be.true
+    expect(apiSecuritySampler.isSampled(getReq, res)).to.be.true
+    expect(apiSecuritySampler.isSampled(postReq, res)).to.be.true
+  })
+
+  it('should create different keys for different status codes', () => {
+    const res200 = { statusCode: 200 }
+    const res404 = { statusCode: 404 }
+
+    expect(apiSecuritySampler.sampleRequest(req, res200)).to.be.true
+    expect(apiSecuritySampler.sampleRequest(req, res404)).to.be.true
+    expect(apiSecuritySampler.isSampled(req, res200)).to.be.true
+    expect(apiSecuritySampler.isSampled(req, res404)).to.be.true
+  })
 })
diff --git a/packages/dd-trace/test/appsec/index.spec.js b/packages/dd-trace/test/appsec/index.spec.js
@@ -533,7 +533,9 @@ describe('AppSec Index', function () {
       AppSec.enable(config)
 
       const req = {
-        url: '/path',
+        route: {
+          path: '/path'
+        },
         headers: {
           'user-agent': 'Arachni',
           host: 'localhost'