Added '{' (object start) handling in ReadStringValue.

[bang75]

[elgonzo]

I can't say whether this is the correct way to resolve the issue, but just a note based on me skimming over your pull request: When a json value is expected, it is not sufficient to just test for an erroneous json object by checking the occurence of {, because there could also be an erroneous json array being present instead of an erroneous json object. Thus, the occurence of either { or [ should be checked (and the unit tests should also include additional testing using a mismatched json array).

[bang75]

I'm fully aware of that this might not be a correct solution, but checking for '{' is actually not for detecting an object just not to read past the '{'. An exception will still be thrown and error handling involved, but instead of an exception inside the internal error handling and deserialization returning null, the Skip() will now work and deserialization continues.

Today was the first time for me looking at the Newtonsoft.Json source so I would be nothing but happy if someone more involved could for start confirm that this is actually a bug and if so, suggest a correct solution :)

[elgonzo]

but checking for '{' is actually not for detecting an object just not to read past the '{'. An exception will still be thrown and error handling involved, but instead of an exception inside the internal error handling and deserialization returning null, the Skip() will now work and deserialization continues.

Yeah, but the { is only there if there is a json object. If you replace the json object with a json array in the json data, like:

{
    "Prop1": [ "a", "b"],                  <---- array instead of object
    "Prop2": "Test Value 2"
}

you are essentially still facing the same problem scenario. However, if there is a json array instead of a json object, there will be an [ instead of { being "eaten". Your current pull request only accounts for json object occuring instead of expected json value, but it does not account for json array occuring instead of expected json value.

But i agree with you, it's probably best if the author or someone else with intimate knowledge of the library would look at the issue, because it's not the first time someone had some issues with Newtonsoft.Json's error handling (like this issue here, for example : https://github.com/JamesNK/Newtonsoft.Json/issues/2316)

[bang75]

Exactly! And I first did another solution making the JsonTextReader go back to start reading position when an exception occured and that would have solved all cases but also changed the fundamental behaviour of the execption handling in JsonTextReader and needed 2 or 3 tests to be modified to still be succeessfull, so with my limited knowledge of the code I did this change instead just to get the ball rolling :)