Escape strings in raw JSON output - #1201
Conversation
There was a problem hiding this comment.
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
.castNilToNSNullmanual 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 thatJSONSerializationcan 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.
| 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)") | ||
| } | ||
| } |
| 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)) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
@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>
There was a problem hiding this comment.
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](orNSArraybridged to that), so conditionally casting to[String]may fail even when the JSON is valid. Cast to[Any]and then downcast the element toStringto 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 anNSDictionarybridged 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 toStringfor 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)
Summary
Fixes #1200
Testing