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

Bump to 1.0.4, fix escaped whitespace before " bug #19

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

mbland
Copy link
Owner

@mbland mbland commented Jan 2, 2024

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.

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.
@mbland mbland self-assigned this Jan 2, 2024
@coveralls
Copy link

Pull Request Test Coverage Report for Build 7387720627

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 7378376363: 0.0%
Covered Lines: 238
Relevant Lines: 238

💛 - Coveralls

@mbland mbland merged commit 03e3ea5 into main Jan 2, 2024
3 checks passed
@mbland mbland deleted the check-string-first-avoid-escaped-whitespace-bug branch January 2, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants