Skip to content

Escape strings in raw JSON output - #1201

Open
ZHUOLIN0928 wants to merge 2 commits into
SwiftyJSON:masterfrom
ZHUOLIN0928:codex/1200-escape-raw-json
Open

Escape strings in raw JSON output#1201
ZHUOLIN0928 wants to merge 2 commits into
SwiftyJSON:masterfrom
ZHUOLIN0928:codex/1200-escape-raw-json

Conversation

@ZHUOLIN0928

Copy link
Copy Markdown

Summary

  • Escape JSON object keys and string values consistently in the castNilToNSNull serialization path.
  • Cover quotes, backslashes, newlines, tabs, and control characters with a JSONSerialization round-trip regression test.

Fixes #1200

Testing

  • git diff --check
  • swift build passed
  • Built-library smoke test passed and JSONSerialization successfully parsed the generated output
  • swift test --filter RawTests was blocked because XCTest is unavailable in the current CommandLineTools environment

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes invalid JSON output produced by rawString([.castNilToNSNull: true]) by consistently escaping dictionary keys and string values (including control characters), and adds a regression test that round-trips the generated JSON through JSONSerialization.

Changes:

  • Added a dedicated escapeJSONString(_:) helper to correctly escape quotes, backslashes, and control characters per JSON string rules.
  • Updated the .castNilToNSNull manual serialization path to apply escaping to dictionary keys and string values (including array string elements).
  • Added an XCTest regression test that validates rawString([.castNilToNSNull: true]) produces JSON that JSONSerialization can parse and round-trip.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
Source/SwiftyJSON/SwiftyJSON.swift Introduces escapeJSONString and applies it to key/value emission in the manual .castNilToNSNull serialization path.
Tests/SwiftJSONTests/RawTests.swift Adds a JSONSerialization-based regression test for escaping behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +107 to +123
func testRawStringEscapesKeysAndControlCharacters() {
let key = "key\"with\ncontrol"
let value = "line1\nline2\t\u{0001}\"\\"
let json: JSON = [key: value]

guard let raw = json.rawString([.castNilToNSNull: true]) else {
XCTFail("Expected a JSON string")
return
}

do {
let object = try JSONSerialization.jsonObject(with: Data(raw.utf8)) as? [String: String]
XCTAssertEqual(object?[key], value)
} catch {
XCTFail("Expected valid JSON, got error: \(error)")
}
}
Comment on lines +541 to +565
private func escapeJSONString(_ string: String) -> String {
string.unicodeScalars.reduce(into: "") { result, scalar in
switch scalar.value {
case 0x08:
result += "\\b"
case 0x09:
result += "\\t"
case 0x0A:
result += "\\n"
case 0x0C:
result += "\\f"
case 0x0D:
result += "\\r"
case 0x22:
result += "\\\""
case 0x5C:
result += "\\\\"
case 0x00 ... 0x1F:
let hex = String(scalar.value, radix: 16, uppercase: true)
result += "\\u" + String(repeating: "0", count: 4 - hex.count) + hex
default:
result.append(contentsOf: String(scalar))
}
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZHUOLIN0928 It would be great if we can apply this change for better performance.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Tests/SwiftJSONTests/RawTests.swift:132

  • JSONSerialization.jsonObject(with:) commonly returns [Any] (or NSArray bridged to that), so conditionally casting to [String] may fail even when the JSON is valid. Cast to [Any] and then downcast the element to String to keep this regression test stable across Foundation implementations.
            let array = try JSONSerialization.jsonObject(with: Data(arrayRaw.utf8)) as? [String]
            XCTAssertEqual(array?.first, value)

Source/SwiftyJSON/SwiftyJSON.swift:627

  • In the castNilToNSNull dictionary serialization path, the key is escaped repeatedly (2–4 times) within the same closure. This is unnecessary work and makes the closure harder to read; compute the escaped key once and reuse it for the null/string/non-string branches.
				let body = try dict.keys.map { key throws -> String in
					guard let value = dict[key] else {
						return "\"\(escapeJSONString(key))\": null"
					}
					guard let unwrappedValue = value else {

Tests/SwiftJSONTests/RawTests.swift:119

  • JSONSerialization.jsonObject(with:) typically returns [String: Any] (or an NSDictionary bridged to that), so conditionally casting to [String: String] can fail even when the JSON is valid. Cast to [String: Any] and then downcast the value to String for a more robust regression test across platforms.

This issue also appears on line 131 of the same file.

            let object = try JSONSerialization.jsonObject(with: Data(dictRaw.utf8)) as? [String: String]
            XCTAssertEqual(object?[key], value)

@wongzigii
wongzigii marked this pull request as ready for review August 18, 2026 07:39
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.

rawString() produces invalid JSON — dict keys and string values containing ", newlines, or control chars are not escaped

3 participants