Skip to content

Commit

Permalink
snap: don't error when cwd is shorter than '{CWD}'
Browse files Browse the repository at this point in the history
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: #1013
  • Loading branch information
isaacs committed Mar 23, 2024
1 parent b4fb630 commit e329bae
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 76 deletions.
17 changes: 5 additions & 12 deletions src/snapshot/src/clean-cwd.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
142 changes: 84 additions & 58 deletions src/snapshot/test/clean-cwd.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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()
})
9 changes: 3 additions & 6 deletions src/stack/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}` : ''
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/stack/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
}

0 comments on commit e329bae

Please sign in to comment.