Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions packages/dd-trace/src/plugins/util/web.js
Original file line number Diff line number Diff line change
Expand Up @@ -333,15 +333,26 @@ const web = {
// GET / POST / etc. case. Node's http module passes `req.method`
// through unchanged, so all standard methods are uppercase; the
// `toLowerCase` fallback covers any non-standard caller.
let headersModified = false
if (req.method === 'OPTIONS' || req.method.toLowerCase() === 'options') {
headers = typeof statusMessage === 'string' ? headers : statusMessage
headers = { ...res.getHeaders(), ...headers }

if (isOriginAllowed(req, headers)) {
addAllowHeaders(req, res, headers)
const headersObj = Array.isArray(headers) ? flatHeadersToObject(headers) : headers
Comment thread
pabloerhard marked this conversation as resolved.
Outdated
const mergedHeaders = { ...res.getHeaders(), ...headersObj }
Comment thread
pabloerhard marked this conversation as resolved.
Outdated
if (isOriginAllowed(req, mergedHeaders)) {
const allowedHeaders = computeAllowedHeaders(req, mergedHeaders)
if (allowedHeaders) {
headers = { ...headersObj, 'access-control-allow-headers': allowedHeaders }
headersModified = true
}
}
}

if (headersModified) {
if (typeof statusMessage === 'string') {
return writeHead.call(this, statusCode, statusMessage, headers)
}
return writeHead.call(this, statusCode, headers)
Comment thread
pabloerhard marked this conversation as resolved.
}
return writeHead.apply(this, arguments)
}
},
Expand Down Expand Up @@ -372,7 +383,7 @@ function normalizeHeadersCarrier (headers) {
return carrier
}

function addAllowHeaders (req, res, headers) {
function computeAllowedHeaders (req, headers) {
const allowHeaders = splitHeader(headers['access-control-allow-headers'])
const requestHeaders = splitHeader(req.headers['access-control-request-headers'])
const contextHeaders = [
Expand All @@ -393,9 +404,7 @@ function addAllowHeaders (req, res, headers) {
}
}

if (allowHeaders.length > 0) {
res.setHeader('access-control-allow-headers', uniq(allowHeaders).join(','))
}
return uniq(allowHeaders).join(',')
}

function isOriginAllowed (req, headers) {
Expand All @@ -409,6 +418,14 @@ function splitHeader (str) {
return typeof str === 'string' ? str.split(',').map((header) => header.trim()) : []
}

function flatHeadersToObject (headers) {
const result = {}
for (let i = 0; i < headers.length; i += 2) {
result[headers[i]] = headers[i + 1]
Comment thread
pabloerhard marked this conversation as resolved.
Outdated
}
return result
}

function addRequestTags (context, spanType) {
const { req, span, inferredProxySpan, config } = context
const spanContext = span.context()
Expand Down
95 changes: 71 additions & 24 deletions packages/dd-trace/test/plugins/util/web.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -826,23 +826,27 @@ describe('plugins/util/web', () => {
req.headers.origin = 'https://example.com'
req.headers['access-control-request-headers'] = 'x-datadog-trace-id'
res.getHeaders.returns({ [ALLOW_ORIGIN]: '*' })
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200)

assert.ok(res.setHeader.notCalled)
assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(res.writeHead.firstCall.args, [200])
})

it('skips allow-header tagging on OPTIONS when the origin is not allowed', () => {
req.method = 'OPTIONS'
req.headers.origin = 'https://evil.example.com'
req.headers['access-control-request-headers'] = 'x-datadog-trace-id'
res.getHeaders.returns({ [ALLOW_ORIGIN]: 'https://good.example.com' })
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200)

assert.ok(res.setHeader.notCalled)
assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(res.writeHead.firstCall.args, [200])
})

it('merges datadog allow-headers on OPTIONS when allow-origin is *', () => {
Expand All @@ -851,14 +855,15 @@ describe('plugins/util/web', () => {
req.headers['access-control-request-headers'] =
'x-datadog-trace-id, x-datadog-parent-id, x-other'
res.getHeaders.returns({ [ALLOW_ORIGIN]: '*' })
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200)

assert.ok(res.setHeader.calledOnce)
assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(
res.setHeader.firstCall.args,
[ALLOW_HEADERS, 'x-datadog-parent-id,x-datadog-trace-id']
res.writeHead.firstCall.args,
[200, { [ALLOW_HEADERS]: 'x-datadog-parent-id,x-datadog-trace-id' }]
)
})

Expand All @@ -867,14 +872,15 @@ describe('plugins/util/web', () => {
req.headers.origin = 'https://example.com'
req.headers['access-control-request-headers'] = 'baggage, traceparent, tracestate, x-other'
res.getHeaders.returns({ [ALLOW_ORIGIN]: '*' })
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200)

assert.ok(res.setHeader.calledOnce)
assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(
res.setHeader.firstCall.args,
[ALLOW_HEADERS, 'baggage,traceparent,tracestate']
res.writeHead.firstCall.args,
[200, { [ALLOW_HEADERS]: 'baggage,traceparent,tracestate' }]
)
})

Expand All @@ -883,14 +889,15 @@ describe('plugins/util/web', () => {
req.headers.origin = 'https://example.com'
req.headers['access-control-request-headers'] = 'x-datadog-trace-id'
res.getHeaders.returns({})
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200, { [ALLOW_ORIGIN]: 'https://example.com' })

assert.ok(res.setHeader.calledOnce)
assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(
res.setHeader.firstCall.args,
[ALLOW_HEADERS, 'x-datadog-trace-id']
res.writeHead.firstCall.args,
[200, { [ALLOW_ORIGIN]: 'https://example.com', [ALLOW_HEADERS]: 'x-datadog-trace-id' }]
)
})

Expand All @@ -899,14 +906,32 @@ describe('plugins/util/web', () => {
req.headers.origin = 'https://example.com'
req.headers['access-control-request-headers'] = 'x-datadog-trace-id'
res.getHeaders.returns({})
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200, 'OK', { [ALLOW_ORIGIN]: '*' })

assert.ok(res.setHeader.calledOnce)
assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(
res.setHeader.firstCall.args,
[ALLOW_HEADERS, 'x-datadog-trace-id']
res.writeHead.firstCall.args,
[200, 'OK', { [ALLOW_ORIGIN]: '*', [ALLOW_HEADERS]: 'x-datadog-trace-id' }]
)
})

it('honours headers passed as a flat array in rawHeaders format', () => {
req.method = 'OPTIONS'
req.headers.origin = 'https://example.com'
req.headers['access-control-request-headers'] = 'x-datadog-trace-id'
res.getHeaders.returns({})
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200, [ALLOW_ORIGIN, '*'])

assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(
res.writeHead.firstCall.args,
[200, { [ALLOW_ORIGIN]: '*', [ALLOW_HEADERS]: 'x-datadog-trace-id' }]
)
})

Expand All @@ -915,14 +940,15 @@ describe('plugins/util/web', () => {
req.headers.origin = 'https://example.com'
req.headers['access-control-request-headers'] = 'x-datadog-trace-id'
res.getHeaders.returns({ [ALLOW_ORIGIN]: '*' })
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200)

assert.ok(res.setHeader.calledOnce)
assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(
res.setHeader.firstCall.args,
[ALLOW_HEADERS, 'x-datadog-trace-id']
res.writeHead.firstCall.args,
[200, { [ALLOW_HEADERS]: 'x-datadog-trace-id' }]
)
})

Expand All @@ -934,14 +960,15 @@ describe('plugins/util/web', () => {
[ALLOW_ORIGIN]: '*',
[ALLOW_HEADERS]: 'content-type, x-datadog-trace-id',
})
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200)

assert.ok(res.setHeader.calledOnce)
assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(
res.setHeader.firstCall.args,
[ALLOW_HEADERS, 'content-type,x-datadog-trace-id']
res.writeHead.firstCall.args,
[200, { [ALLOW_HEADERS]: 'content-type,x-datadog-trace-id' }]
)
})

Expand All @@ -950,11 +977,13 @@ describe('plugins/util/web', () => {
req.headers.origin = 'https://example.com'
req.headers['access-control-request-headers'] = 'content-type, x-other'
res.getHeaders.returns({ [ALLOW_ORIGIN]: '*' })
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200)

assert.ok(res.setHeader.notCalled)
assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(res.writeHead.firstCall.args, [200])
})

it('delegates to the original writeHead with the same arguments', () => {
Expand All @@ -973,19 +1002,37 @@ describe('plugins/util/web', () => {
)
})

it('passes merged allow-headers to writeHead so it survives writeHead precedence', () => {
req.method = 'OPTIONS'
req.headers.origin = 'https://example.com'
req.headers['access-control-request-headers'] = 'x-datadog-trace-id'
res.getHeaders.returns({ [ALLOW_ORIGIN]: '*' })
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200, { [ALLOW_ORIGIN]: '*', [ALLOW_HEADERS]: 'content-type' })

assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(
res.writeHead.firstCall.args,
[200, { [ALLOW_ORIGIN]: '*', [ALLOW_HEADERS]: 'content-type,x-datadog-trace-id' }]
)
})

it('trims whitespace surrounding each requested header entry', () => {
req.method = 'OPTIONS'
req.headers.origin = 'https://example.com'
req.headers['access-control-request-headers'] = ' x-datadog-parent-id ,x-datadog-trace-id '
res.getHeaders.returns({ [ALLOW_ORIGIN]: '*' })
res.writeHead = sinon.spy()

const wrapped = web.wrapWriteHead(context)
wrapped.call(res, 200)

assert.ok(res.setHeader.calledOnce)
assert.ok(res.writeHead.calledOnce)
assert.deepStrictEqual(
res.setHeader.firstCall.args,
[ALLOW_HEADERS, 'x-datadog-parent-id,x-datadog-trace-id']
res.writeHead.firstCall.args,
[200, { [ALLOW_HEADERS]: 'x-datadog-parent-id,x-datadog-trace-id' }]
)
})
})
Expand Down
Loading