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/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-koa/tests/transactions.test.ts index 4c52c932e7b4..1197575d1a96 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,129 @@ 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({ + cookies: {}, + 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 e016bf6ee93b..5b62e3a8e996 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.7.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..0b56d354759c 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,106 @@ 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', { data: { 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', { + headers: { 'Content-Type': 'text/plain' }, + data: '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', { + headers: { 'Content-Type': 'application/octet-stream' }, + data: 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', { + headers: { 'Content-Type': 'application/octet-stream' }, + data: 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..fdd63ad4aa4b 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,129 @@ 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', { data: { 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', { + headers: { + 'Content-Type': 'text/plain', }, - }, - }) - .start(done); + data: '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', { + headers: { 'Content-Type': 'application/octet-stream' }, + data: 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', { headers: { 'Content-Type': 'application/octet-stream' }, data: body }); + }); + }); }); diff --git a/packages/core/src/integrations/requestdata.ts b/packages/core/src/integrations/requestdata.ts index f7846dec6fea..9475973d8e2c 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,26 @@ 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); + // TODO(v9): Eventually we can remove this fallback branch and only rely on the normalizedRequest above + if (!request) { + return event; + } - return addRequestDataToEvent(event, req, addRequestDataOptions); + return addRequestDataToEvent(event, request, addRequestDataOptions); }, }; }) satisfies IntegrationFn; diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index 090c0783507a..6b6fe8aaad40 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -1,18 +1,20 @@ import type * as http from 'node:http'; -import type { RequestOptions } from 'node:http'; +import type { IncomingMessage, RequestOptions } from 'node:http'; import type * as https from 'node:https'; import { VERSION } from '@opentelemetry/core'; import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; import { getRequestInfo } from '@opentelemetry/instrumentation-http'; import { addBreadcrumb, getClient, getIsolationScope, withIsolationScope } from '@sentry/core'; -import type { SanitizedRequestData } from '@sentry/types'; +import type { 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 { getRequestUrl } from '../../utils/getRequestUrl'; @@ -39,6 +41,9 @@ type SentryHttpInstrumentationOptions = InstrumentationConfig & { ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; }; +// We only want to capture request bodies up to 1mb. +const MAX_BODY_BYTE_LENGTH = 1024 * 1024; + /** * This custom HTTP instrumentation is used to isolate incoming requests and annotate them with additional information. * It does not emit any spans. @@ -128,8 +133,27 @@ export class SentryHttpInstrumentation extends InstrumentationBase'; + const protocol = request.socket && (request.socket as { encrypted?: boolean }).encrypted ? 'https' : 'http'; + const originalUrl = request.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 = (request as PolymorphicRequest).cookies; + + const normalizedRequest: Request = { + url: absoluteUrl, + method: request.method, + query_string: extractQueryParams(request), + headers: headersToDict(request.headers), + cookies, + }; + + patchRequestToCaptureBody(request, normalizedRequest); + // Update the isolation scope, isolate this request - isolationScope.setSDKProcessingMetadata({ request }); + isolationScope.setSDKProcessingMetadata({ request, normalizedRequest }); const client = getClient(); if (client && client.getOptions().autoSessionTracking) { @@ -316,3 +340,135 @@ function getBreadcrumbData(request: http.ClientRequest): Partial acc + chunk.byteLength, 0); + } + + /** + * 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) => { + // If we have already read more than the max body length, we stop addiing chunks + // To avoid growing the memory indefinitely if a respons is e.g. streamed + if (getChunksSize() < MAX_BODY_BYTE_LENGTH) { + const chunk = args[0] as Buffer; + chunks.push(chunk); + } else if (DEBUG_BUILD) { + logger.log( + `Dropping request body chunk because it maximum body length of ${MAX_BODY_BYTE_LENGTH}b is exceeded.`, + ); + } + + 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 { + // req.url is path and query string + if (!req.url) { + return; + } + + try { + // The `URL` constructor can't handle internal URLs of the form `/some/path/here`, so stick a dummy protocol and + // hostname as the base. Since the point here is just to grab the query string, it doesn't matter what we use. + const queryParams = new URL(req.url, 'http://dogs.are.great').search.slice(1); + return queryParams.length ? queryParams : undefined; + } catch { + return undefined; + } +} + +function headersToDict(reqHeaders: Record): Record { + const headers: Record = Object.create(null); + + 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/types/src/request.ts b/packages/types/src/request.ts index 3c04a788ded9..f0c2bdb268ca 100644 --- a/packages/types/src/request.ts +++ b/packages/types/src/request.ts @@ -1,4 +1,7 @@ -/** Request data included in an event as sent to Sentry */ +/** + * Request data included in an event as sent to Sentry. + * TODO(v9): Rename this to avoid confusion, because Request is also a native type. + */ export interface Request { url?: string; method?: string; diff --git a/packages/utils/src/requestdata.ts b/packages/utils/src/requestdata.ts index a4eae547edb1..edffc6f67da7 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,50 @@ 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.headers }; + + 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 6b1ae8520693..01efab263fca 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12973,6 +12973,24 @@ body-parser@1.20.3, body-parser@^1.18.3, body-parser@^1.19.0: 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" @@ -28336,6 +28354,13 @@ 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"