Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(serializers): avoid implicit sanitization #2081

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
30 changes: 29 additions & 1 deletion docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
* [pino.stdTimeFunctions](#pino-stdtimefunctions)
* [pino.symbols](#pino-symbols)
* [pino.version](#pino-version)
* [pino.futures](#pino-version)
* [Interfaces](#interfaces)
* [MultiStreamRes](#multistreamres)
* [StreamEntry](#streamentry)
Expand Down Expand Up @@ -515,13 +516,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 Expand Up @@ -600,6 +615,19 @@ const parent = require('pino')({ onChild: (instance) => {
parent.child(bindings)
```

<a id=opt-future></a>
#### `future` (Object)

The `future` object contains _opt-in_ flags specific to a Pino major version. These flags are used to change behavior,
anticipating breaking-changes that will be introduced in the next major version.
```js
const parent = require('pino')({
future: {
skipUnconditionalStdSerializers: true
}
})
```


<a id="destination"></a>
### `destination` (Number | String | Object | DestinationStream | SonicBoomOpts | WritableStream)
Expand Down
21 changes: 17 additions & 4 deletions lib/deprecations.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,21 @@
'use strict'

const warning = require('process-warning')()
module.exports = warning
const warning = require('process-warning')

// const warnName = 'PinoWarning'
/**
* Future flags, specific to the current major-version of Pino. These flags allow for breaking-changes to be introduced
* on a opt-in basis, anticipating behavior of the next major-version. All future flag must be frozen as false. These
* future flags are specific to Pino major-version 9.
*/
const future = Object.freeze({
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed Object.freeze is not used for default configuration. This can be easily refactored, if avoiding this feature is intentional.

skipUnconditionalStdSerializers: false // see PINODEP010
})

// warning.create(warnName, 'PINODEP010', 'A new deprecation')
const PINODEP010 = warning.createDeprecation({ code: 'PINODEP010', message: 'Unconditional execution of standard serializers for HTTP Request and Response will be discontinued in the next major version.' })

module.exports = {
warning: {
PINODEP010
},
future
}
22 changes: 21 additions & 1 deletion lib/proto.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,17 @@ const {
serializersSym,
formattersSym,
errorKeySym,
requestKeySym,
responseKeySym,
messageKeySym,
useOnlyCustomLevelsSym,
needsMetadataGsym,
redactFmtSym,
stringifySym,
formatOptsSym,
stringifiersSym,
msgPrefixSym
msgPrefixSym,
futureSym
} = require('./symbols')
const {
getLevel,
Expand Down Expand Up @@ -88,6 +91,12 @@ function child (bindings, options) {
const formatters = this[formattersSym]
const instance = Object.create(this)

if (options.hasOwnProperty('future') === true) {
throw RangeError('future can only be set in top-level Pino')
}

instance[futureSym] = this[futureSym] // assigning future from parent is safe, as future it is immutable

if (options.hasOwnProperty('serializers') === true) {
instance[serializersSym] = Object.create(null)

Expand Down Expand Up @@ -182,8 +191,11 @@ 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
const future = this[futureSym]
let obj

if (_obj === undefined || _obj === null) {
Expand All @@ -193,6 +205,14 @@ function write (_obj, msg, num) {
if (msg === undefined) {
msg = _obj.message
}
} else if (_obj.method && _obj.headers && _obj.socket) {
if (future.skipUnconditionalStdSerializers) {
obj = { [requestKey]: _obj }
}
} else if (typeof _obj.setHeader === 'function') {
if (future.skipUnconditionalStdSerializers) {
obj = { [responseKey]: _obj }
}
} else {
obj = _obj
if (msg === undefined && _obj[messageKey] === undefined && _obj[errorKey]) {
Expand Down
8 changes: 7 additions & 1 deletion lib/symbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@ const endSym = Symbol('pino.end')
const formatOptsSym = Symbol('pino.formatOpts')
const messageKeySym = Symbol('pino.messageKey')
const errorKeySym = Symbol('pino.errorKey')
const requestKeySym = Symbol('pino.requestKey')
const responseKeySym = Symbol('pino.responseKey')
const nestedKeySym = Symbol('pino.nestedKey')
const nestedKeyStrSym = Symbol('pino.nestedKeyStr')
const mixinMergeStrategySym = Symbol('pino.mixinMergeStrategy')
const msgPrefixSym = Symbol('pino.msgPrefix')
const futureSym = Symbol('pino.future')

const wildcardFirstSym = Symbol('pino.wildcardFirst')

Expand Down Expand Up @@ -62,6 +65,8 @@ module.exports = {
formatOptsSym,
messageKeySym,
errorKeySym,
requestKeySym,
responseKeySym,
nestedKeySym,
wildcardFirstSym,
needsMetadataGsym,
Expand All @@ -70,5 +75,6 @@ module.exports = {
hooksSym,
nestedKeyStrSym,
mixinMergeStrategySym,
msgPrefixSym
msgPrefixSym,
futureSym
}
27 changes: 20 additions & 7 deletions lib/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@ const {
formattersSym,
messageKeySym,
errorKeySym,
requestKeySym,
responseKeySym,
nestedKeyStrSym,
msgPrefixSym
msgPrefixSym,
futureSym
} = require('./symbols')
const { isMainThread } = require('worker_threads')
const transport = require('./transport')
const { warning } = require('./deprecations')

function noop () {
}
Expand All @@ -40,11 +44,14 @@ 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)
if (!this[futureSym].skipUnconditionalStdSerializers) {
warning.PINODEP010()
if (o !== null) {
if (o.method && o.headers && o.socket) {
o = mapHttpRequest(o)
} else if (typeof o.setHeader === 'function') {
o = mapHttpResponse(o)
}
}
}
let formatParams
Expand Down Expand Up @@ -113,6 +120,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 +141,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 && this[futureSym].skipUnconditionalStdSerializers) {
value = serializers.req(value)
} else if (key === responseKey && serializers.res && this[futureSym].skipUnconditionalStdSerializers) {
value = serializers.res(value)
}

const stringifier = stringifiers[key] || wildcardStringifier
Expand Down Expand Up @@ -316,7 +329,7 @@ function createArgsNormalizer (defaultOptions) {
stream = transport({ caller, ...opts.transport, levels: customLevels })
}
opts = Object.assign({}, defaultOptions, opts)
opts.serializers = Object.assign({}, defaultOptions.serializers, opts.serializers)

opts.formatters = Object.assign({}, defaultOptions.formatters, opts.formatters)

if (opts.prettyPrint) {
Expand Down
28 changes: 27 additions & 1 deletion pino.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type TimeFn = () => string;
type MixinFn<CustomLevels extends string = never> = (mergeObject: object, level: number, logger:pino.Logger<CustomLevels>) => object;
type MixinMergeStrategyFn = (mergeObject: object, mixinObject: object) => object;

type CustomLevelLogger<CustomLevels extends string, UseOnlyCustomLevels extends boolean = boolean> = {
type CustomLevelLogger<CustomLevels extends string, UseOnlyCustomLevels extends boolean = boolean> = {
/**
* Define additional logging levels.
*/
Expand Down Expand Up @@ -327,6 +327,9 @@ declare namespace pino {
(msg: string, ...args: any[]): void;
}

/** Future flags for Pino major-version 9. */
type FutureFlags = 'skipUnconditionalStdSerializers'

interface LoggerOptions<CustomLevels extends string = never, UseOnlyCustomLevels extends boolean = boolean> {
transport?: TransportSingleOptions | TransportMultiOptions | TransportPipelineOptions
/**
Expand Down Expand Up @@ -416,6 +419,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;
Comment on lines +422 to +429
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key for request and response are now explicitly configurable. Previously, these had fixed values, set on pino-std-serializers (mapHttpRequest and mapHttpResponse).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this change makes the exported functions mapHttpRequest and mapHttpResponse deprecated, in pino-std-serializers?

/**
* The string key to place any logged object under.
*/
Expand Down Expand Up @@ -662,6 +673,18 @@ declare namespace pino {
* logs newline delimited JSON with `\r\n` instead of `\n`. Default: `false`.
*/
crlf?: boolean;

/**
* The `future` object contains _opt-in_ flags specific to a Pino major version. These flags are used to change behavior,
* anticipating breaking-changes that will be introduced in the next major version.
* @example
* const parent = require('pino')({
* future: {
* skipUnconditionalStdSerializers: true
* }
* })
*/
future?: Partial<Record<FutureFlags, boolean>>
}

interface ChildLoggerOptions<CustomLevels extends string = never> {
Expand Down Expand Up @@ -749,12 +772,15 @@ 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;
readonly useOnlyCustomLevelsSym: unique symbol;
readonly formattersSym: unique symbol;
readonly hooksSym: unique symbol;
readonly futureSym: unique symbol;
};

/**
Expand Down
Loading