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

Json.NET: Reader tries to read too much when "coordinates" is null #65

Closed
airbreather opened this issue Oct 15, 2020 · 0 comments · Fixed by #66
Closed

Json.NET: Reader tries to read too much when "coordinates" is null #65

airbreather opened this issue Oct 15, 2020 · 0 comments · Fixed by #66
Labels
Json.NET Applies to NetTopologySuite.IO.GeoJSON

Comments

@airbreather
Copy link
Member

airbreather commented Oct 15, 2020

var token = JToken.Parse("{\"type\":\"LineString\",\"coordinates\":null}");
var lineString = token.ToObject<LineString>(GeoJsonSerializer.CreateDefault());

In this situation, we will get EndOfStreamException.

The issue is that this code doesn't consider the possibility that we might be on a null token:

case "coordinates":
reader.ReadOrThrow(); //read past start array tag
coords = ReadCoordinates(reader);
break;

As it turns out, 4STJ actually does write out null to signal an empty geometry, as shown in the longer repro below, which effectively means that the Json.NET version doesn't interoperate properly with the 4STJ version 😞:

var stjOptions = new System.Text.Json.JsonSerializerOptions
{
    Converters = { new NetTopologySuite.IO.Converters.GeoJsonConverterFactory() },
};

var seq = GeometryFactory.Default.CoordinateSequenceFactory.Create(0, 2);
var lineString = GeometryFactory.Default.CreateLineString(seq);
string s = System.Text.Json.JsonSerializer.Serialize(lineString, stjOptions);

var jsonNetSerializer = NetTopologySuite.IO.GeoJsonSerializer.CreateDefault();
var jsonNetToken = Newtonsoft.Json.Linq.JToken.Parse(s);
var roundTrip = jsonNetToken.ToObject<LineString>(jsonNetSerializer);
@airbreather airbreather added the Json.NET Applies to NetTopologySuite.IO.GeoJSON label Oct 15, 2020
airbreather added a commit that referenced this issue Oct 16, 2020
airbreather added a commit that referenced this issue Oct 16, 2020
RFC 7946 doesn't say one way or the other what to do with null here, and I can't find support elsewhere for what we were doing.  It does, however, explicitly call out an empty array, and I do see other implementations using this, so we should prefer doing it this way.

Related to #65
airbreather added a commit that referenced this issue Oct 16, 2020
airbreather added a commit that referenced this issue Oct 16, 2020
RFC 7946 doesn't say one way or the other what to do with null here, and I can't find support elsewhere for what we were doing.  It does, however, explicitly call out an empty array, and I do see other implementations using this, so we should prefer doing it this way.

Related to #65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Json.NET Applies to NetTopologySuite.IO.GeoJSON
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant