Skip to content

Commit

Permalink
fix: filter C0 and C1 control characters from logs and cli output
Browse files Browse the repository at this point in the history
  • Loading branch information
wraithgar committed Jan 2, 2024
1 parent 96c957a commit ebd0ede
Show file tree
Hide file tree
Showing 8 changed files with 278 additions and 109 deletions.
14 changes: 7 additions & 7 deletions lib/commands/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,20 +392,20 @@ class View extends BaseCommand {

if (info.keywords.length) {
this.npm.output('')
this.npm.output('keywords:', chalk.yellow(info.keywords.join(', ')))
this.npm.output(`keywords: ${chalk.yellow(info.keywords.join(', '))}`)
}

if (info.bins.length) {
this.npm.output('')
this.npm.output('bin:', chalk.yellow(info.bins.join(', ')))
this.npm.output(`bin: ${chalk.yellow(info.bins.join(', '))}`)
}

this.npm.output('')
this.npm.output('dist')
this.npm.output('.tarball:', info.tarball)
this.npm.output('.shasum:', info.shasum)
info.integrity && this.npm.output('.integrity:', info.integrity)
info.unpackedSize && this.npm.output('.unpackedSize:', info.unpackedSize)
this.npm.output(`.tarball: ${info.tarball}`)
this.npm.output(`.shasum: ${info.shasum}`)
info.integrity && this.npm.output(`.integrity: ${info.integrity}`)
info.unpackedSize && this.npm.output(`.unpackedSize: ${info.unpackedSize}`)

const maxDeps = 24
if (info.deps.length) {
Expand All @@ -420,7 +420,7 @@ class View extends BaseCommand {
if (info.maintainers && info.maintainers.length) {
this.npm.output('')
this.npm.output('maintainers:')
info.maintainers.forEach((u) => this.npm.output('-', u))
info.maintainers.forEach((u) => this.npm.output(`- ${u}`))
}

this.npm.output('')
Expand Down
4 changes: 2 additions & 2 deletions lib/npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ class Npm {
output (...msg) {
log.clearProgress()
// eslint-disable-next-line no-console
console.log(...msg)
console.log(...msg.map(Display.clean))
log.showProgress()
}

Expand Down Expand Up @@ -478,7 +478,7 @@ class Npm {
outputError (...msg) {
log.clearProgress()
// eslint-disable-next-line no-console
console.error(...msg)
console.error(...msg.map(Display.clean))
log.showProgress()
}
}
Expand Down
95 changes: 92 additions & 3 deletions lib/utils/display.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,44 @@ const npmlog = require('npmlog')
const log = require('./log-shim.js')
const { explain } = require('./explain-eresolve.js')

const originalCustomInspect = Symbol('npm.display.original.util.inspect.custom')

// These are most assuredly not a mistake
// https://eslint.org/docs/latest/rules/no-control-regex
/* eslint-disable no-control-regex */
// \x00 through \x1f, \x7f through \x9f, not including \x09 \x0a \x0b \x0d
const hasC01 = /[\x00-\x08\x0c\x0e-\x1f\x7f-\x9f]/
// Allows everything up to '[38;5;255m' in 8 bit notation
const allowedSGR = /^\[[0-9;]{0,8}m/
// '[38;5;255m'.length
const sgrMaxLen = 10

// Strips all ANSI C0 and C1 control characters (except for SGR up to 8 bit)
function stripC01 (str) {
if (!hasC01.test(str)) {
return str
}
let result = ''
for (let i = 0; i < str.length; i++) {
const char = str[i]
const code = char.charCodeAt(0)
if (!hasC01.test(char)) {
// Most characters are in this set so continue early if we can
result = `${result}${char}`
} else if (code === 27 && allowedSGR.test(str.slice(i + 1, i + sgrMaxLen + 1))) {
// \x1b with allowed SGR
result = `${result}\x1b`
} else if (code <= 31) {
// escape all other C0 control characters besides \x7f
result = `${result}^${String.fromCharCode(code + 64)}`
} else {
// hasC01 ensures this is now a C1 control character or \x7f
result = `${result}^${String.fromCharCode(code - 64)}`
}
}
return result
}

class Display {
#chalk = null

Expand All @@ -12,6 +50,57 @@ class Display {
log.pause()
}

static clean (output) {
if (typeof output === 'string') {
// Strings are cleaned inline
return stripC01(output)
}
if (!output || typeof output !== 'object') {
// Numbers, booleans, null all end up here and don't need cleaning
return output
}
// output && typeof output === 'object'
// We can't use hasOwn et al for detecting the original but we can use it
// for detecting the properties we set via defineProperty
if (
output[inspect.custom] &&
(!Object.hasOwn(output, originalCustomInspect))
) {
// Save the old one if we didn't already do it.
Object.defineProperty(output, originalCustomInspect, {
value: output[inspect.custom],
writable: true,
})
}
if (!Object.hasOwn(output, originalCustomInspect)) {
// Put a dummy one in for when we run multiple times on the same object
Object.defineProperty(output, originalCustomInspect, {
value: function () {
return this
},
writable: true,
})
}
// Set the custom inspect to our own function
Object.defineProperty(output, inspect.custom, {
value: function () {
const toClean = this[originalCustomInspect]()
// Custom inspect can return things other than objects, check type again
if (typeof toClean === 'string') {
// Strings are cleaned inline
return stripC01(toClean)
}
if (!toClean || typeof toClean !== 'object') {
// Numbers, booleans, null all end up here and don't need cleaning
return toClean
}
return stripC01(inspect(toClean, { customInspect: false }))
},
writable: true,
})
return output
}

on () {
process.on('log', this.#logHandler)
}
Expand Down Expand Up @@ -103,7 +192,7 @@ class Display {
// Explicitly call these on npmlog and not log shim
// This is the final place we should call npmlog before removing it.
#npmlog (level, ...args) {
npmlog[level](...args)
npmlog[level](...args.map(Display.clean))
}

// Also (and this is a really inexcusable kludge), we patch the
Expand All @@ -112,8 +201,8 @@ class Display {
// highly abbreviated explanation of what's being overridden.
#eresolveWarn (level, heading, message, expl) {
if (level === 'warn' &&
heading === 'ERESOLVE' &&
expl && typeof expl === 'object'
heading === 'ERESOLVE' &&
expl && typeof expl === 'object'
) {
this.#npmlog(level, heading, message)
this.#npmlog(level, '', explain(expl, this.#chalk, 2))
Expand Down
2 changes: 2 additions & 0 deletions lib/utils/log-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const { Minipass } = require('minipass')
const fsMiniPass = require('fs-minipass')
const fs = require('fs/promises')
const log = require('./log-shim')
const Display = require('./display')

const padZero = (n, length) => n.toString().padStart(length.toString().length, '0')
const globify = pattern => pattern.split('\\').join('/')
Expand Down Expand Up @@ -49,6 +50,7 @@ class LogFiles {

return format(...args)
.split(/\r?\n/)
.map(Display.clean)
.reduce((lines, line) =>
lines += prefix + (line ? ' ' : '') + line + os.EOL,
''
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/reify-output.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const reifyOutput = (npm, arb) => {
summary.audit = npm.command === 'audit' ? auditReport
: auditReport.toJSON().metadata
}
npm.output(JSON.stringify(summary, 0, 2))
npm.output(JSON.stringify(summary, null, 2))
} else {
packagesChangedMessage(npm, summary)
packagesFundingMessage(npm, summary)
Expand Down
Loading

0 comments on commit ebd0ede

Please sign in to comment.