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

[SR-6078] crashes with nested throwing values in JSON/PList Encoder #48633

Closed
swift-ci opened this issue Oct 6, 2017 · 6 comments
Closed

[SR-6078] crashes with nested throwing values in JSON/PList Encoder #48633

swift-ci opened this issue Oct 6, 2017 · 6 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. standard library Area: Standard library umbrella

Comments

@swift-ci
Copy link
Collaborator

swift-ci commented Oct 6, 2017

Previous ID SR-6078
Radar rdar://problem/34857336
Original Reporter brendanm500henderson (JIRA User)
Type Bug
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug
Assignee @itaiferber
Priority Medium

md5: 1065e421ab735e38aa8c528f5e90c69c

relates to:

Issue Description:

Referencing encoder deallocated with multiple containers on stack.

when a value throws before storage.popContainer() is called, the storage will not remove the last before deinit. Need to pop the value if a value was pushed (in box_ and box(Date or Data) (same applies for JSON closure(Date or Data, self) strategies)).

example (in box_) :

if encode throws here, the value is not removed

try value.encode(to: self)

(checks for depth count)

return self.storage.popContainer()

where this will crash:

struct Test<V: Encodable>: Encodable {
    var value: V
}

_ = try JSONEncoder().encode([Test(value: [Float.infinity])])

(Array is first to call super encoder, Test is second to add to storage, and Array<Float> adds the second value to storage and throws)

[[[Float.infinity]]] calls superEncoder() before storage count goes above 1 so it gets around this. Found a good solution if anyone is working on this. Example:

func box<T: Encodable>(_ value: T) throws -> Any {
    switch value {
    case is Date, is NSDate: return try self.box(value as! Date)
    default: return try self.box_(value)
    }
}
// in _JSONEncoder.box(Date):
case .deferred: return try self.box_(date)
case .closure(let c): return try self.box_(date, closure: c)
func box_<T: Encodable>(_ value: T, at codingPath: [CodingKey]) throws -> Any {
    return try self.box_(value, at: codingPath, closure: {try $0.encode(to: $1) })
}

func box_<T: Encodable>(_ value: T, closure: (T, Encoder)throws->Void) throws -> Any {
    let depth = self.storage.count

    func popIfEncoded() -> Any? {
        switch self.storage.count {
        case depth + 1: return self.storage.removeLast()
        case depth: return nil
        default:
            if self.storage.count > depth + 1 {
                fatalError()
            } else {
                fatalError("encoder lost values from storage while encoding")
            }
        }
    }

    do {
        try closure(value, self)
    } catch {
        _ = popIfEncoded()
        throw error
    }

    return popIfEncoded() ?? { ()->Any in fatalError("value did not encode a value") }()  
}
@belkadan
Copy link
Contributor

belkadan commented Oct 6, 2017

Good catch (no pun intended). cc @itaiferber

@itaiferber
Copy link
Contributor

Indeed — thanks!

@itaiferber
Copy link
Contributor

@swift-ci Create

@swift-ci
Copy link
Collaborator Author

Comment by Brendan Henderson (JIRA)

Cleaned up the description and added an example fix.

@itaiferber
Copy link
Contributor

Fix went into master in PR-13879; now up against Swift 4.1 at PR-13900

@itaiferber
Copy link
Contributor

Merged to swift-4.1-branch in PR-13900

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

3 participants