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

Jump to bottom

[SR-6563] JSON decoder's internal state messed up after nested decoding failed #3773

Closed
ffried opened this issue Dec 8, 2017 · 6 comments
Closed

[SR-6563] JSON decoder's internal state messed up after nested decoding failed #3773

ffried opened this issue Dec 8, 2017 · 6 comments
Labels
bug Codable Foundation

Comments

@ffried
Copy link
Contributor

ffried commented Dec 8, 2017

Previous ID SR-6563
Radar rdar://problem/36253570
Original Reporter @ffried
Type Bug
Status Resolved
Resolution Duplicate

Attachment: Download

Environment

Apple Swift version 4.0.3 (swiftlang-900.0.74.1 clang-900.0.39.2)
Target: x86_64-apple-macosx10.9

Additional Detail from JIRA
Votes 0
Component/s Foundation
Labels Bug, Codable
Assignee None
Priority Medium

md5: 55d61821d711ce161aab03b2cca1221c

duplicates:

  • SR-6408 JSONDecoder internal state could be corrupted

Issue Description:

When decoding an object's nested object fails, the decoder's internal state is messed up and decoding a valid object fails too.

Given the following code:

import Foundation

enum Either<A, B> {
    case a(A)
    case b(B)
}

protocol ModelProtocol {}

struct ModelA: Codable, ModelProtocol {
    let guid: String
    let age: Int
}

struct ModelB: Codable, ModelProtocol {
    let id: Int
    let name: String
}

struct BaseModel<Model: ModelProtocol & Codable>: Codable {
    let model: Model
}

struct ModelWrapper: Decodable {
    let eitherModel: Either<BaseModel<ModelA>, BaseModel<ModelB>>

    init(from decoder: Decoder) throws {
        let container = try decoder.singleValueContainer()
        do {
            eitherModel = try .a(container.decode(BaseModel<ModelA>.self))
        } catch {
            print("ModelA failed to decode:\n\(error)\n")
            print("Trying ModelB...")
            eitherModel = try .b(container.decode(BaseModel<ModelB>.self))
            print("ModelB worked!")
        }
    }
}

let jsonStr = """
{
  "model": {
    "id": 1,
    "name": "Test"
  }
}
"""
let jsonData = jsonStr.data(using: .utf8)!

do {
    let wrapper = try JSONDecoder().decode(ModelWrapper.self, from: jsonData)
    switch wrapper.eitherModel {
    case .a(let modelA):
        print("Found A: \(modelA)!")
    case .b(let modelB):
        print("Found B: \(modelB)!")
    }
} catch {
    print("Encountered error: \(error)")
}

The ModelWrapper tries to first decode BaseModel<ModelA> which fails (due to the fact that the structure of ModelA doesn't match). But BaseModel<ModelB> should work, since ModelB matches the JSONs structure. However, this also fails to decode because they key "model" is apparently not present (which it is!).

In another example (which is bit more complex), this lead to messed up output for us. We have a setup where we need to decode and process only parts of a (rather huge) JSON structure, but re-encode everything. We've come up with AnyCodable which decodes any value in an opaque way and re-encodes it later. While this worked with Swift 4.0.2, it broke in Swift 4.0.3. In Swift 4.0.3, the output does not match the input structure. Nested objects move up the structure tree and end up on the top level.

I've attached both, s4_codableError.swift which contains the sample above, and s4_anycodable.swift which contains a reproduction of the aforementioned more complex case.
@ffried
Copy link
Contributor Author

ffried commented Dec 8, 2017

After further inspection, this seems to be caused by the internal container stack not being popped again in e.g. this line in case an error occurs.

@belkadan
Copy link

belkadan commented Dec 8, 2017

@itaiferber, you have one of these already, right?

@swift-ci
Copy link
Contributor

swift-ci commented Jan 2, 2018

Comment by Thorsten Karrer (JIRA)

Having the same problem (filed as radar 36253570 before finding this JIRA). Pretty positive that this is caused by the code line @ffried identified. With Swift's exception-like error model, these mistakes are super easy to make.

@ffried
Copy link
Contributor Author

ffried commented Jan 2, 2018

sune (JIRA User) thanks for the radar. I've added it to the SR.

As a workaround, it worked for me not to ask the single value container to decode a nested object (lines 28 and 29 in s4_anycodable.swift), but instead call its initializer directly with the encoder that was passed to AnyCodable in this case. This essentially works around the aforementioned problematic line for now.

@swift-ci
Copy link
Contributor

swift-ci commented Jan 2, 2018

Comment by Thorsten Karrer (JIRA)

@ffried, thanks for the workaround suggestion! Unfortunately, it does not work for me because there is more decoding going on in the calling function using the same decoder. There, I already use the initializer directly, but it still fails because of the broken container stack (needs a keyed container at this place but at the top of the stack is the single value container that never got removed).

Addendum: For now, I will just ship a private fixed JSONDecoder implementation. Simply called the pop via defer before the try.

@itaiferber
Copy link
Contributor

Confirmed that this is indeed a dupe of SR-6408 and should be resolved with fixes there.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Codable Foundation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@itaiferber @belkadan @ffried @swift-ci