Skip to content

Commit

Permalink
Bump to 1.0.4, fix escaped whitespace before " bug
Browse files Browse the repository at this point in the history
Before this change, an escaped whitespace before the closing double
quote character of a string would cause stripJsonComments() to see the
double quote as escaped. This would throw off the comment removal
algorithm since the string would still appear open.

Escaped whitespace inside a string will cause a JSON parse error, but
that shouldn't prevent stripJsonComments() from removing comments from
otherwise legitimate JSON.

The new test reproduced the error and verified the fix, which was to
check the `inString` case before the whitespace-preserving cases.
  • Loading branch information
mbland committed Jan 2, 2024
1 parent 9d3677a commit b071ce6
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 9 deletions.
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ Running the wrapper will generate the local `file://` URL to the generated
`index.html` file, e.g.:

```text
file:///path/to/jsdoc/output/jsdoc-cli-wrapper/1.0.3/index.html
file:///path/to/jsdoc/output/jsdoc-cli-wrapper/1.0.4/index.html
```

You can click on or copy this link to open it in your browser. You can also open
Expand Down Expand Up @@ -99,10 +99,10 @@ This wrapper resolves both of these minor annoyances.
```sh
$ pnpm jsdoc

> jsdoc-cli-wrapper@1.0.3 jsdoc /path/to/jsdoc-cli-wrapper
> jsdoc-cli-wrapper@1.0.4 jsdoc /path/to/jsdoc-cli-wrapper
> node index.js -c jsdoc.json .

file:///path/to/jsdoc-cli-wrapper/jsdoc/jsdoc-cli-wrapper/1.0.3/index.html
file:///path/to/jsdoc-cli-wrapper/jsdoc/jsdoc-cli-wrapper/1.0.4/index.html
```

Of course, your own project would use `jsdoc-cli-wrapper` instead of `node
Expand Down
10 changes: 5 additions & 5 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,19 @@ export function stripJsonComments(str) {
for (let i = 0; i !== str.length; ++i) {
let c = str[i]

if (c === '\n') {
if (comment === 'line') comment = null
} else if (c.trimStart() === '') { // preserve other existing whitespace
} else if (inString) {
if (inString) { // check first so illegally escaped whitespace won't hide "
inString = c !== '"' || escaped
escaped = c === '\\' && !escaped
} else if (c === '\n') {
if (comment === 'line') comment = null
} else if (c.trimStart() === '') { // preserve all other existing whitespace
} else if (comment) {
if (c === '/' && comment === 'block' && str[i-1] === '*') comment = null
c = ' '
} else if (c === '/' || c === '*') { // maybe a comment, don't update comma
if (str[i-1] === '/') { // definitely a comment, or else a syntax error
comment = (c === '/') ? 'line' : 'block'
c = result[i-1] = ' '
result[i-1] = c = ' '
}
} else if (c === ',') {
comma = i
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "jsdoc-cli-wrapper",
"version": "1.0.3",
"version": "1.0.4",
"description": "JSDoc command line interface wrapper",
"main": "index.js",
"bin": "./index.js",
Expand Down
29 changes: 29 additions & 0 deletions test/stripJsonComments.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,5 +286,34 @@ describe('stripJsonComments', () => {
` JSON at position ${src.indexOf(', // ...but this last comma')}`
)
})

test('a string contains an escaped space before the closing quote', () => {
const src = [
'{',
' "opts": {',
' // This comment should disappear regardless.',
' "fubar": "If not handled, the comments below will remain.\\ "',
' /* Escaped space is illegal JSON, but it shouldn\'t hide a',
' * closing quote and prevent comment removal. */',
' }',
'}'].join('\n')

const result = stripJsonComments(src)

expect(result).toBe([
'{',
' "opts": {',
' ',
' "fubar": "If not handled, the comments below will remain.\\ "',
' ',
' ',
' }',
'}'].join('\n'))
expect(() => JSON.parse(result)).toThrowError(
// It will choke on the space, not the escape slash.
errPrefix(' ', 'Bad escaped character in') +
` JSON at position ${src.indexOf('\\ "') + 1}`
)
})
})
})

0 comments on commit b071ce6

Please sign in to comment.