Skip to content

Commit 6796afd

Browse files
authored
Get response headers on Fastify by the end of the request (#5831)
* set headers manually before calling writeHead * use writeHead dc instead * linter * get right fastify span * merge reply and request headers * remove get-port * fix tests * avoid unecessary object creation * avoid nullish storedResponseHeaders * linter
1 parent 7ecad19 commit 6796afd

File tree

5 files changed

+177
-8
lines changed

5 files changed

+177
-8
lines changed
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict'
2+
3+
const tracer = require('dd-trace')
4+
tracer.init({
5+
flushInterval: 0
6+
})
7+
8+
const fastify = require('fastify')
9+
const app = fastify()
10+
11+
app.get('/', (request, reply) => {
12+
reply.headers({
13+
'content-type': 'text/plain',
14+
'content-language': 'en-US',
15+
'content-length': '3'
16+
})
17+
return 'end'
18+
})
19+
20+
const port = process.env.APP_PORT ? Number(process.env.APP_PORT) : 0
21+
app.listen({ port }, (err) => {
22+
process.send?.({ port: app.server.address().port })
23+
})
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
'use strict'
2+
3+
const { assert } = require('chai')
4+
const path = require('path')
5+
const Axios = require('axios')
6+
7+
const {
8+
createSandbox,
9+
FakeAgent,
10+
spawnProc
11+
} = require('../helpers')
12+
13+
describe('Headers collection - Fastify', () => {
14+
let axios, sandbox, cwd, appFile, agent, proc
15+
16+
before(async () => {
17+
sandbox = await createSandbox(['fastify'])
18+
cwd = sandbox.folder
19+
appFile = path.join(cwd, 'appsec/data-collection/fastify.js')
20+
})
21+
22+
after(async () => {
23+
await sandbox.remove()
24+
})
25+
26+
beforeEach(async () => {
27+
agent = await new FakeAgent().start()
28+
29+
const env = {
30+
DD_TRACE_AGENT_PORT: agent.port,
31+
DD_APPSEC_ENABLED: true,
32+
DD_APPSEC_RULES: path.join(cwd, 'appsec/data-collection/data-collection-rules.json')
33+
}
34+
proc = await spawnProc(appFile, { cwd, env, execArgv: [] })
35+
axios = Axios.create({ baseURL: proc.url })
36+
})
37+
38+
afterEach(async () => {
39+
proc.kill()
40+
await agent.stop()
41+
})
42+
43+
it('should collect response headers with Fastify', async () => {
44+
let fastifyRequestReceived = false
45+
const requestHeaders = [
46+
'user-agent',
47+
'accept',
48+
'host',
49+
'accept-encoding'
50+
]
51+
52+
const responseHeaders = [
53+
'content-type',
54+
'content-language',
55+
'content-length'
56+
]
57+
58+
await axios.get('/', {
59+
headers: { 'User-Agent': 'Arachni/v1' }
60+
})
61+
62+
await agent.assertMessageReceived(({ headers, payload }) => {
63+
if (payload[0][0].name !== 'fastify.request') {
64+
throw new Error('Not the span we are looking for')
65+
}
66+
67+
fastifyRequestReceived = true
68+
assert.equal(
69+
Object.keys(payload[0][0].meta).filter(tagName => tagName.startsWith('http.request.headers.')).length,
70+
requestHeaders.length
71+
)
72+
requestHeaders.forEach((headerName) => {
73+
assert.property(payload[0][0].meta, `http.request.headers.${headerName}`)
74+
})
75+
76+
// Response headers
77+
assert.equal(
78+
Object.keys(payload[0][0].meta).filter(tagName => tagName.startsWith('http.response.headers.')).length,
79+
responseHeaders.length
80+
)
81+
responseHeaders.forEach((headerName) => {
82+
assert.property(payload[0][0].meta, `http.response.headers.${headerName}`)
83+
})
84+
}, 30000, 10, true)
85+
86+
assert.isTrue(fastifyRequestReceived)
87+
})
88+
})

packages/dd-trace/src/appsec/index.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const rasp = require('./rasp')
3737
const { isInServerlessEnvironment } = require('../serverless')
3838

3939
const responseAnalyzedSet = new WeakSet()
40+
const storedResponseHeaders = new WeakMap()
4041

4142
let isEnabled = false
4243
let config
@@ -187,7 +188,13 @@ function incomingHttpEndTranslator ({ req, res }) {
187188

188189
waf.disposeContext(req)
189190

190-
Reporter.finishRequest(req, res)
191+
const storedHeaders = storedResponseHeaders.get(req) || {}
192+
193+
Reporter.finishRequest(req, res, storedHeaders)
194+
195+
if (storedHeaders) {
196+
storedResponseHeaders.delete(req)
197+
}
191198
}
192199

193200
function onPassportVerify ({ framework, login, user, success, abortController }) {
@@ -285,6 +292,10 @@ function onResponseBody ({ req, res, body }) {
285292
}
286293

287294
function onResponseWriteHead ({ req, res, abortController, statusCode, responseHeaders }) {
295+
if (Object.keys(responseHeaders).length) {
296+
storedResponseHeaders.set(req, responseHeaders)
297+
}
298+
288299
// avoid "write after end" error
289300
if (isBlocked(res)) {
290301
abortController?.abort()

packages/dd-trace/src/appsec/reporter.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,14 +140,16 @@ function filterExtendedHeaders (headers, excludedHeaderNames, tagPrefix, limit =
140140
return result
141141
}
142142

143-
function getCollectedHeaders (req, res, shouldCollectEventHeaders) {
143+
function getCollectedHeaders (req, res, shouldCollectEventHeaders, storedResponseHeaders = {}) {
144144
// Mandatory
145145
const mandatoryCollectedHeaders = filterHeaders(req.headers, REQUEST_HEADERS_MAP)
146146

147147
// Basic collection
148148
if (!shouldCollectEventHeaders) return mandatoryCollectedHeaders
149149

150-
const responseHeaders = res.getHeaders()
150+
const responseHeaders = Object.keys(storedResponseHeaders).length === 0
151+
? res.getHeaders()
152+
: { ...storedResponseHeaders, ...res.getHeaders() }
151153

152154
const requestEventCollectedHeaders = filterHeaders(req.headers, EVENT_HEADERS_MAP)
153155
const responseEventCollectedHeaders = filterHeaders(responseHeaders, RESPONSE_HEADERS_MAP)
@@ -399,7 +401,7 @@ function reportDerivatives (derivatives) {
399401
rootSpan.addTags(tags)
400402
}
401403

402-
function finishRequest (req, res) {
404+
function finishRequest (req, res, storedResponseHeaders) {
403405
const rootSpan = web.root(req)
404406
if (!rootSpan) return
405407

@@ -453,7 +455,7 @@ function finishRequest (req, res) {
453455

454456
const tags = rootSpan.context()._tags
455457

456-
const newTags = getCollectedHeaders(req, res, shouldCollectEventHeaders(tags))
458+
const newTags = getCollectedHeaders(req, res, shouldCollectEventHeaders(tags), storedResponseHeaders)
457459

458460
if (tags['appsec.event'] === 'true' && typeof req.route?.path === 'string') {
459461
newTags['http.endpoint'] = req.route.path

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

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,52 @@ describe('AppSec Index', function () {
424424

425425
expect(waf.run).to.have.not.been.called
426426

427-
expect(Reporter.finishRequest).to.have.been.calledOnceWithExactly(req, res)
427+
expect(Reporter.finishRequest).to.have.been.calledOnceWithExactly(req, res, {})
428+
})
429+
430+
it('should pass stored response headers to Reporter.finishRequest', () => {
431+
const req = {
432+
url: '/path',
433+
headers: {
434+
'user-agent': 'Arachni',
435+
host: 'localhost'
436+
},
437+
method: 'POST',
438+
socket: {
439+
remoteAddress: '127.0.0.1',
440+
remotePort: 8080
441+
}
442+
}
443+
const res = {
444+
getHeaders: () => ({
445+
'content-type': 'application/json',
446+
'content-length': 42
447+
}),
448+
statusCode: 200
449+
}
450+
451+
const storedHeaders = {
452+
'content-type': 'text/plain',
453+
'content-language': 'en-US',
454+
'content-length': '15'
455+
}
456+
457+
web.patch(req)
458+
459+
sinon.stub(Reporter, 'finishRequest')
460+
sinon.stub(waf, 'disposeContext')
461+
462+
responseWriteHead.publish({
463+
req,
464+
res,
465+
abortController: { abort: sinon.stub() },
466+
statusCode: 200,
467+
responseHeaders: storedHeaders
468+
})
469+
470+
AppSec.incomingHttpEndTranslator({ req, res })
471+
472+
expect(Reporter.finishRequest).to.have.been.calledOnceWithExactly(req, res, storedHeaders)
428473
})
429474

430475
it('should not propagate incoming http end data with invalid framework properties', () => {
@@ -462,7 +507,7 @@ describe('AppSec Index', function () {
462507

463508
expect(waf.run).to.have.not.been.called
464509

465-
expect(Reporter.finishRequest).to.have.been.calledOnceWithExactly(req, res)
510+
expect(Reporter.finishRequest).to.have.been.calledOnceWithExactly(req, res, {})
466511
})
467512

468513
it('should propagate incoming http end data with express', () => {
@@ -512,7 +557,7 @@ describe('AppSec Index', function () {
512557
'server.request.query': { b: '2' }
513558
}
514559
}, req)
515-
expect(Reporter.finishRequest).to.have.been.calledOnceWithExactly(req, res)
560+
expect(Reporter.finishRequest).to.have.been.calledOnceWithExactly(req, res, {})
516561
})
517562
})
518563

0 commit comments

Comments
 (0)