Skip to content

[5.2] SR-12275: JSONEncoder on Linux can't encode number JSON fragments #2840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 24, 2020
Merged

[5.2] SR-12275: JSONEncoder on Linux can't encode number JSON fragments #2840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08e608d
SR-12275: JSONEncoder on Linux can't encode number JSON fragments
spevans Mar 3, 2020
758a29e
JSONSerialization: Add support for .withoutEscapingSlashes, .fragment…
spevans Mar 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions Foundation/JSONEncoder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,15 +238,8 @@ open class JSONEncoder {
throw EncodingError.invalidValue(value, EncodingError.Context(codingPath: [], debugDescription: "Top-level \(T.self) did not encode any values."))
}

if topLevel is NSNull {
throw EncodingError.invalidValue(value, EncodingError.Context(codingPath: [], debugDescription: "Top-level \(T.self) encoded as null JSON fragment."))
} else if topLevel is NSNumber {
throw EncodingError.invalidValue(value, EncodingError.Context(codingPath: [], debugDescription: "Top-level \(T.self) encoded as number JSON fragment."))
} else if topLevel is NSString {
throw EncodingError.invalidValue(value, EncodingError.Context(codingPath: [], debugDescription: "Top-level \(T.self) encoded as string JSON fragment."))
}
let writingOptions = JSONSerialization.WritingOptions(rawValue: self.outputFormatting.rawValue).union(.fragmentsAllowed)

let writingOptions = JSONSerialization.WritingOptions(rawValue: self.outputFormatting.rawValue)
do {
return try JSONSerialization.data(withJSONObject: topLevel, options: writingOptions)
} catch {
Expand Down Expand Up @@ -1155,7 +1148,7 @@ open class JSONDecoder {
open func decode<T : Decodable>(_ type: T.Type, from data: Data) throws -> T {
let topLevel: Any
do {
topLevel = try JSONSerialization.jsonObject(with: data)
topLevel = try JSONSerialization.jsonObject(with: data, options: .allowFragments)
} catch {
throw DecodingError.dataCorrupted(DecodingError.Context(codingPath: [], debugDescription: "The given data was not valid JSON.", underlyingError: error))
}
Expand Down
21 changes: 14 additions & 7 deletions Foundation/JSONSerialization.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ extension JSONSerialization {

public static let prettyPrinted = WritingOptions(rawValue: 1 << 0)
public static let sortedKeys = WritingOptions(rawValue: 1 << 1)
public static let fragmentsAllowed = WritingOptions(rawValue: 1 << 2)
public static let withoutEscapingSlashes = WritingOptions(rawValue: 1 << 3)
Copy link
Author

Choose a reason for hiding this comment

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

can we add this in a 5.2 or not since it's public and adding API?

Could we still do this if we make sure no new public types are added?

WDYT @millenomi ?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can always add any API that Darwin already has.

Copy link
Contributor

Choose a reason for hiding this comment

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

(As is the case here.)

Copy link
Author

Choose a reason for hiding this comment

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

That's great to hear, thank you :-)

}
}

Expand Down Expand Up @@ -139,8 +141,7 @@ open class JSONSerialization : NSObject {
var jsonStr = [UInt8]()

var writer = JSONWriter(
pretty: opt.contains(.prettyPrinted),
sortedKeys: opt.contains(.sortedKeys),
options: opt,
writer: { (str: String?) in
if let str = str {
jsonStr.append(contentsOf: str.utf8)
Expand All @@ -157,7 +158,10 @@ open class JSONSerialization : NSObject {
} else if let container = value as? Dictionary<AnyHashable, Any> {
try writer.serializeJSON(container)
} else {
fatalError("Top-level object was not NSArray or NSDictionary") // This is a fatal error in objective-c too (it is an NSInvalidArgumentException)
guard opt.contains(.fragmentsAllowed) else {
fatalError("Top-level object was not NSArray or NSDictionary") // This is a fatal error in objective-c too (it is an NSInvalidArgumentException)
}
try writer.serializeJSON(value)
}

let count = jsonStr.count
Expand Down Expand Up @@ -301,11 +305,13 @@ private struct JSONWriter {
var indent = 0
let pretty: Bool
let sortedKeys: Bool
let withoutEscapingSlashes: Bool
let writer: (String?) -> Void

init(pretty: Bool = false, sortedKeys: Bool = false, writer: @escaping (String?) -> Void) {
self.pretty = pretty
self.sortedKeys = sortedKeys
init(options: JSONSerialization.WritingOptions, writer: @escaping (String?) -> Void) {
pretty = options.contains(.prettyPrinted)
sortedKeys = options.contains(.sortedKeys)
withoutEscapingSlashes = options.contains(.withoutEscapingSlashes)
self.writer = writer
}

Expand Down Expand Up @@ -379,7 +385,8 @@ private struct JSONWriter {
case "\\":
writer("\\\\") // U+005C reverse solidus
case "/":
writer("\\/") // U+002F solidus
if !withoutEscapingSlashes { writer("\\") }
writer("/") // U+002F solidus
case "\u{8}":
writer("\\b") // U+0008 backspace
case "\u{c}":
Expand Down
49 changes: 45 additions & 4 deletions TestFoundation/TestJSONEncoder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,40 @@ struct TopLevelObjectWrapper<T: Codable & Equatable>: Codable, Equatable {

class TestJSONEncoder : XCTestCase {

// MARK: - Encoding Top-Level fragments
func test_encodingTopLevelFragments() {

func _testFragment<T: Codable & Equatable>(value: T, fragment: String) {
let data: Data
let payload: String

do {
data = try JSONEncoder().encode(value)
payload = try XCTUnwrap(String.init(decoding: data, as: UTF8.self))
XCTAssertEqual(fragment, payload)
} catch {
XCTFail("Failed to encode \(T.self) to JSON: \(error)")
return
}
do {
let decodedValue = try JSONDecoder().decode(T.self, from: data)
XCTAssertEqual(value, decodedValue)
} catch {
XCTFail("Failed to decode \(payload) to \(T.self): \(error)")
}
}

_testFragment(value: 2, fragment: "2")
_testFragment(value: false, fragment: "false")
_testFragment(value: true, fragment: "true")
_testFragment(value: Float(1), fragment: "1")
_testFragment(value: Double(2), fragment: "2")
_testFragment(value: Decimal(Double.leastNormalMagnitude), fragment: "0.0000000000000000000000000000000000000000000000000002225073858507201792")
_testFragment(value: "test", fragment: "\"test\"")
let v: Int? = nil
_testFragment(value: v, fragment: "null")
}

// MARK: - Encoding Top-Level Empty Types
func test_encodingTopLevelEmptyStruct() {
let empty = EmptyStruct()
Expand All @@ -34,20 +68,20 @@ class TestJSONEncoder : XCTestCase {

// MARK: - Encoding Top-Level Single-Value Types
func test_encodingTopLevelSingleValueEnum() {
_testEncodeFailure(of: Switch.off)
_testEncodeFailure(of: Switch.on)
_testRoundTrip(of: Switch.off)
_testRoundTrip(of: Switch.on)

_testRoundTrip(of: TopLevelArrayWrapper(Switch.off))
_testRoundTrip(of: TopLevelArrayWrapper(Switch.on))
}

func test_encodingTopLevelSingleValueStruct() {
_testEncodeFailure(of: Timestamp(3141592653))
_testRoundTrip(of: Timestamp(3141592653))
_testRoundTrip(of: TopLevelArrayWrapper(Timestamp(3141592653)))
}

func test_encodingTopLevelSingleValueClass() {
_testEncodeFailure(of: Counter())
_testRoundTrip(of: Counter())
_testRoundTrip(of: TopLevelArrayWrapper(Counter()))
}

Expand Down Expand Up @@ -387,6 +421,11 @@ class TestJSONEncoder : XCTestCase {
}
}

func test_codingOfNil() {
let x: Int? = nil
test_codingOf(value: x, toAndFrom: "null")
}

func test_codingOfInt8() {
test_codingOf(value: Int8(-42), toAndFrom: "-42")
}
Expand Down Expand Up @@ -1259,6 +1298,7 @@ fileprivate struct JSON: Equatable {
extension TestJSONEncoder {
static var allTests: [(String, (TestJSONEncoder) -> () throws -> Void)] {
return [
("test_encodingTopLevelFragments", test_encodingTopLevelFragments),
("test_encodingTopLevelEmptyStruct", test_encodingTopLevelEmptyStruct),
("test_encodingTopLevelEmptyClass", test_encodingTopLevelEmptyClass),
("test_encodingTopLevelSingleValueEnum", test_encodingTopLevelSingleValueEnum),
Expand All @@ -1282,6 +1322,7 @@ extension TestJSONEncoder {
("test_nestedContainerCodingPaths", test_nestedContainerCodingPaths),
("test_superEncoderCodingPaths", test_superEncoderCodingPaths),
("test_codingOfBool", test_codingOfBool),
("test_codingOfNil", test_codingOfNil),
("test_codingOfInt8", test_codingOfInt8),
("test_codingOfUInt8", test_codingOfUInt8),
("test_codingOfInt16", test_codingOfInt16),
Expand Down
22 changes: 22 additions & 0 deletions TestFoundation/TestJSONSerialization.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,8 @@ extension TestJSONSerialization {
("test_serialize_Decimal", test_serialize_Decimal),
("test_serialize_NSDecimalNumber", test_serialize_NSDecimalNumber),
("test_serialize_stringEscaping", test_serialize_stringEscaping),
("test_serialize_fragments", test_serialize_fragments),
("test_serialize_withoutEscapingSlashes", test_serialize_withoutEscapingSlashes),
("test_jsonReadingOffTheEndOfBuffers", test_jsonReadingOffTheEndOfBuffers),
("test_jsonObjectToOutputStreamBuffer", test_jsonObjectToOutputStreamBuffer),
("test_jsonObjectToOutputStreamFile", test_jsonObjectToOutputStreamFile),
Expand Down Expand Up @@ -1371,6 +1373,26 @@ extension TestJSONSerialization {
XCTAssertEqual(try trySerialize(json), "[\"j\\/\"]")
}

func test_serialize_fragments() {
XCTAssertEqual(try trySerialize(2, options: .fragmentsAllowed), "2")
XCTAssertEqual(try trySerialize(false, options: .fragmentsAllowed), "false")
XCTAssertEqual(try trySerialize(true, options: .fragmentsAllowed), "true")
XCTAssertEqual(try trySerialize(Float(1), options: .fragmentsAllowed), "1")
XCTAssertEqual(try trySerialize(Double(2), options: .fragmentsAllowed), "2")
XCTAssertEqual(try trySerialize(Decimal(Double.leastNormalMagnitude), options: .fragmentsAllowed), "0.0000000000000000000000000000000000000000000000000002225073858507201792")
XCTAssertEqual(try trySerialize("test", options: .fragmentsAllowed), "\"test\"")
}

func test_serialize_withoutEscapingSlashes() {
// .withoutEscapingSlashes controls whether a "/" is encoded as "\\/" or "/"
let testString = "This /\\/ is a \\ \\\\ \\\\\\ \"string\"\n\r\t\u{0}\u{1}\u{8}\u{c}\u{f}"
let escapedString = "\"This \\/\\\\\\/ is a \\\\ \\\\\\\\ \\\\\\\\\\\\ \\\"string\\\"\\n\\r\\t\\u0000\\u0001\\b\\f\\u000f\""
let unescapedString = "\"This /\\\\/ is a \\\\ \\\\\\\\ \\\\\\\\\\\\ \\\"string\\\"\\n\\r\\t\\u0000\\u0001\\b\\f\\u000f\""

XCTAssertEqual(try trySerialize(testString, options: .fragmentsAllowed), escapedString)
XCTAssertEqual(try trySerialize(testString, options: [.withoutEscapingSlashes, .fragmentsAllowed]), unescapedString)
}

/* These are a programming error and should not be done
Ideally the interface for JSONSerialization should at compile time prevent this type of thing
by overloading the interface such that it can only accept dictionaries and arrays.
Expand Down