diff --git a/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjFeatureCollectionConverter.cs b/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjFeatureCollectionConverter.cs index 514070a..828ec78 100644 --- a/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjFeatureCollectionConverter.cs +++ b/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjFeatureCollectionConverter.cs @@ -65,7 +65,7 @@ public override FeatureCollection Read(ref Utf8JsonReader reader, Type objectTyp else { reader.ReadOrThrow(); - reader.Skip(); + reader.SkipOrThrow(); reader.ReadOrThrow(); } } diff --git a/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjFeatureConverter.cs b/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjFeatureConverter.cs index 5c2bcda..df1e947 100644 --- a/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjFeatureConverter.cs +++ b/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjFeatureConverter.cs @@ -162,7 +162,7 @@ public override IFeature Read(ref Utf8JsonReader reader, Type objectType, JsonSe default: // If property name is not one of the above: skip it entirely (foreign member) - reader.Skip(); + reader.SkipOrThrow(); // Advance while (reader.Read()) { diff --git a/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjGeometryConverter.cs b/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjGeometryConverter.cs index e5d4c1a..a1304d7 100644 --- a/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjGeometryConverter.cs +++ b/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/StjGeometryConverter.cs @@ -96,7 +96,7 @@ public override Geometry Read(ref Utf8JsonReader reader, Type typeToConvert, Jso break; default: // included "bbox" property //read, but can't do anything with it (see NetTopologySuite.IO.GeoJSON => NetTopologySuite.IO.Converters.GeometryConverter.ParseGeometry) - reader.Skip(); + reader.SkipOrThrow(); reader.Read(); break; } diff --git a/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/Utility.cs b/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/Utility.cs index 52829e0..08c1b02 100644 --- a/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/Utility.cs +++ b/src/NetTopologySuite.IO.GeoJSON4STJ/Converters/Utility.cs @@ -61,6 +61,21 @@ internal static void AssertToken(this ref Utf8JsonReader reader, JsonTokenType r } } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal static void SkipOrThrow(this ref Utf8JsonReader reader) + { + // #143: Even though JsonConverter always seems to see a Utf8JsonReader positioned on + // a full node, that node might be contained in a larger incomplete block. For whatever + // reason, reader.Skip() throws JsonException immediately if the reader is in a partial + // block, even if the current block has plenty of data to read past it. TrySkip will do + // the right thing, but we should still check the return value to make sure that we get + // a JsonException if the reader *does* terminate abruptly (airbreather 2024-04-24). + if (!reader.TrySkip()) + { + ThrowForUnexpectedPartialJson(); + } + } + internal static object ObjectFromJsonNode(JsonNode node, JsonSerializerOptions serializerOptions) { switch (node) @@ -93,6 +108,10 @@ internal static JsonNode ObjectToJsonNode(object obj, JsonSerializerOptions seri private static void ThrowForUnexpectedEndOfStream() => throw new JsonException(Resources.EX_UnexpectedEndOfStream); + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ThrowForUnexpectedPartialJson() + => throw new JsonException(Resources.EX_UnexpectedPartialJson); + [MethodImpl(MethodImplOptions.NoInlining)] private static void ThrowForUnexpectedToken(JsonTokenType requiredNextTokenType, ref Utf8JsonReader reader) => throw new JsonException(string.Format(Resources.EX_UnexpectedToken, requiredNextTokenType, reader.TokenType, CurrentTokenAsString(in reader))); diff --git a/src/NetTopologySuite.IO.GeoJSON4STJ/Properties/Resources.Designer.cs b/src/NetTopologySuite.IO.GeoJSON4STJ/Properties/Resources.Designer.cs index 46224fc..36acb56 100644 --- a/src/NetTopologySuite.IO.GeoJSON4STJ/Properties/Resources.Designer.cs +++ b/src/NetTopologySuite.IO.GeoJSON4STJ/Properties/Resources.Designer.cs @@ -131,5 +131,14 @@ internal static string EX_UnexpectedToken { return ResourceManager.GetString("EX_UnexpectedToken", resourceCulture); } } + + /// + /// Looks up a localized string similar to JsonConverter received partial JSON. This is likely the result of a bug in the System.Text.Json library.. + /// + internal static string EX_UnexpectedPartialJson { + get { + return ResourceManager.GetString("EX_UnexpectedPartialJson", resourceCulture); + } + } } } diff --git a/src/NetTopologySuite.IO.GeoJSON4STJ/Properties/Resources.resx b/src/NetTopologySuite.IO.GeoJSON4STJ/Properties/Resources.resx index d0f48e0..163a1bd 100644 --- a/src/NetTopologySuite.IO.GeoJSON4STJ/Properties/Resources.resx +++ b/src/NetTopologySuite.IO.GeoJSON4STJ/Properties/Resources.resx @@ -141,4 +141,7 @@ Expected token is '{0}' but was '{1}' (Value '{2}'). + + JsonConverter received partial JSON. This is likely the result of a bug in the System.Text.Json library. + \ No newline at end of file diff --git a/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/FeatureCollectionConverterTest.cs b/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/FeatureCollectionConverterTest.cs index ef55457..1a1b323 100644 --- a/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/FeatureCollectionConverterTest.cs +++ b/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/FeatureCollectionConverterTest.cs @@ -1,4 +1,6 @@ using System; +using System.Linq; +using System.Text.Json; using System.Text.Json.Serialization; using NetTopologySuite.Features; using NetTopologySuite.Geometries; @@ -68,5 +70,20 @@ public void TestSanD(OgcGeometryType type, int num, bool threeD) for (int i = 0; i < fc.Count; i++) FeatureConverterTest.CheckEquality(fc[i], d[i]); } + + [Test] + [GeoJsonIssueNumber(143)] + public void SkippedPropertiesShouldNotThrowWithCompleteObjectInPartialBuffer() + { + var options = DefaultOptions; + var node = JsonSerializer.SerializeToNode(new FeatureCollection(), options)!; + node["_skippedProperty"] = "irrelevant"; + var nodes = Enumerable.Repeat(node, 500).ToArray(); + using SingleByteReadingMemoryStream stream = new(); + JsonSerializer.Serialize(stream, nodes, options); + stream.Position = 0; + var roundtrip = JsonSerializer.Deserialize(stream, options); + Assert.That(roundtrip, Has.Length.EqualTo(500)); + } } } diff --git a/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/FeatureConverterTest.cs b/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/FeatureConverterTest.cs index 7dbc9e3..d18c9b8 100644 --- a/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/FeatureConverterTest.cs +++ b/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/FeatureConverterTest.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Text.Json; using System.Text.Json.Serialization; @@ -128,6 +129,21 @@ public void WriteJsonWithIdTest(string idPropertyName, object id) CheckEquality(value, deserialized, idPropertyName); } + [Test] + [GeoJsonIssueNumber(143)] + public void SkippedPropertiesShouldNotThrowWithCompleteObjectInPartialBuffer() + { + var options = DefaultOptions; + var node = JsonSerializer.SerializeToNode(new Feature(), options)!; + node["_skippedProperty"] = "irrelevant"; + var nodes = Enumerable.Repeat(node, 500).ToArray(); + using SingleByteReadingMemoryStream stream = new(); + JsonSerializer.Serialize(stream, nodes, options); + stream.Position = 0; + var roundtrip = JsonSerializer.Deserialize(stream, options); + Assert.That(roundtrip, Has.Length.EqualTo(500)); + } + public static IEnumerable FeatureIdTestCases { get diff --git a/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/GeometryConverterTest.cs b/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/GeometryConverterTest.cs index 8a1ddf4..efbb1e0 100644 --- a/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/GeometryConverterTest.cs +++ b/test/NetTopologySuite.IO.GeoJSON4STJ.Test/Converters/GeometryConverterTest.cs @@ -1,4 +1,5 @@ using System.IO; +using System.Linq; using System.Text.Json; using NetTopologySuite.Geometries; using NUnit.Framework; @@ -184,5 +185,20 @@ public void TestWriteReadWkt(string wkt) Assert.That(geomS.IsEmpty ? geomD.IsEmpty : geomS.EqualsTopologically(geomD)); } + + [Test] + [GeoJsonIssueNumber(143)] + public void SkippedPropertiesShouldNotThrowWithCompleteObjectInPartialBuffer() + { + var options = DefaultOptions; + var node = JsonSerializer.SerializeToNode(Point.Empty, options)!; + node["_skippedProperty"] = "irrelevant"; + var nodes = Enumerable.Repeat(node, 500).ToArray(); + using SingleByteReadingMemoryStream stream = new(); + JsonSerializer.Serialize(stream, nodes, options); + stream.Position = 0; + var roundtrip = JsonSerializer.Deserialize(stream, options); + Assert.That(roundtrip, Has.Length.EqualTo(500)); + } } } diff --git a/test/NetTopologySuite.IO.GeoJSON4STJ.Test/SingleByteReadingMemoryStream.cs b/test/NetTopologySuite.IO.GeoJSON4STJ.Test/SingleByteReadingMemoryStream.cs new file mode 100644 index 0000000..c132e65 --- /dev/null +++ b/test/NetTopologySuite.IO.GeoJSON4STJ.Test/SingleByteReadingMemoryStream.cs @@ -0,0 +1,32 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +namespace NetTopologySuite.IO.GeoJSON4STJ.Test; + +/// +/// A implementation that will never read more than one byte at a time. +/// +internal sealed class SingleByteReadingMemoryStream : MemoryStream +{ + public override int Read(byte[] buffer, int offset, int count) + { + return base.Read(buffer, offset, Math.Min(1, count)); + } + + public override int Read(Span buffer) + { + return base.Read(buffer[.. Math.Min(1, buffer.Length)]); + } + + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + return base.ReadAsync(buffer, offset, Math.Min(1, count), cancellationToken); + } + + public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { + return base.ReadAsync(buffer[.. Math.Min(1, buffer.Length)], cancellationToken); + } +}