From 7d0ace06f603ff5962a989ccf0239b6dab0745ba Mon Sep 17 00:00:00 2001 From: ishabi Date: Mon, 7 Oct 2024 14:01:10 +0200 Subject: [PATCH] use route path instead of url --- .../src/appsec/api_security_sampler.js | 2 +- packages/dd-trace/src/appsec/index.js | 2 +- .../test/appsec/api_security_sampler.spec.js | 39 ++++++++++++++++--- packages/dd-trace/test/appsec/index.spec.js | 4 +- 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/packages/dd-trace/src/appsec/api_security_sampler.js b/packages/dd-trace/src/appsec/api_security_sampler.js index d53bd1d85a..e8f1d834ed 100644 --- 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 index 56b645dbcb..8655b76728 100644 --- 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 index f2d0b0da7f..11e5bc6173 100644 --- 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 index 13f6b197f0..18937c6db3 100644 --- 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'