Skip to content

Commit

Permalink
feat(serializers): avoid implicit sanitization
Browse files Browse the repository at this point in the history
When serializers are defined for HTTP Request or HTTP Response, do not
run the correspondent `stdSerializers` before calling the provided
serializer functions.

ISSUE #1991
  • Loading branch information
gerardo-lima-moonfare committed Nov 9, 2024
1 parent 11c3d86 commit f01ca88
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 38 deletions.
16 changes: 15 additions & 1 deletion docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -515,13 +515,27 @@ Default: `'msg'`

The string key for the 'message' in the JSON object.

<a id=opt-messagekey></a>
<a id=opt-errorkey></a>
#### `errorKey` (String)

Default: `'err'`

The string key for the 'error' in the JSON object.

<a id=opt-requestkey></a>
#### `requestKey` (String)

Default: `'req'`

The string key for the 'Request' in the JSON object.

<a id=opt-responsekey></a>
#### `responseKey` (String)

Default: `'res'`

The string key for the 'Response' in the JSON object.

<a id=opt-nestedkey></a>
#### `nestedKey` (String)

Expand Down
9 changes: 9 additions & 0 deletions lib/proto.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/* eslint no-prototype-builtins: 0 */

const { EventEmitter } = require('node:events')
const { IncomingMessage, ServerResponse} = require('node:http')
const {
lsCacheSym,
levelValSym,
Expand All @@ -20,6 +21,8 @@ const {
serializersSym,
formattersSym,
errorKeySym,
requestKeySym,
responseKeySym,
messageKeySym,
useOnlyCustomLevelsSym,
needsMetadataGsym,
Expand Down Expand Up @@ -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
Expand All @@ -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]) {
Expand Down
6 changes: 5 additions & 1 deletion lib/symbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -70,5 +72,7 @@ module.exports = {
hooksSym,
nestedKeyStrSym,
mixinMergeStrategySym,
msgPrefixSym
msgPrefixSym,
responseKeySym,
requestKeySym,
}
15 changes: 8 additions & 7 deletions lib/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ const {
formattersSym,
messageKeySym,
errorKeySym,
requestKeySym,
responseKeySym,
nestedKeyStrSym,
msgPrefixSym
} = require('./symbols')
Expand All @@ -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]
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 10 additions & 0 deletions pino.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 13 additions & 1 deletion pino.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ const {
formatOptsSym,
messageKeySym,
errorKeySym,
requestKeySym,
responseKeySym,
nestedKeySym,
mixinSym,
levelCompSym,
Expand All @@ -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) {
Expand Down Expand Up @@ -95,6 +103,8 @@ function pino (...args) {
timestamp,
messageKey,
errorKey,
requestKey,
responseKey,
nestedKey,
base,
name,
Expand Down Expand Up @@ -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)}:{` : '',
Expand Down
46 changes: 18 additions & 28 deletions test/http.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit f01ca88

Please sign in to comment.