Conversation
…ON strings Replace the regex with a single-pass scanner that tracks whether it's currently inside a JSON string and only treats backquotes and a string to escape if it is outside of a double quote block.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes base.ConvertBackQuotedStrings so it no longer corrupts backticks that appear inside existing JSON strings (e.g., in sync function source embedded in JSON). It replaces the previous regex-based implementation with a single-pass scanner that tracks JSON string context, and expands test coverage (including fuzzing) to lock in the corrected behavior.
Changes:
- Replaced the regex backquote conversion logic with a stateful scanner that ignores backticks inside JSON double-quoted strings.
- Added extensive unit tests and fuzz tests validating idempotence and “valid JSON in => semantically identical JSON out”.
- Updated an admin API test to exercise more sync-function variants containing backticks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rest/adminapitest/admin_api_test.go | Expands the collection sync-function test into subtests covering multiple backtick scenarios. |
| base/util.go | Reimplements ConvertBackQuotedStrings using a single-pass scanner with helpers for closing-delimiter detection and escaping. |
| base/util_test.go | Adds many new cases plus fuzz targets to validate correctness, idempotence, and preservation of valid JSON. |
Comment on lines
+3396
to
3400
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| // Start each subtest from a clean slate. | ||
| _ = rt.SendAdminRequest(http.MethodDelete, "/db/", "") | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CBG-5637 Fix ConvertBackQuotedStrings corrupting backquotes inside JSON strings
Replace the regex with a single-pass scanner that tracks whether it's currently inside a JSON string and only treats backquotes and a string to escape if it is outside of a double quote block.
Fixes code:
Note that the only backticks can occur in comments since otto does not support ES6 template literals.
Pre-review checklist
fmt.Print,log.Print, ...)base.UD(docID),base.MD(dbName))docs/apiIntegration Tests