From e329bae181e1faafb4c206b065cc7bdf3d8768f0 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sat, 23 Mar 2024 11:12:40 -0700 Subject: [PATCH] snap: don't error when cwd is shorter than '{CWD}' This also incidentally exposed a problem with range errors in the stack handling, because RangeErrors don't have a stack, but the stack may be set to a non-string value like `null` rather than just being unset. Fix: https://github.com/tapjs/tapjs/issues/1013 --- src/snapshot/src/clean-cwd.ts | 17 ++-- src/snapshot/test/clean-cwd.ts | 142 +++++++++++++++++++-------------- src/stack/src/index.ts | 9 +-- src/stack/test/index.ts | 6 ++ 4 files changed, 98 insertions(+), 76 deletions(-) diff --git a/src/snapshot/src/clean-cwd.ts b/src/snapshot/src/clean-cwd.ts index 811408bea..e4871b9f2 100644 --- a/src/snapshot/src/clean-cwd.ts +++ b/src/snapshot/src/clean-cwd.ts @@ -19,21 +19,14 @@ for (const c of [...cwds]) { cwds.add(esc(esc(esc(esc(c))))) } +const reSpecial = /[/\-\\^$*+?.()|[\]{}]/g +const escapeRegex = (s: string) => s.replace(reSpecial, '\\$&') + export const cleanCWD = (snap: string) => { const tag = '{CWD}' for (const c of cwds) { - let i = -1 - // pad it out so that the length matches through the walk - const replace = tag + '\u0001'.repeat(c.length - tag.length) - while ( - -1 !== (i = snap.toLowerCase().indexOf(c.toLowerCase(), i)) - ) { - snap = - snap.substring(0, i) + - replace + - snap.substring(i + replace.length) - } - snap = snap.split(replace).join(tag) + const r = new RegExp(escapeRegex(c), 'gi') + snap = snap.replace(r, tag) } return snap } diff --git a/src/snapshot/test/clean-cwd.ts b/src/snapshot/test/clean-cwd.ts index b1047615a..cecd6b3d9 100644 --- a/src/snapshot/test/clean-cwd.ts +++ b/src/snapshot/test/clean-cwd.ts @@ -1,43 +1,50 @@ +import {resolve} from 'node:path' import * as nodeUrl from 'node:url' import t from 'tap' -t.capture(process, 'cwd', () => 'D:\\some\\TEST-Path') +t.test('short CWDs are not a problem', async t => { + const s = resolve('/x') + const { cleanCWD } = await t.mockImport< + typeof import('../dist/esm/clean-cwd.js') + >('../dist/esm/clean-cwd.js', { + '@tapjs/core': { + cwd: s, + }, + url: t.createMock(nodeUrl, { + pathToFileURL: (path: string): URL => + path === s + ? new URL('file:///x') + : nodeUrl.pathToFileURL(path), + }), + }) + t.capture(process, 'cwd', () => s) -const s = 'D:\\some\\TEST-Path' -const u = 'file:///D:/some/TEST-Path' - -const { cleanCWD } = await t.mockImport< - typeof import('../dist/esm/clean-cwd.js') ->('../dist/esm/clean-cwd.js', { - '@tapjs/core': { - cwd: s, - }, - url: t.createMock(nodeUrl, { - pathToFileURL: (path: string): URL => - path === s ? new URL(u) : nodeUrl.pathToFileURL(path), - }), + // gut-check + t.ok(s.length < '{CWD}'.length, 'actually testing something') + t.equal(cleanCWD(s), '{CWD}') }) -const p = s.replace(/\\/g, '/') +t.test('normal length cwd path', async t => { + t.capture(process, 'cwd', () => 'D:\\some\\TEST-Path') + + const s = 'D:\\some\\TEST-Path' + const u = 'file:///D:/some/TEST-Path' -const { - href, - origin, - protocol, - username, - password, - host, - hostname, - port, - pathname, - search, - searchParams, - hash, -} = new URL(u) + const { cleanCWD } = await t.mockImport< + typeof import('../dist/esm/clean-cwd.js') + >('../dist/esm/clean-cwd.js', { + '@tapjs/core': { + cwd: s, + }, + url: t.createMock(nodeUrl, { + pathToFileURL: (path: string): URL => + path === s ? new URL(u) : nodeUrl.pathToFileURL(path), + }), + }) -const obj = { - s: [s, p, s, p, u], - url: { + const p = s.replace(/\\/g, '/') + + const { href, origin, protocol, @@ -50,32 +57,51 @@ const obj = { search, searchParams, hash, - }, -} -const j1 = JSON.stringify(obj) -const j2 = JSON.stringify({ json: j1 }) -const j3 = JSON.stringify({ json: j2 }) -const j4 = JSON.stringify({ json: j3 }) -const tests = new Set([s, p, j1, j2, j3, j4]) -for (const s of [...tests]) { - tests.add(s.toUpperCase()) - tests.add(s.toLowerCase()) -} + } = new URL(u) + + const obj = { + s: [s, p, s, p, u], + url: { + href, + origin, + protocol, + username, + password, + host, + hostname, + port, + pathname, + search, + searchParams, + hash, + }, + } + const j1 = JSON.stringify(obj) + const j2 = JSON.stringify({ json: j1 }) + const j3 = JSON.stringify({ json: j2 }) + const j4 = JSON.stringify({ json: j3 }) + const tests = new Set([s, p, j1, j2, j3, j4]) + for (const s of [...tests]) { + tests.add(s.toUpperCase()) + tests.add(s.toLowerCase()) + } -for (const c of tests) { - t.equal( - cleanCWD(c).toLowerCase().indexOf('test-path'), + for (const c of tests) { + t.equal( + cleanCWD(c).toLowerCase().indexOf('test-path'), + -1, + 'escaped', + { + string: c, + } + ) + } + + // just capture the limit. only 4 levels of escaping supported + t.not( + cleanCWD(JSON.stringify(j4)).toLowerCase().indexOf('test-path'), -1, - 'escaped', - { - string: c, - } + 'does not escape beyond 4 levels' ) -} - -// just capture the limit. only 4 levels of escaping supported -t.not( - cleanCWD(JSON.stringify(j4)).toLowerCase().indexOf('test-path'), - -1, - 'does not escape beyond 4 levels' -) + t.end() +}) diff --git a/src/stack/src/index.ts b/src/stack/src/index.ts index c2c4944b9..2b02a785c 100644 --- a/src/stack/src/index.ts +++ b/src/stack/src/index.ts @@ -306,12 +306,8 @@ export const captureError = ( e: Error | NodeJS.ErrnoException ): CallSiteLike[] => { // errors almost always have these fields - const { - stack = '', - message = '', - name = '', - code, - } = e as NodeJS.ErrnoException + const { message = '', name = '', code } = e as NodeJS.ErrnoException + const stack = typeof e.stack === 'string' ? e.stack : '' const head = name && message ? `${name}: ${message}\n` : '' const errnoHead = name && message && code ? `${name} [${code}]: ${message}` : '' @@ -330,6 +326,7 @@ export const captureError = ( .filter(l => !!l) .map(line => new CallSiteLike(e, line)) ) + // if we didn't clean the header cleanly, then sweep the stack for // any weird junk it might contain return cleanHead diff --git a/src/stack/test/index.ts b/src/stack/test/index.ts index af99e69e5..c99dff221 100644 --- a/src/stack/test/index.ts +++ b/src/stack/test/index.ts @@ -494,6 +494,12 @@ ${short.stack?.split('\n').slice(1).join('\n')}` t.end() }) + t.test('error objects sometimes have weird stacks', t => { + const e = Object.assign(new Error('weird'), { stack: 1243 }) + t.same(captureError(e), []) + t.end() + }) + t.end() }) }