From 37546abc819558560d991aaac23a71bffe1f0555 Mon Sep 17 00:00:00 2001 From: William Conti <58711692+wconti27@users.noreply.github.com> Date: Wed, 22 Jan 2025 12:44:27 -0500 Subject: [PATCH] inject cloned request headers for http requests (#5144) * fix aws-sdk invalid signature exception * fix fetch and undici tests --- packages/datadog-plugin-fetch/src/index.js | 6 +- .../datadog-plugin-fetch/test/index.spec.js | 100 +------------ packages/datadog-plugin-http/src/client.js | 38 +---- .../datadog-plugin-http/test/client.spec.js | 141 ++---------------- 4 files changed, 23 insertions(+), 262 deletions(-) diff --git a/packages/datadog-plugin-fetch/src/index.js b/packages/datadog-plugin-fetch/src/index.js index 44173a561ca..943a1908ddb 100644 --- a/packages/datadog-plugin-fetch/src/index.js +++ b/packages/datadog-plugin-fetch/src/index.js @@ -9,7 +9,7 @@ class FetchPlugin extends HttpClientPlugin { bindStart (ctx) { const req = ctx.req const options = new URL(req.url) - const headers = options.headers = Object.fromEntries(req.headers.entries()) + options.headers = Object.fromEntries(req.headers.entries()) options.method = req.method @@ -17,9 +17,9 @@ class FetchPlugin extends HttpClientPlugin { const store = super.bindStart(ctx) - for (const name in headers) { + for (const name in options.headers) { if (!req.headers.has(name)) { - req.headers.set(name, headers[name]) + req.headers.set(name, options.headers[name]) } } diff --git a/packages/datadog-plugin-fetch/test/index.spec.js b/packages/datadog-plugin-fetch/test/index.spec.js index 1d322de04a4..1d20d375d79 100644 --- a/packages/datadog-plugin-fetch/test/index.spec.js +++ b/packages/datadog-plugin-fetch/test/index.spec.js @@ -14,7 +14,9 @@ const HTTP_RESPONSE_HEADERS = tags.HTTP_RESPONSE_HEADERS const SERVICE_NAME = DD_MAJOR < 3 ? 'test-http-client' : 'test' const describe = globalThis.fetch ? globalThis.describe : globalThis.describe.skip -describe('Plugin', () => { +describe('Plugin', function () { + this.timeout(0) + let express let fetch let appListener @@ -215,102 +217,6 @@ describe('Plugin', () => { }) }) - it('should skip injecting if the Authorization header contains an AWS signature', done => { - const app = express() - - app.get('/', (req, res) => { - try { - expect(req.get('x-datadog-trace-id')).to.be.undefined - expect(req.get('x-datadog-parent-id')).to.be.undefined - - res.status(200).send() - - done() - } catch (e) { - done(e) - } - }) - - appListener = server(app, port => { - fetch(`http://localhost:${port}/`, { - headers: { - Authorization: 'AWS4-HMAC-SHA256 ...' - } - }) - }) - }) - - it('should skip injecting if one of the Authorization headers contains an AWS signature', done => { - const app = express() - - app.get('/', (req, res) => { - try { - expect(req.get('x-datadog-trace-id')).to.be.undefined - expect(req.get('x-datadog-parent-id')).to.be.undefined - - res.status(200).send() - - done() - } catch (e) { - done(e) - } - }) - - appListener = server(app, port => { - fetch(`http://localhost:${port}/`, { - headers: { - Authorization: ['AWS4-HMAC-SHA256 ...'] - } - }) - }) - }) - - it('should skip injecting if the X-Amz-Signature header is set', done => { - const app = express() - - app.get('/', (req, res) => { - try { - expect(req.get('x-datadog-trace-id')).to.be.undefined - expect(req.get('x-datadog-parent-id')).to.be.undefined - - res.status(200).send() - - done() - } catch (e) { - done(e) - } - }) - - appListener = server(app, port => { - fetch(`http://localhost:${port}/`, { - headers: { - 'X-Amz-Signature': 'abc123' - } - }) - }) - }) - - it('should skip injecting if the X-Amz-Signature query param is set', done => { - const app = express() - - app.get('/', (req, res) => { - try { - expect(req.get('x-datadog-trace-id')).to.be.undefined - expect(req.get('x-datadog-parent-id')).to.be.undefined - - res.status(200).send() - - done() - } catch (e) { - done(e) - } - }) - - appListener = server(app, port => { - fetch(`http://localhost:${port}/?X-Amz-Signature=abc123`) - }) - }) - it('should handle connection errors', done => { let error diff --git a/packages/datadog-plugin-http/src/client.js b/packages/datadog-plugin-http/src/client.js index d4c105d2508..2bc408e648b 100644 --- a/packages/datadog-plugin-http/src/client.js +++ b/packages/datadog-plugin-http/src/client.js @@ -59,6 +59,11 @@ class HttpClientPlugin extends ClientPlugin { } if (this.shouldInjectTraceHeaders(options, uri)) { + // Clone the headers object in case an upstream lib has a reference to the original headers + // Implemented due to aws-sdk issue where request signing is broken if we mutate the headers + // Explained further in: + // https://github.com/open-telemetry/opentelemetry-js-contrib/issues/1609#issuecomment-1826167348 + options.headers = Object.assign({}, options.headers) this.tracer.inject(span, HTTP_HEADERS, options.headers) } @@ -72,10 +77,6 @@ class HttpClientPlugin extends ClientPlugin { } shouldInjectTraceHeaders (options, uri) { - if (hasAmazonSignature(options) && !this.config.enablePropagationWithAmazonHeaders) { - return false - } - if (!this.config.propagationFilter(uri)) { return false } @@ -212,31 +213,6 @@ function getHooks (config) { return { request } } -function hasAmazonSignature (options) { - if (!options) { - return false - } - - if (options.headers) { - const headers = Object.keys(options.headers) - .reduce((prev, next) => Object.assign(prev, { - [next.toLowerCase()]: options.headers[next] - }), {}) - - if (headers['x-amz-signature']) { - return true - } - - if ([].concat(headers.authorization).some(startsWith('AWS4-HMAC-SHA256'))) { - return true - } - } - - const search = options.search || options.path - - return search && search.toLowerCase().indexOf('x-amz-signature=') !== -1 -} - function extractSessionDetails (options) { if (typeof options === 'string') { return new URL(options).host @@ -248,8 +224,4 @@ function extractSessionDetails (options) { return { host, port } } -function startsWith (searchString) { - return value => String(value).startsWith(searchString) -} - module.exports = HttpClientPlugin diff --git a/packages/datadog-plugin-http/test/client.spec.js b/packages/datadog-plugin-http/test/client.spec.js index 42f4c8436f8..ff2d220d0cd 100644 --- a/packages/datadog-plugin-http/test/client.spec.js +++ b/packages/datadog-plugin-http/test/client.spec.js @@ -446,97 +446,24 @@ describe('Plugin', () => { }) }) - it('should skip injecting if the Authorization header contains an AWS signature', done => { - const app = express() - - app.get('/', (req, res) => { - try { - expect(req.get('x-datadog-trace-id')).to.be.undefined - expect(req.get('x-datadog-parent-id')).to.be.undefined - - res.status(200).send() - - done() - } catch (e) { - done(e) - } - }) - - appListener = server(app, port => { - const req = http.request({ - port, - headers: { - Authorization: 'AWS4-HMAC-SHA256 ...' - } - }) + it('should inject tracing header into request without mutating the header', done => { + // ensures that the tracer clones request headers instead of mutating. + // Fixes aws-sdk InvalidSignatureException, more info: + // https://github.com/open-telemetry/opentelemetry-js-contrib/issues/1609#issuecomment-1826167348 - req.end() - }) - }) - - it('should skip injecting if one of the Authorization headers contains an AWS signature', done => { const app = express() - app.get('/', (req, res) => { - try { - expect(req.get('x-datadog-trace-id')).to.be.undefined - expect(req.get('x-datadog-parent-id')).to.be.undefined - - res.status(200).send() - - done() - } catch (e) { - done(e) - } - }) - - appListener = server(app, port => { - const req = http.request({ - port, - headers: { - Authorization: ['AWS4-HMAC-SHA256 ...'] - } - }) - - req.end() - }) - }) - - it('should skip injecting if the X-Amz-Signature header is set', done => { - const app = express() + const originalHeaders = { + Authorization: 'AWS4-HMAC-SHA256 ...' + } app.get('/', (req, res) => { try { - expect(req.get('x-datadog-trace-id')).to.be.undefined - expect(req.get('x-datadog-parent-id')).to.be.undefined - - res.status(200).send() - - done() - } catch (e) { - done(e) - } - }) - - appListener = server(app, port => { - const req = http.request({ - port, - headers: { - 'X-Amz-Signature': 'abc123' - } - }) - - req.end() - }) - }) - - it('should skip injecting if the X-Amz-Signature query param is set', done => { - const app = express() + expect(req.get('x-datadog-trace-id')).to.be.a('string') + expect(req.get('x-datadog-parent-id')).to.be.a('string') - app.get('/', (req, res) => { - try { - expect(req.get('x-datadog-trace-id')).to.be.undefined - expect(req.get('x-datadog-parent-id')).to.be.undefined + expect(originalHeaders['x-datadog-trace-id']).to.be.undefined + expect(originalHeaders['x-datadog-parent-id']).to.be.undefined res.status(200).send() @@ -549,7 +476,7 @@ describe('Plugin', () => { appListener = server(app, port => { const req = http.request({ port, - path: '/?X-Amz-Signature=abc123' + headers: originalHeaders }) req.end() @@ -1093,50 +1020,6 @@ describe('Plugin', () => { }) }) - describe('with config enablePropagationWithAmazonHeaders enabled', () => { - let config - - beforeEach(() => { - config = { - enablePropagationWithAmazonHeaders: true - } - - return agent.load('http', config) - .then(() => { - http = require(pluginToBeLoaded) - express = require('express') - }) - }) - - it('should inject tracing header into AWS signed request', done => { - const app = express() - - app.get('/', (req, res) => { - try { - expect(req.get('x-datadog-trace-id')).to.be.a('string') - expect(req.get('x-datadog-parent-id')).to.be.a('string') - - res.status(200).send() - - done() - } catch (e) { - done(e) - } - }) - - appListener = server(app, port => { - const req = http.request({ - port, - headers: { - Authorization: 'AWS4-HMAC-SHA256 ...' - } - }) - - req.end() - }) - }) - }) - describe('with validateStatus configuration', () => { let config