JamesNK/Newtonsoft.Json

`StringEnumConverter` throws `JsonSerializationException` when passed `JToken.CreateReader()` as SetToken() is never called.

CBurberry opened this issue · 4 comments

Summary

When calling StringEnumConverter.ReadJson() from within another custom JsonConverter.ReadJson() and passing JToken.CreateReader() as parameter, JsonSerializationException: Unexpected token None when parsing enum. is thrown (line permalink).

Error is thrown because JToken.CreateReader() returns a reader that never calls protected member SetToken() on construction. Since this token is not set by default, the exception as thrown as StringEnumConverter.ReadJson() is if/else'ing on that token type.

Steps to reproduce

//Example calling context: Nested JsonConverter calls on ReadJson
JToken t = JToken.Load(reader);
//...

//Where fieldType is an enum type of the json value being passed and the value is not an integer.
stringEnumConverter.ReadJson(t.CreateReader(), fieldType, null, serializer);

Expected behavior

StringEnumConverter should return a valid enum value matching the string representation. (System.Enum.TryParse returned true when passed the same string).

Actual behavior

JsonSerializationException: Unexpected token None when parsing enum. is thrown.

The reader created by JToken.CreateReader() hasn't been advanced to the first value yet, so it's current value is still None. And many JsonConverters (at least not those that are part of the Newtonsoft.Json package, afaict) do not themselves advance the JsonReader but try to consume its current value -- which of course fails when the JsonReaders current value is still None.

Invoke the Read() method on the JsonReader freshly created by JToken.CreateReader() before passing it to a converter, and Bob's your uncle... (JsonReader.Read() operates conceptually in a similar manner like IEnumerator.MoveNext().)

Invoke the Read() method on the JsonReader freshly created by JToken.CreateReader() before passing it to a converter, and Bob's your uncle...

I'm aware this is how it works for this converter (and frankly that is the workaround I use) however it means needing to type check on the converter type in common code (that other converters may be called) because reading the first token similar to IEnumerator.MoveNext() advances the reader.

it means needing to type check on the converter type in common code

Why exactly would you need to do this; or rather, what has advancing the freshly created JsonReader to do with that? Considering that the deserializer itself passes JsonReader instances to JsonConverters that are already advanced to the value the converter should consume, and therefore JsonConverters typically expecting and having to deal with JsonReaders that are already advanced to the value they should consume, i unfortunately fail to understand this argument. I mean, you doing JsonReader.Read on the reader you created via JToken.CreateReader() is preparing the reader in the same manner as the deserializer would before passing it to a JsonConverter.ReadJson method. So, what exactly is your pain point here?

@elgonzo Given you've well illustrated my lack of understanding here I'll close this issue. Due to using System.Linq a lot and not realizing that JsonReader implements the same pattern I had forgotten IEnumerator/JsonReader starts at -1.

As for the use case, nested custom JsonConverters that were using JObject Linq access to read and pass around readers rather than interacting with the reader directly caused me to scratch my head on this for awhile. JObject calls seemed to access without any prior calls to read and so I thought I was fine passing a reader without advancing to a nested JsonConverter.

Thanks for your review.