Skip to content

Commit 9bcb63d

Browse files
pabloerhardpabloerhard
committed
fix(web): add flat array support and fix presedence issue
1 parent f72b07e commit 9bcb63d

2 files changed

Lines changed: 96 additions & 32 deletions

File tree

‎packages/dd-trace/src/plugins/util/web.js‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -333,15 +333,26 @@ const web = {
333333
// GET / POST / etc. case. Node's http module passes `req.method`
334334
// through unchanged, so all standard methods are uppercase; the
335335
// `toLowerCase` fallback covers any non-standard caller.
336+
let headersModified = false
336337
if (req.method === 'OPTIONS' || req.method.toLowerCase() === 'options') {
337338
headers = typeof statusMessage === 'string' ? headers : statusMessage
338-
headers = { ...res.getHeaders(), ...headers }
339-
340-
if (isOriginAllowed(req, headers)) {
341-
addAllowHeaders(req, res, headers)
339+
const headersObj = Array.isArray(headers) ? flatHeadersToObject(headers) : headers
340+
const mergedHeaders = { ...res.getHeaders(), ...headersObj }
341+
if (isOriginAllowed(req, mergedHeaders)) {
342+
const allowedHeaders = computeAllowedHeaders(req, mergedHeaders)
343+
if (allowedHeaders) {
344+
headers = { ...headersObj, 'access-control-allow-headers': allowedHeaders }
345+
headersModified = true
346+
}
342347
}
343348
}
344349

350+
if (headersModified) {
351+
if (typeof statusMessage === 'string') {
352+
return writeHead.call(this, statusCode, statusMessage, headers)
353+
}
354+
return writeHead.call(this, statusCode, headers)
355+
}
345356
return writeHead.apply(this, arguments)
346357
}
347358
},
@@ -372,7 +383,7 @@ function normalizeHeadersCarrier (headers) {
372383
return carrier
373384
}
374385

375-
function addAllowHeaders (req, res, headers) {
386+
function computeAllowedHeaders (req, headers) {
376387
const allowHeaders = splitHeader(headers['access-control-allow-headers'])
377388
const requestHeaders = splitHeader(req.headers['access-control-request-headers'])
378389
const contextHeaders = [
@@ -393,9 +404,7 @@ function addAllowHeaders (req, res, headers) {
393404
}
394405
}
395406

396-
if (allowHeaders.length > 0) {
397-
res.setHeader('access-control-allow-headers', uniq(allowHeaders).join(','))
398-
}
407+
return uniq(allowHeaders).join(',')
399408
}
400409

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

421+
function flatHeadersToObject (headers) {
422+
const result = {}
423+
for (let i = 0; i < headers.length; i += 2) {
424+
result[headers[i]] = headers[i + 1]
425+
}
426+
return result
427+
}
428+
412429
function addRequestTags (context, spanType) {
413430
const { req, span, inferredProxySpan, config } = context
414431
const spanContext = span.context()

‎packages/dd-trace/test/plugins/util/web.spec.js‎

Lines changed: 71 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -826,23 +826,27 @@ describe('plugins/util/web', () => {
826826
req.headers.origin = 'https://example.com'
827827
req.headers['access-control-request-headers'] = 'x-datadog-trace-id'
828828
res.getHeaders.returns({ [ALLOW_ORIGIN]: '*' })
829+
res.writeHead = sinon.spy()
829830

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

833-
assert.ok(res.setHeader.notCalled)
834+
assert.ok(res.writeHead.calledOnce)
835+
assert.deepStrictEqual(res.writeHead.firstCall.args, [200])
834836
})
835837

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

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

845-
assert.ok(res.setHeader.notCalled)
848+
assert.ok(res.writeHead.calledOnce)
849+
assert.deepStrictEqual(res.writeHead.firstCall.args, [200])
846850
})
847851

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

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

858-
assert.ok(res.setHeader.calledOnce)
863+
assert.ok(res.writeHead.calledOnce)
859864
assert.deepStrictEqual(
860-
res.setHeader.firstCall.args,
861-
[ALLOW_HEADERS, 'x-datadog-parent-id,x-datadog-trace-id']
865+
res.writeHead.firstCall.args,
866+
[200, { [ALLOW_HEADERS]: 'x-datadog-parent-id,x-datadog-trace-id' }]
862867
)
863868
})
864869

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

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

874-
assert.ok(res.setHeader.calledOnce)
880+
assert.ok(res.writeHead.calledOnce)
875881
assert.deepStrictEqual(
876-
res.setHeader.firstCall.args,
877-
[ALLOW_HEADERS, 'baggage,traceparent,tracestate']
882+
res.writeHead.firstCall.args,
883+
[200, { [ALLOW_HEADERS]: 'baggage,traceparent,tracestate' }]
878884
)
879885
})
880886

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

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

890-
assert.ok(res.setHeader.calledOnce)
897+
assert.ok(res.writeHead.calledOnce)
891898
assert.deepStrictEqual(
892-
res.setHeader.firstCall.args,
893-
[ALLOW_HEADERS, 'x-datadog-trace-id']
899+
res.writeHead.firstCall.args,
900+
[200, { [ALLOW_ORIGIN]: 'https://example.com', [ALLOW_HEADERS]: 'x-datadog-trace-id' }]
894901
)
895902
})
896903

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

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

906-
assert.ok(res.setHeader.calledOnce)
914+
assert.ok(res.writeHead.calledOnce)
907915
assert.deepStrictEqual(
908-
res.setHeader.firstCall.args,
909-
[ALLOW_HEADERS, 'x-datadog-trace-id']
916+
res.writeHead.firstCall.args,
917+
[200, 'OK', { [ALLOW_ORIGIN]: '*', [ALLOW_HEADERS]: 'x-datadog-trace-id' }]
918+
)
919+
})
920+
921+
it('honours headers passed as a flat array in rawHeaders format', () => {
922+
req.method = 'OPTIONS'
923+
req.headers.origin = 'https://example.com'
924+
req.headers['access-control-request-headers'] = 'x-datadog-trace-id'
925+
res.getHeaders.returns({})
926+
res.writeHead = sinon.spy()
927+
928+
const wrapped = web.wrapWriteHead(context)
929+
wrapped.call(res, 200, [ALLOW_ORIGIN, '*'])
930+
931+
assert.ok(res.writeHead.calledOnce)
932+
assert.deepStrictEqual(
933+
res.writeHead.firstCall.args,
934+
[200, { [ALLOW_ORIGIN]: '*', [ALLOW_HEADERS]: 'x-datadog-trace-id' }]
910935
)
911936
})
912937

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

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

922-
assert.ok(res.setHeader.calledOnce)
948+
assert.ok(res.writeHead.calledOnce)
923949
assert.deepStrictEqual(
924-
res.setHeader.firstCall.args,
925-
[ALLOW_HEADERS, 'x-datadog-trace-id']
950+
res.writeHead.firstCall.args,
951+
[200, { [ALLOW_HEADERS]: 'x-datadog-trace-id' }]
926952
)
927953
})
928954

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

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

941-
assert.ok(res.setHeader.calledOnce)
968+
assert.ok(res.writeHead.calledOnce)
942969
assert.deepStrictEqual(
943-
res.setHeader.firstCall.args,
944-
[ALLOW_HEADERS, 'content-type,x-datadog-trace-id']
970+
res.writeHead.firstCall.args,
971+
[200, { [ALLOW_HEADERS]: 'content-type,x-datadog-trace-id' }]
945972
)
946973
})
947974

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

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

957-
assert.ok(res.setHeader.notCalled)
985+
assert.ok(res.writeHead.calledOnce)
986+
assert.deepStrictEqual(res.writeHead.firstCall.args, [200])
958987
})
959988

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

1005+
it('passes merged allow-headers to writeHead so it survives writeHead precedence', () => {
1006+
req.method = 'OPTIONS'
1007+
req.headers.origin = 'https://example.com'
1008+
req.headers['access-control-request-headers'] = 'x-datadog-trace-id'
1009+
res.getHeaders.returns({ [ALLOW_ORIGIN]: '*' })
1010+
res.writeHead = sinon.spy()
1011+
1012+
const wrapped = web.wrapWriteHead(context)
1013+
wrapped.call(res, 200, { [ALLOW_ORIGIN]: '*', [ALLOW_HEADERS]: 'content-type' })
1014+
1015+
assert.ok(res.writeHead.calledOnce)
1016+
assert.deepStrictEqual(
1017+
res.writeHead.firstCall.args,
1018+
[200, { [ALLOW_ORIGIN]: '*', [ALLOW_HEADERS]: 'content-type,x-datadog-trace-id' }]
1019+
)
1020+
})
1021+
9761022
it('trims whitespace surrounding each requested header entry', () => {
9771023
req.method = 'OPTIONS'
9781024
req.headers.origin = 'https://example.com'
9791025
req.headers['access-control-request-headers'] = ' x-datadog-parent-id ,x-datadog-trace-id '
9801026
res.getHeaders.returns({ [ALLOW_ORIGIN]: '*' })
1027+
res.writeHead = sinon.spy()
9811028

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

985-
assert.ok(res.setHeader.calledOnce)
1032+
assert.ok(res.writeHead.calledOnce)
9861033
assert.deepStrictEqual(
987-
res.setHeader.firstCall.args,
988-
[ALLOW_HEADERS, 'x-datadog-parent-id,x-datadog-trace-id']
1034+
res.writeHead.firstCall.args,
1035+
[200, { [ALLOW_HEADERS]: 'x-datadog-parent-id,x-datadog-trace-id' }]
9891036
)
9901037
})
9911038
})

0 commit comments

Comments
 (0)