Skip to content

Commit 37546ab

Browse files
wconti27wconti27
authored
inject cloned request headers for http requests (#5144)
* fix aws-sdk invalid signature exception * fix fetch and undici tests
1 parent c13d368 commit 37546ab

4 files changed

Lines changed: 23 additions & 262 deletions

File tree

‎packages/datadog-plugin-fetch/src/index.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,17 @@ class FetchPlugin extends HttpClientPlugin {
99
bindStart (ctx) {
1010
const req = ctx.req
1111
const options = new URL(req.url)
12-
const headers = options.headers = Object.fromEntries(req.headers.entries())
12+
options.headers = Object.fromEntries(req.headers.entries())
1313

1414
options.method = req.method
1515

1616
ctx.args = { options }
1717

1818
const store = super.bindStart(ctx)
1919

20-
for (const name in headers) {
20+
for (const name in options.headers) {
2121
if (!req.headers.has(name)) {
22-
req.headers.set(name, headers[name])
22+
req.headers.set(name, options.headers[name])
2323
}
2424
}
2525

‎packages/datadog-plugin-fetch/test/index.spec.js‎

Lines changed: 3 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ const HTTP_RESPONSE_HEADERS = tags.HTTP_RESPONSE_HEADERS
1414
const SERVICE_NAME = DD_MAJOR < 3 ? 'test-http-client' : 'test'
1515
const describe = globalThis.fetch ? globalThis.describe : globalThis.describe.skip
1616

17-
describe('Plugin', () => {
17+
describe('Plugin', function () {
18+
this.timeout(0)
19+
1820
let express
1921
let fetch
2022
let appListener
@@ -215,102 +217,6 @@ describe('Plugin', () => {
215217
})
216218
})
217219

218-
it('should skip injecting if the Authorization header contains an AWS signature', done => {
219-
const app = express()
220-
221-
app.get('/', (req, res) => {
222-
try {
223-
expect(req.get('x-datadog-trace-id')).to.be.undefined
224-
expect(req.get('x-datadog-parent-id')).to.be.undefined
225-
226-
res.status(200).send()
227-
228-
done()
229-
} catch (e) {
230-
done(e)
231-
}
232-
})
233-
234-
appListener = server(app, port => {
235-
fetch(`http://localhost:${port}/`, {
236-
headers: {
237-
Authorization: 'AWS4-HMAC-SHA256 ...'
238-
}
239-
})
240-
})
241-
})
242-
243-
it('should skip injecting if one of the Authorization headers contains an AWS signature', done => {
244-
const app = express()
245-
246-
app.get('/', (req, res) => {
247-
try {
248-
expect(req.get('x-datadog-trace-id')).to.be.undefined
249-
expect(req.get('x-datadog-parent-id')).to.be.undefined
250-
251-
res.status(200).send()
252-
253-
done()
254-
} catch (e) {
255-
done(e)
256-
}
257-
})
258-
259-
appListener = server(app, port => {
260-
fetch(`http://localhost:${port}/`, {
261-
headers: {
262-
Authorization: ['AWS4-HMAC-SHA256 ...']
263-
}
264-
})
265-
})
266-
})
267-
268-
it('should skip injecting if the X-Amz-Signature header is set', done => {
269-
const app = express()
270-
271-
app.get('/', (req, res) => {
272-
try {
273-
expect(req.get('x-datadog-trace-id')).to.be.undefined
274-
expect(req.get('x-datadog-parent-id')).to.be.undefined
275-
276-
res.status(200).send()
277-
278-
done()
279-
} catch (e) {
280-
done(e)
281-
}
282-
})
283-
284-
appListener = server(app, port => {
285-
fetch(`http://localhost:${port}/`, {
286-
headers: {
287-
'X-Amz-Signature': 'abc123'
288-
}
289-
})
290-
})
291-
})
292-
293-
it('should skip injecting if the X-Amz-Signature query param is set', done => {
294-
const app = express()
295-
296-
app.get('/', (req, res) => {
297-
try {
298-
expect(req.get('x-datadog-trace-id')).to.be.undefined
299-
expect(req.get('x-datadog-parent-id')).to.be.undefined
300-
301-
res.status(200).send()
302-
303-
done()
304-
} catch (e) {
305-
done(e)
306-
}
307-
})
308-
309-
appListener = server(app, port => {
310-
fetch(`http://localhost:${port}/?X-Amz-Signature=abc123`)
311-
})
312-
})
313-
314220
it('should handle connection errors', done => {
315221
let error
316222

‎packages/datadog-plugin-http/src/client.js‎

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ class HttpClientPlugin extends ClientPlugin {
5959
}
6060

6161
if (this.shouldInjectTraceHeaders(options, uri)) {
62+
// Clone the headers object in case an upstream lib has a reference to the original headers
63+
// Implemented due to aws-sdk issue where request signing is broken if we mutate the headers
64+
// Explained further in:
65+
// https://github.com/open-telemetry/opentelemetry-js-contrib/issues/1609#issuecomment-1826167348
66+
options.headers = Object.assign({}, options.headers)
6267
this.tracer.inject(span, HTTP_HEADERS, options.headers)
6368
}
6469

@@ -72,10 +77,6 @@ class HttpClientPlugin extends ClientPlugin {
7277
}
7378

7479
shouldInjectTraceHeaders (options, uri) {
75-
if (hasAmazonSignature(options) && !this.config.enablePropagationWithAmazonHeaders) {
76-
return false
77-
}
78-
7980
if (!this.config.propagationFilter(uri)) {
8081
return false
8182
}
@@ -212,31 +213,6 @@ function getHooks (config) {
212213
return { request }
213214
}
214215

215-
function hasAmazonSignature (options) {
216-
if (!options) {
217-
return false
218-
}
219-
220-
if (options.headers) {
221-
const headers = Object.keys(options.headers)
222-
.reduce((prev, next) => Object.assign(prev, {
223-
[next.toLowerCase()]: options.headers[next]
224-
}), {})
225-
226-
if (headers['x-amz-signature']) {
227-
return true
228-
}
229-
230-
if ([].concat(headers.authorization).some(startsWith('AWS4-HMAC-SHA256'))) {
231-
return true
232-
}
233-
}
234-
235-
const search = options.search || options.path
236-
237-
return search && search.toLowerCase().indexOf('x-amz-signature=') !== -1
238-
}
239-
240216
function extractSessionDetails (options) {
241217
if (typeof options === 'string') {
242218
return new URL(options).host
@@ -248,8 +224,4 @@ function extractSessionDetails (options) {
248224
return { host, port }
249225
}
250226

251-
function startsWith (searchString) {
252-
return value => String(value).startsWith(searchString)
253-
}
254-
255227
module.exports = HttpClientPlugin

‎packages/datadog-plugin-http/test/client.spec.js‎

Lines changed: 12 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -446,97 +446,24 @@ describe('Plugin', () => {
446446
})
447447
})
448448

449-
it('should skip injecting if the Authorization header contains an AWS signature', done => {
450-
const app = express()
451-
452-
app.get('/', (req, res) => {
453-
try {
454-
expect(req.get('x-datadog-trace-id')).to.be.undefined
455-
expect(req.get('x-datadog-parent-id')).to.be.undefined
456-
457-
res.status(200).send()
458-
459-
done()
460-
} catch (e) {
461-
done(e)
462-
}
463-
})
464-
465-
appListener = server(app, port => {
466-
const req = http.request({
467-
port,
468-
headers: {
469-
Authorization: 'AWS4-HMAC-SHA256 ...'
470-
}
471-
})
449+
it('should inject tracing header into request without mutating the header', done => {
450+
// ensures that the tracer clones request headers instead of mutating.
451+
// Fixes aws-sdk InvalidSignatureException, more info:
452+
// https://github.com/open-telemetry/opentelemetry-js-contrib/issues/1609#issuecomment-1826167348
472453

473-
req.end()
474-
})
475-
})
476-
477-
it('should skip injecting if one of the Authorization headers contains an AWS signature', done => {
478454
const app = express()
479455

480-
app.get('/', (req, res) => {
481-
try {
482-
expect(req.get('x-datadog-trace-id')).to.be.undefined
483-
expect(req.get('x-datadog-parent-id')).to.be.undefined
484-
485-
res.status(200).send()
486-
487-
done()
488-
} catch (e) {
489-
done(e)
490-
}
491-
})
492-
493-
appListener = server(app, port => {
494-
const req = http.request({
495-
port,
496-
headers: {
497-
Authorization: ['AWS4-HMAC-SHA256 ...']
498-
}
499-
})
500-
501-
req.end()
502-
})
503-
})
504-
505-
it('should skip injecting if the X-Amz-Signature header is set', done => {
506-
const app = express()
456+
const originalHeaders = {
457+
Authorization: 'AWS4-HMAC-SHA256 ...'
458+
}
507459

508460
app.get('/', (req, res) => {
509461
try {
510-
expect(req.get('x-datadog-trace-id')).to.be.undefined
511-
expect(req.get('x-datadog-parent-id')).to.be.undefined
512-
513-
res.status(200).send()
514-
515-
done()
516-
} catch (e) {
517-
done(e)
518-
}
519-
})
520-
521-
appListener = server(app, port => {
522-
const req = http.request({
523-
port,
524-
headers: {
525-
'X-Amz-Signature': 'abc123'
526-
}
527-
})
528-
529-
req.end()
530-
})
531-
})
532-
533-
it('should skip injecting if the X-Amz-Signature query param is set', done => {
534-
const app = express()
462+
expect(req.get('x-datadog-trace-id')).to.be.a('string')
463+
expect(req.get('x-datadog-parent-id')).to.be.a('string')
535464

536-
app.get('/', (req, res) => {
537-
try {
538-
expect(req.get('x-datadog-trace-id')).to.be.undefined
539-
expect(req.get('x-datadog-parent-id')).to.be.undefined
465+
expect(originalHeaders['x-datadog-trace-id']).to.be.undefined
466+
expect(originalHeaders['x-datadog-parent-id']).to.be.undefined
540467

541468
res.status(200).send()
542469

@@ -549,7 +476,7 @@ describe('Plugin', () => {
549476
appListener = server(app, port => {
550477
const req = http.request({
551478
port,
552-
path: '/?X-Amz-Signature=abc123'
479+
headers: originalHeaders
553480
})
554481

555482
req.end()
@@ -1093,50 +1020,6 @@ describe('Plugin', () => {
10931020
})
10941021
})
10951022

1096-
describe('with config enablePropagationWithAmazonHeaders enabled', () => {
1097-
let config
1098-
1099-
beforeEach(() => {
1100-
config = {
1101-
enablePropagationWithAmazonHeaders: true
1102-
}
1103-
1104-
return agent.load('http', config)
1105-
.then(() => {
1106-
http = require(pluginToBeLoaded)
1107-
express = require('express')
1108-
})
1109-
})
1110-
1111-
it('should inject tracing header into AWS signed request', done => {
1112-
const app = express()
1113-
1114-
app.get('/', (req, res) => {
1115-
try {
1116-
expect(req.get('x-datadog-trace-id')).to.be.a('string')
1117-
expect(req.get('x-datadog-parent-id')).to.be.a('string')
1118-
1119-
res.status(200).send()
1120-
1121-
done()
1122-
} catch (e) {
1123-
done(e)
1124-
}
1125-
})
1126-
1127-
appListener = server(app, port => {
1128-
const req = http.request({
1129-
port,
1130-
headers: {
1131-
Authorization: 'AWS4-HMAC-SHA256 ...'
1132-
}
1133-
})
1134-
1135-
req.end()
1136-
})
1137-
})
1138-
})
1139-
11401023
describe('with validateStatus configuration', () => {
11411024
let config
11421025

0 commit comments

Comments
 (0)