joeldev/ADNKit

__NSTaggedDate in NSJSONSerialization

ffried opened this issue · 7 comments

ffried commented

I noticed a bug when trying to update a stream marker. After I called the updateStreamMarker method of the ANKClient it crashed. Error message: Error with NSJSONSerialization - Invalid type in JSON write (__NSTaggedDate).
I traced it down to the JSON Serialization of the ANKStreamMarker object and found out, that dates appear as NSTaggedDate which is a private class used by Apple. It then tries to get a JSON object from the ANKValueTransformations class where no method JSONObjectFromNSTaggedDate exists of course. Adding such a method fixed the bug for now, but I don't think that using a private class is a good solution.
Any better ideas?

marramgrass commented

I have a branch in my fork with the same fix - see the diff. I'd been meaning to bring it up here, so thanks :)

This isn't actually using a private class, though, is it? It's simply dealing with how an NSDate may internally be an __NSTaggedDate, which the JSON serializer isn't happy with.

ffried commented

Well, what happens if Apple spontaneously decides to use a new class (__NSMarkedDate for example) instead of __NSTaggedDate internally? They don't have to announce that publicly and still every app using ADNKit to send any dates will crash. That's what I mean with "using a private class". I already thought about checking for NS* classes in the JSON Serialization method and not looking after their superclasses. Still not a really nice solution though.

marramgrass commented

I agree, but I'm not sure how better to handle it. I think what we're left with is the approach taken here for __NSCF* - but even that ultimately comes down to catching problem cases as they're observed.

rrbrambley commented

Hmm, I just got this crash, but had never seen it til now. Any final solution on this?

berg commented

@rrbrambley you may want to try using [value classForCoder] ?: [value class] here: https://github.com/joeldev/ADNKit/blame/master/ADNKit/ANKResource.m#L288

I don't think that case was necessarily handled properly when we added classForCoder elsewhere.

rrbrambley commented

@berg Yes, you are right, this seems to fix it.

berg commented

send in a PR!