Repository navigation
Escape strings in raw JSON output - #1201
ZHUOLIN0928 wants to merge 2 commits into
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.
💡 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)") | ||
| } | ||
| } |
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