Skip to content
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.

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

Empty quoted strings are parsed as null #301

Closed
JensAyton opened this issue Mar 9, 2021 · 2 comments
Closed

Empty quoted strings are parsed as null #301

JensAyton opened this issue Mar 9, 2021 · 2 comments
Labels

Comments

@JensAyton
Copy link

Note: This is not the same as #253. However, the issue @tjprescott referenced in #253 is actually this.


When decoding a document like this:

stringA: "Hello"
stringB: ""

into a struct with optional string members like this:

struct TestType: Codable {
    var stringA: String?
    var stringB: String?
    var stringC: String?
}

stringB is read as nil rather than an empty string. If TestType.stringB is made non-optional, it’s read as an empty string.

On further inspection, this could also happen for string values of "~", "null", "Null" or "NULL". This is surprising and dangerous behaviour.

The YAML spec explicitly states: “Note that a null is different from an empty string.” While the style of the spec is confusing, I don’t believe it intends for the string "null" to be of null type either.

This can be fixed by changing NSNull.construct to return nil unless scalar.style == .plain. However, I’m leery of making a PR with this change since it may impact existing code with unfortunate dependencies on the current behaviour.

@JensAyton
Copy link
Author

Full example:

import Foundation
import Yams

let testYAML = #"""
stringA: "Hello"
stringB: ""
"""#

let testJSON = #"""
{
    "stringA": "Hello",
    "stringB": ""
}
"""#.data(using: .utf8)!

struct TestType: Codable {
    var stringA: String?
    var stringB: String?
    var stringC: String?
}

// Prints wrong result TestType(stringA: Optional("Hello"), stringB: nil, stringC: nil)
let testFromYAML = try! YAMLDecoder().decode(TestType.self, from: testYAML)
print(String(reflecting: testFromYAML))

// Prints correct result TestType(stringA: Optional("Hello"), stringB: nil, stringC: nil)
let testFromJSON = try! JSONDecoder().decode(TestType.self, from: testJSON)
print(String(reflecting: testFromJSON))

@jpsim jpsim added the bug label Mar 23, 2021
liamnichols added a commit to CreateAPI/Yams that referenced this issue Jun 9, 2022
…l types (#1)

* Add TopLevelDecoderTests.testDecodeOptionalTypes() to reproduce jpsim#301

* Update ConstructorTests.testNull() to include assertions about handling of null specifiers that are wrapped in quotes

* Update NSNull.construct(from:) to only return NSNull if the style is .plain since non-plain style scalars don't represent null (fixes bug)

* Update assertion in NodeTests to explicitly require .plain style when making assertion

* Correct SwiftLint violations
jpsim pushed a commit that referenced this issue Oct 15, 2023
Add TopLevelDecoderTests.testDecodeOptionalTypes() to reproduce #301

Update ConstructorTests.testNull() to include assertions about handling of null specifiers that are wrapped in quotes

Update NSNull.construct(from:) to only return NSNull if the style is .plain since non-plain style scalars don't represent null (fixes bug)

Update assertion in NodeTests to explicitly require .plain style when making assertion

Correct SwiftLint violations

Add changelog entry
@jpsim
Copy link
Owner

jpsim commented Oct 15, 2023

Fixed in 0c4ff78.

@jpsim jpsim closed this as completed Oct 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants