diff --git a/dev-packages/e2e-tests/test-applications/node-koa/index.js b/dev-packages/e2e-tests/test-applications/node-koa/index.js index ddc17f62e6f7..9e800a4fcc99 100644 --- a/dev-packages/e2e-tests/test-applications/node-koa/index.js +++ b/dev-packages/e2e-tests/test-applications/node-koa/index.js @@ -14,10 +14,12 @@ const port1 = 3030; const port2 = 3040; const Koa = require('koa'); +const { bodyParser } = require('@koa/bodyparser'); const Router = require('@koa/router'); const http = require('http'); const app1 = new Koa(); +app1.use(bodyParser()); Sentry.setupKoaErrorHandler(app1); @@ -109,6 +111,10 @@ router1.get('/test-assert/:condition', async ctx => { ctx.assert(condition, 400, 'ctx.assert failed'); }); +router1.post('/test-post', async ctx => { + ctx.body = { status: 'ok', body: ctx.request.body }; +}); + app1.use(router1.routes()).use(router1.allowedMethods()); app1.listen(port1); diff --git a/dev-packages/e2e-tests/test-applications/node-koa/package.json b/dev-packages/e2e-tests/test-applications/node-koa/package.json index 79a4e540c089..dd8a17d0f4b5 100644 --- a/dev-packages/e2e-tests/test-applications/node-koa/package.json +++ b/dev-packages/e2e-tests/test-applications/node-koa/package.json @@ -10,6 +10,7 @@ "test:assert": "pnpm test" }, "dependencies": { + "@koa/bodyparser": "^5.1.1", "@koa/router": "^12.0.1", "@sentry/node": "latest || *", "@sentry/types": "latest || *", diff --git a/dev-packages/e2e-tests/test-applications/node-koa/tests/assert.test.ts b/dev-packages/e2e-tests/test-applications/node-koa/tests/assert.test.ts index 0f9f724ef237..5bf468fc3772 100644 --- a/dev-packages/e2e-tests/test-applications/node-koa/tests/assert.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-koa/tests/assert.test.ts @@ -16,7 +16,6 @@ test('Returns 400 from failed assert', async ({ baseURL }) => { expect(errorEvent.request).toEqual({ method: 'GET', - cookies: {}, headers: expect.any(Object), url: 'http://localhost:3030/test-assert/false', }); diff --git a/dev-packages/e2e-tests/test-applications/node-koa/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-koa/tests/errors.test.ts index 1fbb1fd42613..03216b5cc01e 100644 --- a/dev-packages/e2e-tests/test-applications/node-koa/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-koa/tests/errors.test.ts @@ -15,7 +15,6 @@ test('Sends correct error event', async ({ baseURL }) => { expect(errorEvent.request).toEqual({ method: 'GET', - cookies: {}, headers: expect.any(Object), url: 'http://localhost:3030/test-exception/123', }); diff --git a/dev-packages/e2e-tests/test-applications/node-koa/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-koa/tests/transactions.test.ts index 4c52c932e7b4..8dcad3e01dff 100644 --- a/dev-packages/e2e-tests/test-applications/node-koa/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-koa/tests/transactions.test.ts @@ -46,76 +46,128 @@ test('Sends an API route transaction', async ({ baseURL }) => { origin: 'auto.http.otel.http', }); - expect(transactionEvent).toEqual( - expect.objectContaining({ - spans: [ - { - data: { - 'koa.name': '', - 'koa.type': 'middleware', - 'sentry.origin': 'auto.http.otel.koa', - 'sentry.op': 'middleware.koa', - }, - op: 'middleware.koa', - origin: 'auto.http.otel.koa', - description: '< unknown >', - parent_span_id: expect.any(String), - span_id: expect.any(String), - start_timestamp: expect.any(Number), - status: 'ok', - timestamp: expect.any(Number), - trace_id: expect.any(String), - }, - { - data: { - 'http.route': '/test-transaction', - 'koa.name': '/test-transaction', - 'koa.type': 'router', - 'sentry.origin': 'auto.http.otel.koa', - 'sentry.op': 'router.koa', - }, - op: 'router.koa', - description: '/test-transaction', - parent_span_id: expect.any(String), - span_id: expect.any(String), - start_timestamp: expect.any(Number), - status: 'ok', - timestamp: expect.any(Number), - trace_id: expect.any(String), - origin: 'auto.http.otel.koa', - }, - { - data: { - 'sentry.origin': 'manual', - }, - description: 'test-span', - parent_span_id: expect.any(String), - span_id: expect.any(String), - start_timestamp: expect.any(Number), - status: 'ok', - timestamp: expect.any(Number), - trace_id: expect.any(String), - origin: 'manual', - }, - { - data: { - 'sentry.origin': 'manual', - }, - description: 'child-span', - parent_span_id: expect.any(String), - span_id: expect.any(String), - start_timestamp: expect.any(Number), - status: 'ok', - timestamp: expect.any(Number), - trace_id: expect.any(String), - origin: 'manual', - }, - ], - transaction: 'GET /test-transaction', - type: 'transaction', - transaction_info: { - source: 'route', + expect(transactionEvent).toMatchObject({ + transaction: 'GET /test-transaction', + type: 'transaction', + transaction_info: { + source: 'route', + }, + }); + + expect(transactionEvent.spans).toEqual([ + { + data: { + 'koa.name': 'bodyParser', + 'koa.type': 'middleware', + 'sentry.op': 'middleware.koa', + 'sentry.origin': 'auto.http.otel.koa', + }, + description: 'bodyParser', + op: 'middleware.koa', + origin: 'auto.http.otel.koa', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + }, + { + data: { + 'koa.name': '', + 'koa.type': 'middleware', + 'sentry.origin': 'auto.http.otel.koa', + 'sentry.op': 'middleware.koa', + }, + op: 'middleware.koa', + origin: 'auto.http.otel.koa', + description: '< unknown >', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + }, + { + data: { + 'http.route': '/test-transaction', + 'koa.name': '/test-transaction', + 'koa.type': 'router', + 'sentry.origin': 'auto.http.otel.koa', + 'sentry.op': 'router.koa', }, + op: 'router.koa', + description: '/test-transaction', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + origin: 'auto.http.otel.koa', + }, + { + data: { + 'sentry.origin': 'manual', + }, + description: 'test-span', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + origin: 'manual', + }, + { + data: { + 'sentry.origin': 'manual', + }, + description: 'child-span', + parent_span_id: expect.any(String), + span_id: expect.any(String), + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: expect.any(String), + origin: 'manual', + }, + ]); +}); + +test('Captures request metadata', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('node-koa', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'POST /test-post' + ); + }); + + const res = await fetch(`${baseURL}/test-post`, { + method: 'POST', + body: JSON.stringify({ foo: 'bar', other: 1 }), + headers: { + 'Content-Type': 'application/json', + }, + }); + const resBody = await res.json(); + + expect(resBody).toEqual({ status: 'ok', body: { foo: 'bar', other: 1 } }); + + const transactionEvent = await transactionEventPromise; + + expect(transactionEvent.request).toEqual({ + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: expect.objectContaining({ + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/json', }), - ); + data: JSON.stringify({ + foo: 'bar', + other: 1, + }), + }); + + expect(transactionEvent.user).toEqual(undefined); }); diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index 36ba8691f065..7dc88ee70400 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -41,6 +41,7 @@ "amqplib": "^0.10.4", "apollo-server": "^3.11.1", "axios": "^1.6.7", + "body-parser": "^1.20.3", "connect": "^3.7.0", "cors": "^2.8.5", "cron": "^3.1.6", diff --git a/dev-packages/node-integration-tests/suites/express/tracing/server.js b/dev-packages/node-integration-tests/suites/express/tracing/server.js index 81560806097e..f9b4ae24b339 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/server.js +++ b/dev-packages/node-integration-tests/suites/express/tracing/server.js @@ -13,11 +13,15 @@ Sentry.init({ // express must be required after Sentry is initialized const express = require('express'); const cors = require('cors'); +const bodyParser = require('body-parser'); const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); const app = express(); app.use(cors()); +app.use(bodyParser.json()); +app.use(bodyParser.text()); +app.use(bodyParser.raw()); app.get('/test/express', (_req, res) => { res.send({ response: 'response 1' }); @@ -35,6 +39,10 @@ app.get(['/test/arr/:id', /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/ res.send({ response: 'response 4' }); }); +app.post('/test-post', function (req, res) { + res.send({ status: 'ok', body: req.body }); +}); + Sentry.setupExpressErrorHandler(app); startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/test.ts index 44852233ed67..56826177ab48 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/test.ts @@ -137,5 +137,109 @@ describe('express tracing', () => { .start(done) .makeRequest('get', `/test/${segment}`); }) as any); + + describe('request data', () => { + test('correctly captures JSON request data', done => { + const runner = createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/json', + }, + data: JSON.stringify({ + foo: 'bar', + other: 1, + }), + }, + }, + }) + .start(done); + + runner.makeRequest('post', '/test-post', {}, { foo: 'bar', other: 1 }); + }); + + test('correctly captures plain text request data', done => { + const runner = createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'text/plain', + }, + data: 'some plain text', + }, + }, + }) + .start(done); + + runner.makeRequest( + 'post', + '/test-post', + { + 'Content-Type': 'text/plain', + }, + 'some plain text', + ); + }); + + test('correctly captures text buffer request data', done => { + const runner = createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/octet-stream', + }, + data: 'some plain text in buffer', + }, + }, + }) + .start(done); + + runner.makeRequest( + 'post', + '/test-post', + { 'Content-Type': 'application/octet-stream' }, + Buffer.from('some plain text in buffer'), + ); + }); + + test('correctly captures non-text buffer request data', done => { + const runner = createRunner(__dirname, 'server.js') + .expect({ + transaction: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/octet-stream', + }, + // This is some non-ascii string representation + data: expect.any(String), + }, + }, + }) + .start(done); + + const body = new Uint8Array([1, 2, 3, 4, 5]).buffer; + + runner.makeRequest('post', '/test-post', { 'Content-Type': 'application/octet-stream' }, body); + }); + }); }); }); diff --git a/dev-packages/node-integration-tests/suites/express/without-tracing/server.ts b/dev-packages/node-integration-tests/suites/express/without-tracing/server.ts index 2a85d39b83b8..5b96e8b1a2a3 100644 --- a/dev-packages/node-integration-tests/suites/express/without-tracing/server.ts +++ b/dev-packages/node-integration-tests/suites/express/without-tracing/server.ts @@ -8,10 +8,15 @@ Sentry.init({ }); import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; +import bodyParser from 'body-parser'; import express from 'express'; const app = express(); +app.use(bodyParser.json()); +app.use(bodyParser.text()); +app.use(bodyParser.raw()); + Sentry.setTag('global', 'tag'); app.get('/test/isolationScope/:id', (req, res) => { @@ -24,6 +29,12 @@ app.get('/test/isolationScope/:id', (req, res) => { res.send({}); }); +app.post('/test-post', function (req, res) { + Sentry.captureException(new Error('This is an exception')); + + res.send({ status: 'ok', body: req.body }); +}); + Sentry.setupExpressErrorHandler(app); startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express/without-tracing/test.ts b/dev-packages/node-integration-tests/suites/express/without-tracing/test.ts index 7c304062bc22..c10d8706c42f 100644 --- a/dev-packages/node-integration-tests/suites/express/without-tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/express/without-tracing/test.ts @@ -4,26 +4,133 @@ afterAll(() => { cleanupChildProcesses(); }); -test('correctly applies isolation scope even without tracing', done => { - const runner = createRunner(__dirname, 'server.ts') - .expect({ - event: { - transaction: 'GET /test/isolationScope/1', - tags: { - global: 'tag', - 'isolation-scope': 'tag', - 'isolation-scope-1': '1', +describe('express without tracing', () => { + test('correctly applies isolation scope even without tracing', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + event: { + transaction: 'GET /test/isolationScope/1', + tags: { + global: 'tag', + 'isolation-scope': 'tag', + 'isolation-scope-1': '1', + }, + // Request is correctly set + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test\/isolationScope\/1$/), + method: 'GET', + headers: { + 'user-agent': expect.stringContaining(''), + }, + }, }, - // Request is correctly set - request: { - url: expect.stringContaining('/test/isolationScope/1'), - headers: { - 'user-agent': expect.stringContaining(''), + }) + .start(done); + + runner.makeRequest('get', '/test/isolationScope/1'); + }); + + describe('request data', () => { + test('correctly captures JSON request data', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + event: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/json', + }, + data: JSON.stringify({ + foo: 'bar', + other: 1, + }), + }, + }, + }) + .start(done); + + runner.makeRequest('post', '/test-post', {}, { foo: 'bar', other: 1 }); + }); + + test('correctly captures plain text request data', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + event: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'text/plain', + }, + data: 'some plain text', + }, }, + }) + .start(done); + + runner.makeRequest( + 'post', + '/test-post', + { + 'Content-Type': 'text/plain', }, - }, - }) - .start(done); + 'some plain text', + ); + }); + + test('correctly captures text buffer request data', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + event: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/octet-stream', + }, + data: 'some plain text in buffer', + }, + }, + }) + .start(done); + + runner.makeRequest( + 'post', + '/test-post', + { 'Content-Type': 'application/octet-stream' }, + Buffer.from('some plain text in buffer'), + ); + }); + + test('correctly captures non-text buffer request data', done => { + const runner = createRunner(__dirname, 'server.ts') + .expect({ + event: { + transaction: 'POST /test-post', + request: { + url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/), + method: 'POST', + headers: { + 'user-agent': expect.stringContaining(''), + 'content-type': 'application/octet-stream', + }, + // This is some non-ascii string representation + data: expect.any(String), + }, + }, + }) + .start(done); + + const body = new Uint8Array([1, 2, 3, 4, 5]).buffer; - runner.makeRequest('get', '/test/isolationScope/1'); + runner.makeRequest('post', '/test-post', { 'Content-Type': 'application/octet-stream' }, body); + }); + }); }); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index bde5bd06cd21..b586c1f5bad0 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -484,7 +484,7 @@ export function createRunner(...paths: string[]) { method: 'get' | 'post', path: string, headers: Record = {}, - data?: any, // axios accept any as data + data?: unknown, ): Promise { try { await waitFor(() => scenarioServerPort !== undefined); diff --git a/packages/core/src/integrations/requestdata.ts b/packages/core/src/integrations/requestdata.ts index f7846dec6fea..80b01ea4861f 100644 --- a/packages/core/src/integrations/requestdata.ts +++ b/packages/core/src/integrations/requestdata.ts @@ -1,5 +1,6 @@ import type { IntegrationFn } from '@sentry/types'; import type { AddRequestDataToEventOptions, TransactionNamingScheme } from '@sentry/utils'; +import { addNormalizedRequestDataToEvent } from '@sentry/utils'; import { addRequestDataToEvent } from '@sentry/utils'; import { defineIntegration } from '../integration'; @@ -73,15 +74,25 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) = // that's happened, it will be easier to add this logic in without worrying about unexpected side effects.) const { sdkProcessingMetadata = {} } = event; - const req = sdkProcessingMetadata.request; + const { request, normalizedRequest } = sdkProcessingMetadata; - if (!req) { + const addRequestDataOptions = convertReqDataIntegrationOptsToAddReqDataOpts(_options); + + // If this is set, it takes precedence over the plain request object + if (normalizedRequest) { + // Some other data is not available in standard HTTP requests, but can sometimes be augmented by e.g. Express or Next.js + const ipAddress = request ? request.ip || (request.socket && request.socket.remoteAddress) : undefined; + const user = request ? request.user : undefined; + + addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress, user }, addRequestDataOptions); return event; } - const addRequestDataOptions = convertReqDataIntegrationOptsToAddReqDataOpts(_options); + if (!request) { + return event; + } - return addRequestDataToEvent(event, req, addRequestDataOptions); + return addRequestDataToEvent(event, request, addRequestDataOptions); }, }; }) satisfies IntegrationFn; diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index d6796aa866e5..c28fd6f78f6e 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -13,14 +13,16 @@ import { setCapturedScopesOnSpan, } from '@sentry/core'; import { getClient } from '@sentry/opentelemetry'; -import type { IntegrationFn, SanitizedRequestData } from '@sentry/types'; +import type { IntegrationFn, PolymorphicRequest, Request, SanitizedRequestData } from '@sentry/types'; import { getBreadcrumbLogLevelFromHttpStatusCode, getSanitizedUrlString, + logger, parseUrl, stripUrlQueryAndFragment, } from '@sentry/utils'; +import { DEBUG_BUILD } from '../debug-build'; import type { NodeClient } from '../sdk/client'; import { setIsolationScope } from '../sdk/scope'; import type { HTTPModuleRequestIncomingMessage } from '../transports/http-module'; @@ -153,8 +155,27 @@ export const instrumentHttp = Object.assign( const isolationScope = (scopes.isolationScope || getIsolationScope()).clone(); const scope = scopes.scope || getCurrentScope(); + const headers = req.headers; + const host = headers.host || ''; + const protocol = req.socket && (req.socket as { encrypted?: boolean }).encrypted ? 'https' : 'http'; + const originalUrl = req.url || ''; + const absoluteUrl = originalUrl.startsWith(protocol) ? originalUrl : `${protocol}://${host}${originalUrl}`; + + // This is non-standard, but may be set on e.g. Next.js or Express requests + const cookies = (req as PolymorphicRequest).cookies; + + const normalizedRequest: Request = { + url: absoluteUrl, + method: req.method, + query_string: extractQueryParams(req), + headers: headersToDict(req.headers), + cookies, + }; + + patchRequestToCaptureBody(req, normalizedRequest); + // Update the isolation scope, isolate this request - isolationScope.setSDKProcessingMetadata({ request: req }); + isolationScope.setSDKProcessingMetadata({ request: req, normalizedRequest }); const client = getClient(); if (client && client.getOptions().autoSessionTracking) { @@ -310,3 +331,128 @@ function isKnownPrefetchRequest(req: HTTPModuleRequestIncomingMessage): boolean // Currently only handles Next.js prefetch requests but may check other frameworks in the future. return req.headers['next-router-prefetch'] === '1'; } + +/** + * This method patches the request object to capture the body. + * Instead of actually consuming the streamed body ourselves, which has potential side effects, + * we monkey patch `req.on('data')` to intercept the body chunks. + * This way, we only read the body if the user also consumes the body, ensuring we do not change any behavior in unexpected ways. + */ +function patchRequestToCaptureBody(req: HTTPModuleRequestIncomingMessage, normalizedRequest: Request): void { + const chunks: Buffer[] = []; + + /** + * We need to keep track of the original callbacks, in order to be able to remove listeners again. + * Since `off` depends on having the exact same function reference passed in, we need to be able to map + * original listeners to our wrapped ones. + */ + const callbackMap = new WeakMap(); + + try { + // eslint-disable-next-line @typescript-eslint/unbound-method + req.on = new Proxy(req.on, { + apply: (target, thisArg, args: Parameters) => { + const [event, listener, ...restArgs] = args; + + if (event === 'data') { + const callback = new Proxy(listener, { + apply: (target, thisArg, args: Parameters) => { + const chunk = args[0]; + chunks.push(chunk); + return Reflect.apply(target, thisArg, args); + }, + }); + + callbackMap.set(listener, callback); + + return Reflect.apply(target, thisArg, [event, callback, ...restArgs]); + } + + if (event === 'end') { + const callback = new Proxy(listener, { + apply: (target, thisArg, args) => { + try { + const body = Buffer.concat(chunks).toString('utf-8'); + + // We mutate the passed in normalizedRequest and add the body to it + if (body) { + normalizedRequest.data = body; + } + } catch { + // ignore errors here + } + + return Reflect.apply(target, thisArg, args); + }, + }); + + callbackMap.set(listener, callback); + + return Reflect.apply(target, thisArg, [event, callback, ...restArgs]); + } + + return Reflect.apply(target, thisArg, args); + }, + }); + + // Ensure we also remove callbacks correctly + // eslint-disable-next-line @typescript-eslint/unbound-method + req.off = new Proxy(req.off, { + apply: (target, thisArg, args: Parameters) => { + const [, listener] = args; + + const callback = callbackMap.get(listener); + if (callback) { + callbackMap.delete(listener); + + const modifiedArgs = args.slice(); + modifiedArgs[1] = callback; + return Reflect.apply(target, thisArg, modifiedArgs); + } + + return Reflect.apply(target, thisArg, args); + }, + }); + } catch { + // ignore errors if we can't patch stuff + } +} + +function extractQueryParams(req: IncomingMessage): string | undefined { + // url (including path and query string): + let originalUrl = req.url || ''; + + if (!originalUrl) { + return; + } + + // The `URL` constructor can't handle internal URLs of the form `/some/path/here`, so stick a dummy protocol and + // hostname on the beginning. Since the point here is just to grab the query string, it doesn't matter what we use. + if (originalUrl.startsWith('/')) { + originalUrl = `http://dogs.are.great${originalUrl}`; + } + + try { + const queryParams = new URL(originalUrl).search.slice(1); + return queryParams.length ? queryParams : undefined; + } catch { + return undefined; + } +} + +function headersToDict(reqHeaders: Record): Record { + const headers: Record = {}; + + try { + Object.entries(reqHeaders).forEach(([key, value]) => { + if (typeof value === 'string') { + headers[key] = value; + } + }); + } catch (e) { + DEBUG_BUILD && + logger.warn('Sentry failed extracting headers from a request object. If you see this, please file an issue.'); + } + + return headers; +} diff --git a/packages/node/src/transports/http-module.ts b/packages/node/src/transports/http-module.ts index f5cbe6fd35f9..65bf99349b10 100644 --- a/packages/node/src/transports/http-module.ts +++ b/packages/node/src/transports/http-module.ts @@ -10,7 +10,8 @@ export type HTTPModuleRequestOptions = HTTPRequestOptions | HTTPSRequestOptions export interface HTTPModuleRequestIncomingMessage { headers: IncomingHttpHeaders; statusCode?: number; - on(event: 'data' | 'end', listener: () => void): void; + on(event: 'data' | 'end', listener: (chunk: Buffer) => void): void; + off(event: 'data' | 'end', listener: (chunk: Buffer) => void): void; setEncoding(encoding: string): void; } diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index 29e8fc123b16..20c67b8857fe 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -101,7 +101,7 @@ export type ProfileItem = BaseEnvelopeItem; export type ProfileChunkItem = BaseEnvelopeItem; export type SpanItem = BaseEnvelopeItem>; -export type EventEnvelopeHeaders = { event_id: string; sent_at: string; trace?: DynamicSamplingContext }; +export type EventEnvelopeHeaders = { event_id: string; sent_at: string; trace?: Partial }; type SessionEnvelopeHeaders = { sent_at: string }; type CheckInEnvelopeHeaders = { trace?: DynamicSamplingContext }; type ClientReportEnvelopeHeaders = BaseEnvelopeHeaders; diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts index c8c16b8ce514..bdccaa2b58dd 100644 --- a/packages/types/src/event.ts +++ b/packages/types/src/event.ts @@ -2,13 +2,15 @@ import type { Attachment } from './attachment'; import type { Breadcrumb } from './breadcrumb'; import type { Contexts } from './context'; import type { DebugMeta } from './debugMeta'; +import type { DynamicSamplingContext } from './envelope'; import type { Exception } from './exception'; import type { Extras } from './extra'; import type { Measurements } from './measurement'; import type { Mechanism } from './mechanism'; import type { Primitive } from './misc'; +import type { PolymorphicRequest } from './polymorphics'; import type { Request } from './request'; -import type { CaptureContext } from './scope'; +import type { CaptureContext, Scope } from './scope'; import type { SdkInfo } from './sdkinfo'; import type { SeverityLevel } from './severity'; import type { MetricSummary, SpanJSON } from './span'; @@ -51,7 +53,15 @@ export interface Event { measurements?: Measurements; debug_meta?: DebugMeta; // A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get sent to Sentry - sdkProcessingMetadata?: { [key: string]: any }; + // Note: This is considered internal and is subject to change in minors + sdkProcessingMetadata?: { [key: string]: unknown } & { + request?: PolymorphicRequest; + normalizedRequest?: Request; + dynamicSamplingContext?: Partial; + capturedSpanScope?: Scope; + capturedSpanIsolationScope?: Scope; + spanCountBeforeProcessing?: number; + }; transaction_info?: { source: TransactionSource; }; diff --git a/packages/utils/src/requestdata.ts b/packages/utils/src/requestdata.ts index a4eae547edb1..0e09ecb8cb31 100644 --- a/packages/utils/src/requestdata.ts +++ b/packages/utils/src/requestdata.ts @@ -1,7 +1,9 @@ +/* eslint-disable max-lines */ import type { Event, ExtractedNodeRequestData, PolymorphicRequest, + Request, TransactionSource, WebFetchHeaders, WebFetchRequest, @@ -12,6 +14,7 @@ import { DEBUG_BUILD } from './debug-build'; import { isPlainObject, isString } from './is'; import { logger } from './logger'; import { normalize } from './normalize'; +import { truncate } from './string'; import { stripUrlQueryAndFragment } from './url'; import { getClientIPAddress, ipHeaderNames } from './vendor/getIpAddress'; @@ -228,14 +231,27 @@ export function extractRequestData( if (method === 'GET' || method === 'HEAD') { break; } + // NOTE: As of v8, request is (unless a user sets this manually) ALWAYS a http request + // Which does not have a body by default + // However, in our http instrumentation, we patch the request to capture the body and store it on the + // request as `.body` anyhow + // In v9, we may update requestData to only work with plain http requests // body data: // express, koa, nextjs: req.body // // when using node by itself, you have to read the incoming stream(see // https://nodejs.dev/learn/get-http-request-body-data-using-nodejs); if a user is doing that, we can't know // where they're going to store the final result, so they'll have to capture this data themselves - if (req.body !== undefined) { - requestData.data = isString(req.body) ? req.body : JSON.stringify(normalize(req.body)); + const body = req.body; + if (body !== undefined) { + const stringBody: string = isString(body) + ? body + : isPlainObject(body) + ? JSON.stringify(normalize(body)) + : truncate(`${body}`, 1024); + if (stringBody) { + requestData.data = stringBody; + } } break; } @@ -250,6 +266,61 @@ export function extractRequestData( return requestData; } +/** + * Add already normalized request data to an event. + * This mutates the passed in event. + */ +export function addNormalizedRequestDataToEvent( + event: Event, + req: Request, + // This is non-standard data that is not part of the regular HTTP request + additionalData: { ipAddress?: string; user?: Record }, + options: AddRequestDataToEventOptions, +): void { + const include = { + ...DEFAULT_INCLUDES, + ...(options && options.include), + }; + + if (include.request) { + const includeRequest = Array.isArray(include.request) ? [...include.request] : [...DEFAULT_REQUEST_INCLUDES]; + if (include.ip) { + includeRequest.push('ip'); + } + + const extractedRequestData = extractNormalizedRequestData(req, { include: includeRequest }); + + event.request = { + ...event.request, + ...extractedRequestData, + }; + } + + if (include.user) { + const extractedUser = + additionalData.user && isPlainObject(additionalData.user) + ? extractUserData(additionalData.user, include.user) + : {}; + + if (Object.keys(extractedUser).length) { + event.user = { + ...event.user, + ...extractedUser, + }; + } + } + + if (include.ip) { + const ip = (req.headers && getClientIPAddress(req.headers)) || additionalData.ipAddress; + if (ip) { + event.user = { + ...event.user, + ip_address: ip, + }; + } + } +} + /** * Add data from the given request to the given event * @@ -374,3 +445,51 @@ export function winterCGRequestToRequestData(req: WebFetchRequest): PolymorphicR headers, }; } + +function extractNormalizedRequestData(normalizedRequest: Request, { include }: { include: string[] }): Request { + const includeKeys = include ? (Array.isArray(include) ? include : DEFAULT_REQUEST_INCLUDES) : []; + + const requestData: Request = {}; + + const { headers } = normalizedRequest; + + if (includeKeys.includes('headers')) { + requestData.headers = headers; + + // Remove the Cookie header in case cookie data should not be included in the event + if (!include.includes('cookies')) { + delete (headers as { cookie?: string }).cookie; + } + + // Remove IP headers in case IP data should not be included in the event + if (!include.includes('ip')) { + ipHeaderNames.forEach(ipHeaderName => { + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete (headers as Record)[ipHeaderName]; + }); + } + } + + if (includeKeys.includes('method')) { + requestData.method = normalizedRequest.method; + } + + if (includeKeys.includes('url')) { + requestData.url = normalizedRequest.url; + } + + if (includeKeys.includes('cookies')) { + const cookies = normalizedRequest.cookies || (headers && headers.cookie ? parseCookie(headers.cookie) : undefined); + requestData.cookies = cookies; + } + + if (includeKeys.includes('query_string')) { + requestData.query_string = normalizedRequest.query_string; + } + + if (includeKeys.includes('data')) { + requestData.data = normalizedRequest.data; + } + + return requestData; +} diff --git a/yarn.lock b/yarn.lock index 113e665c1752..770b98e35276 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12907,6 +12907,24 @@ body-parser@1.20.2: type-is "~1.6.18" unpipe "1.0.0" +body-parser@^1.20.3: + version "1.20.3" + resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.3.tgz#1953431221c6fb5cd63c4b36d53fab0928e548c6" + integrity sha512-7rAxByjUMqQ3/bHJy7D6OGXvx/MMc4IqBn/X0fcM1QUcAItpZrBEYhWGem+tzXH90c+G01ypMcYJBO9Y30203g== + dependencies: + bytes "3.1.2" + content-type "~1.0.5" + debug "2.6.9" + depd "2.0.0" + destroy "1.2.0" + http-errors "2.0.0" + iconv-lite "0.4.24" + on-finished "2.4.1" + qs "6.13.0" + raw-body "2.5.2" + type-is "~1.6.18" + unpipe "1.0.0" + body@^5.1.0: version "5.1.0" resolved "https://registry.yarnpkg.com/body/-/body-5.1.0.tgz#e4ba0ce410a46936323367609ecb4e6553125069" @@ -26036,6 +26054,11 @@ object-inspect@^1.12.2, object-inspect@^1.9.0: resolved "https://registry.yarnpkg.com/object-inspect/-/object-inspect-1.12.2.tgz#c0641f26394532f28ab8d796ab954e43c009a8ea" integrity sha512-z+cPxW0QGUp0mcqcsgQyLVRDoXFQbXOwBaqyF7VIgI4TWNQsDHrBpUQslRmIfAoYWdYzs6UlKJtB2XJpTaNSpQ== +object-inspect@^1.13.1: + version "1.13.2" + resolved "https://registry.yarnpkg.com/object-inspect/-/object-inspect-1.13.2.tgz#dea0088467fb991e67af4058147a24824a3043ff" + integrity sha512-IRZSRuzJiynemAXPYtPe5BoI/RESNYR7TYm50MC5Mqbd3Jmw5y790sErYw3V6SryFJD64b74qQQs9wn5Bg/k3g== + object-is@^1.0.1: version "1.1.5" resolved "https://registry.yarnpkg.com/object-is/-/object-is-1.1.5.tgz#b9deeaa5fc7f1846a0faecdceec138e5778f53ac" @@ -28432,6 +28455,13 @@ qs@6.11.0, qs@^6.4.0: dependencies: side-channel "^1.0.4" +qs@6.13.0: + version "6.13.0" + resolved "https://registry.yarnpkg.com/qs/-/qs-6.13.0.tgz#6ca3bd58439f7e245655798997787b0d88a51906" + integrity sha512-+38qI9SOr8tfZ4QmJNplMUxqjbe7LKvvZgWdExBOmd+egZTtjLB67Gu0HRX3u/XOq7UU2Nx6nsjvS16Z9uwfpg== + dependencies: + side-channel "^1.0.6" + query-string@^4.2.2: version "4.3.4" resolved "https://registry.yarnpkg.com/query-string/-/query-string-4.3.4.tgz#bbb693b9ca915c232515b228b1a02b609043dbeb" @@ -30347,6 +30377,16 @@ side-channel@^1.0.4: get-intrinsic "^1.0.2" object-inspect "^1.9.0" +side-channel@^1.0.6: + version "1.0.6" + resolved "https://registry.yarnpkg.com/side-channel/-/side-channel-1.0.6.tgz#abd25fb7cd24baf45466406b1096b7831c9215f2" + integrity sha512-fDW/EZ6Q9RiO8eFG8Hj+7u/oW+XrPTIChwCOM2+th2A6OblDtYYIpve9m+KvI9Z4C9qSEXlaGR6bTEYHReuglA== + dependencies: + call-bind "^1.0.7" + es-errors "^1.3.0" + get-intrinsic "^1.2.4" + object-inspect "^1.13.1" + sift@13.5.2: version "13.5.2" resolved "https://registry.yarnpkg.com/sift/-/sift-13.5.2.tgz#24a715e13c617b086166cd04917d204a591c9da6"