Conversation
🦋 Changeset detectedLatest commit: 830d6ba The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR adds proper JSON escaping to the Biome CLI's JSON reporter. A private Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_cli/src/reporter/json.rs`:
- Around line 14-24: The current json_escape function only handles some
characters via chained replace calls and misses many control characters; change
json_escape to iterate over the input chars (replacing the existing chained
replace implementation) and for each char emit the appropriate escape:
keep/backslash-escape '"' and '\\' and map
'\n','\r','\t','\u0008'('\b'),'\u000C'('\f') to their short escapes, and for any
codepoint <= 0x1F produce a "\\u00XX" escape (hex uppercase or lowercase is
fine) so every control character in U+0000..U+001F is encoded; update references
to the function name json_escape only — no other public API changes.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_cli/tests/snapshots/main_cases_reporter_json/reports_diagnostics_json_with_escaped_quotes.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/fix-json-reporter-escaped-quotes.mdcrates/biome_cli/src/reporter/json.rscrates/biome_cli/tests/cases/reporter_json.rs
| /// Escapes special characters in a string for safe JSON output | ||
| fn json_escape(input: &str) -> String { | ||
| input | ||
| .replace('\\', "\\\\") | ||
| .replace('"', "\\\"") | ||
| .replace('\n', "\\n") | ||
| .replace('\r', "\\r") | ||
| .replace('\t', "\\t") | ||
| .replace('\x08', "\\b") | ||
| .replace('\x0C', "\\f") | ||
| } |
There was a problem hiding this comment.
Escape every control character (\u0000..\u001F) to guarantee valid JSON.
Line 17-Line 23 cover common escapes, but not all JSON-required control chars. Values containing e.g. \u0000 or \u000B can still break output validity.
Suggested fix
fn json_escape(input: &str) -> String {
- input
- .replace('\\', "\\\\")
- .replace('"', "\\\"")
- .replace('\n', "\\n")
- .replace('\r', "\\r")
- .replace('\t', "\\t")
- .replace('\x08', "\\b")
- .replace('\x0C', "\\f")
+ let mut out = String::with_capacity(input.len());
+ for ch in input.chars() {
+ match ch {
+ '\\' => out.push_str("\\\\"),
+ '"' => out.push_str("\\\""),
+ '\n' => out.push_str("\\n"),
+ '\r' => out.push_str("\\r"),
+ '\t' => out.push_str("\\t"),
+ '\u{08}' => out.push_str("\\b"),
+ '\u{0C}' => out.push_str("\\f"),
+ c if c <= '\u{1F}' => {
+ use std::fmt::Write as _;
+ let _ = write!(out, "\\u{:04X}", c as u32);
+ }
+ c => out.push(c),
+ }
+ }
+ out
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Escapes special characters in a string for safe JSON output | |
| fn json_escape(input: &str) -> String { | |
| input | |
| .replace('\\', "\\\\") | |
| .replace('"', "\\\"") | |
| .replace('\n', "\\n") | |
| .replace('\r', "\\r") | |
| .replace('\t', "\\t") | |
| .replace('\x08', "\\b") | |
| .replace('\x0C', "\\f") | |
| } | |
| /// Escapes special characters in a string for safe JSON output | |
| fn json_escape(input: &str) -> String { | |
| let mut out = String::with_capacity(input.len()); | |
| for ch in input.chars() { | |
| match ch { | |
| '\\' => out.push_str("\\\\"), | |
| '"' => out.push_str("\\\""), | |
| '\n' => out.push_str("\\n"), | |
| '\r' => out.push_str("\\r"), | |
| '\t' => out.push_str("\\t"), | |
| '\u{08}' => out.push_str("\\b"), | |
| '\u{0C}' => out.push_str("\\f"), | |
| c if c <= '\u{1F}' => { | |
| use std::fmt::Write as _; | |
| let _ = write!(out, "\\u{:04X}", c as u32); | |
| } | |
| c => out.push(c), | |
| } | |
| } | |
| out | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_cli/src/reporter/json.rs` around lines 14 - 24, The current
json_escape function only handles some characters via chained replace calls and
misses many control characters; change json_escape to iterate over the input
chars (replacing the existing chained replace implementation) and for each char
emit the appropriate escape: keep/backslash-escape '"' and '\\' and map
'\n','\r','\t','\u0008'('\b'),'\u000C'('\f') to their short escapes, and for any
codepoint <= 0x1F produce a "\\u00XX" escape (hex uppercase or lowercase is
fine) so every control character in U+0000..U+001F is encoded; update references
to the function name json_escape only — no other public API changes.
| use camino::{Utf8Path, Utf8PathBuf}; | ||
|
|
||
| /// Escapes special characters in a string for safe JSON output | ||
| fn json_escape(input: &str) -> String { |
There was a problem hiding this comment.
Do we need to add json_escape() for AnyJsonValue::JsonStringValue(json_string_value(json_string_literal(&location.path))) too since Windows path contains back slash like this(C:\Projects\File\hoge.rs)?
3 participants
Summary
Bug found when I was testing the JSON reporter in the ecosystem CI
I generated the code with AI. The escaping uses the same code of checklist reporter.
Test Plan
Added a test
Docs