From f01ca8853b5f938ff13232c5a2cec7db62a53ec3 Mon Sep 17 00:00:00 2001 From: Gerardo Lima Date: Sat, 9 Nov 2024 18:55:35 +0000 Subject: [PATCH] feat(serializers): avoid implicit sanitization When serializers are defined for HTTP Request or HTTP Response, do not run the correspondent `stdSerializers` before calling the provided serializer functions. ISSUE https://github.com/pinojs/pino/issues/1991 --- docs/api.md | 16 +++++++++++++++- lib/proto.js | 9 +++++++++ lib/symbols.js | 6 +++++- lib/tools.js | 15 ++++++++------- pino.d.ts | 10 ++++++++++ pino.js | 14 +++++++++++++- test/http.test.js | 46 ++++++++++++++++++---------------------------- 7 files changed, 78 insertions(+), 38 deletions(-) diff --git a/docs/api.md b/docs/api.md index b3d5a0b6a..e941ad3d5 100644 --- a/docs/api.md +++ b/docs/api.md @@ -515,13 +515,27 @@ Default: `'msg'` The string key for the 'message' in the JSON object. - + #### `errorKey` (String) Default: `'err'` The string key for the 'error' in the JSON object. + +#### `requestKey` (String) + +Default: `'req'` + +The string key for the 'Request' in the JSON object. + + +#### `responseKey` (String) + +Default: `'res'` + +The string key for the 'Response' in the JSON object. + #### `nestedKey` (String) diff --git a/lib/proto.js b/lib/proto.js index ef49a7122..8519a6d49 100644 --- a/lib/proto.js +++ b/lib/proto.js @@ -3,6 +3,7 @@ /* eslint no-prototype-builtins: 0 */ const { EventEmitter } = require('node:events') +const { IncomingMessage, ServerResponse} = require('node:http') const { lsCacheSym, levelValSym, @@ -20,6 +21,8 @@ const { serializersSym, formattersSym, errorKeySym, + requestKeySym, + responseKeySym, messageKeySym, useOnlyCustomLevelsSym, needsMetadataGsym, @@ -182,6 +185,8 @@ function write (_obj, msg, num) { const t = this[timeSym]() const mixin = this[mixinSym] const errorKey = this[errorKeySym] + const requestKey = this[requestKeySym] + const responseKey = this[responseKeySym] const messageKey = this[messageKeySym] const mixinMergeStrategy = this[mixinMergeStrategySym] || defaultMixinMergeStrategy let obj @@ -193,6 +198,10 @@ function write (_obj, msg, num) { if (msg === undefined) { msg = _obj.message } + } else if (_obj instanceof IncomingMessage) { + obj = { [requestKey]: _obj } + } else if (_obj instanceof ServerResponse) { + obj = { [responseKey]: _obj } } else { obj = _obj if (msg === undefined && _obj[messageKey] === undefined && _obj[errorKey]) { diff --git a/lib/symbols.js b/lib/symbols.js index 69f1a9d25..050fe912a 100644 --- a/lib/symbols.js +++ b/lib/symbols.js @@ -29,6 +29,8 @@ const nestedKeySym = Symbol('pino.nestedKey') const nestedKeyStrSym = Symbol('pino.nestedKeyStr') const mixinMergeStrategySym = Symbol('pino.mixinMergeStrategy') const msgPrefixSym = Symbol('pino.msgPrefix') +const responseKeySym = Symbol('pino.responseKey') +const requestKeySym = Symbol('pino.requestKey') const wildcardFirstSym = Symbol('pino.wildcardFirst') @@ -70,5 +72,7 @@ module.exports = { hooksSym, nestedKeyStrSym, mixinMergeStrategySym, - msgPrefixSym + msgPrefixSym, + responseKeySym, + requestKeySym, } diff --git a/lib/tools.js b/lib/tools.js index 5f68f588e..f3c746d8c 100644 --- a/lib/tools.js +++ b/lib/tools.js @@ -21,6 +21,8 @@ const { formattersSym, messageKeySym, errorKeySym, + requestKeySym, + responseKeySym, nestedKeyStrSym, msgPrefixSym } = require('./symbols') @@ -40,13 +42,6 @@ function genLog (level, hook) { function LOG (o, ...n) { if (typeof o === 'object') { let msg = o - if (o !== null) { - if (o.method && o.headers && o.socket) { - o = mapHttpRequest(o) - } else if (typeof o.setHeader === 'function') { - o = mapHttpResponse(o) - } - } let formatParams if (msg === null && n.length === 0) { formatParams = [null] @@ -113,6 +108,8 @@ function asJson (obj, msg, num, time) { const formatters = this[formattersSym] const messageKey = this[messageKeySym] const errorKey = this[errorKeySym] + const responseKey = this[responseKeySym] + const requestKey = this[requestKeySym] let data = this[lsCacheSym][num] + time // we need the child bindings added to the output first so instance logged @@ -132,6 +129,10 @@ function asJson (obj, msg, num, time) { value = serializers[key](value) } else if (key === errorKey && serializers.err) { value = serializers.err(value) + } else if (key === requestKey && serializers.req) { + value = serializers.req(value) + } else if (key === responseKey && serializers.res) { + value = serializers.res(value) } const stringifier = stringifiers[key] || wildcardStringifier diff --git a/pino.d.ts b/pino.d.ts index f8cad6f07..684641886 100644 --- a/pino.d.ts +++ b/pino.d.ts @@ -408,6 +408,14 @@ declare namespace pino { * The string key for the 'error' in the JSON object. Default: "err". */ errorKey?: string; + /** + * The string key for the 'Request' in the JSON object. Default: "req". + */ + requestKey?: string; + /** + * The string key for the 'Response' in the JSON object. Default: "res". + */ + responseKey?: string; /** * The string key to place any logged object under. */ @@ -741,6 +749,8 @@ declare namespace pino { readonly formatOptsSym: unique symbol; readonly messageKeySym: unique symbol; readonly errorKeySym: unique symbol; + readonly responseKeySym: unique symbol; + readonly requestKeySym: unique symbol; readonly nestedKeySym: unique symbol; readonly wildcardFirstSym: unique symbol; readonly needsMetadataGsym: unique symbol; diff --git a/pino.js b/pino.js index 740b97daa..a424be9b4 100644 --- a/pino.js +++ b/pino.js @@ -35,6 +35,8 @@ const { formatOptsSym, messageKeySym, errorKeySym, + requestKeySym, + responseKeySym, nestedKeySym, mixinSym, levelCompSym, @@ -49,17 +51,23 @@ const { epochTime, nullTime } = time const { pid } = process const hostname = os.hostname() const defaultErrorSerializer = stdSerializers.err +const defaultRequestSerializer = stdSerializers.req +const defaultResponseSerializer = stdSerializers.res const defaultOptions = { level: 'info', levelComparison: SORTING_ORDER.ASC, levels: DEFAULT_LEVELS, messageKey: 'msg', errorKey: 'err', + requestKey: 'req', + responseKey: 'res', nestedKey: null, enabled: true, base: { pid, hostname }, serializers: Object.assign(Object.create(null), { - err: defaultErrorSerializer + err: defaultErrorSerializer, + req: defaultRequestSerializer, + res: defaultResponseSerializer, }), formatters: Object.assign(Object.create(null), { bindings (bindings) { @@ -95,6 +103,8 @@ function pino (...args) { timestamp, messageKey, errorKey, + requestKey, + responseKey, nestedKey, base, name, @@ -182,6 +192,8 @@ function pino (...args) { [formatOptsSym]: formatOpts, [messageKeySym]: messageKey, [errorKeySym]: errorKey, + [requestKeySym]: requestKey, + [responseKeySym]: responseKey, [nestedKeySym]: nestedKey, // protect against injection [nestedKeyStrSym]: nestedKey ? `,${JSON.stringify(nestedKey)}:{` : '', diff --git a/test/http.test.js b/test/http.test.js index 650868ffe..c6eace883 100644 --- a/test/http.test.js +++ b/test/http.test.js @@ -44,33 +44,27 @@ test('http request support', async ({ ok, same, error, teardown }) => { server.close() }) -test('http request support via serializer', async ({ ok, same, error, teardown }) => { +test('http request support via serializer', async ({ error, match }) => { let originalReq const instance = pino({ serializers: { - req: pino.stdSerializers.req + req: (req) => req.arbitraryProperty, } - }, sink((chunk, enc) => { - ok(new Date(chunk.time) <= new Date(), 'time is greater than Date.now()') - delete chunk.time - same(chunk, { + }, sink((chunk, _enc) => { + match(chunk, { pid, hostname, level: 30, msg: 'my request', - req: { - method: originalReq.method, - url: originalReq.url, - headers: originalReq.headers, - remoteAddress: originalReq.socket.remoteAddress, - remotePort: originalReq.socket.remotePort - } + req: originalReq.arbitraryProperty, }) })) const server = http.createServer(function (req, res) { + req.arbitraryProperty = Math.random() + originalReq = req - instance.info({ req }, 'my request') + instance.info(req, 'my request') res.end('hello') }) server.unref() @@ -160,34 +154,30 @@ test('http response support', async ({ ok, same, error, teardown }) => { server.close() }) -test('http response support via a serializer', async ({ ok, same, error, teardown }) => { +test('http response support via a serializer', async ({ match, error }) => { + let originalRes const instance = pino({ serializers: { - res: pino.stdSerializers.res + res: (res) => res.arbitraryProperty, } }, sink((chunk, enc) => { - ok(new Date(chunk.time) <= new Date(), 'time is greater than Date.now()') - delete chunk.time - same(chunk, { + match(chunk, { pid, hostname, level: 30, msg: 'my response', - res: { - statusCode: 200, - headers: { - 'x-single': 'y', - 'x-multi': [1, 2] - } - } + res: originalRes.arbitraryProperty, }) })) - const server = http.createServer(function (req, res) { + const server = http.createServer(function (_req, res) { + res.arbitraryProperty = Math.random() + + originalRes = res res.setHeader('x-single', 'y') res.setHeader('x-multi', [1, 2]) res.end('hello') - instance.info({ res }, 'my response') + instance.info(res, 'my response') }) server.unref()