Do not overwrite JsonException metadata (#56903)

* Do not overwrite JsonException metadata. Fix #51537.

* remove redundant semicolon

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.ReadCore.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.ReadCore.cs
index 5b7d102..ae5ae40 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.ReadCore.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.ReadCore.cs
@@ -76,22 +76,27 @@ namespace System.Text.Json.Serialization
             }
             catch (JsonReaderException ex)
             {
-                ThrowHelper.ReThrowWithPath(state, ex);
+                ThrowHelper.ReThrowWithPath(ref state, ex);
                 return default;
             }
             catch (FormatException ex) when (ex.Source == ThrowHelper.ExceptionSourceValueToRethrowAsJsonException)
             {
-                ThrowHelper.ReThrowWithPath(state, reader, ex);
+                ThrowHelper.ReThrowWithPath(ref state, reader, ex);
                 return default;
             }
             catch (InvalidOperationException ex) when (ex.Source == ThrowHelper.ExceptionSourceValueToRethrowAsJsonException)
             {
-                ThrowHelper.ReThrowWithPath(state, reader, ex);
+                ThrowHelper.ReThrowWithPath(ref state, reader, ex);
                 return default;
             }
-            catch (JsonException ex)
+            catch (JsonException ex) when (ex.Path == null)
             {
-                ThrowHelper.AddJsonExceptionInformation(state, reader, ex);
+                // JsonExceptions where the Path property is already set
+                // typically originate from nested calls to JsonSerializer;
+                // treat these cases as any other exception type and do not
+                // overwrite any exception information.
+
+                ThrowHelper.AddJsonExceptionInformation(ref state, reader, ex);
                 throw;
             }
             catch (NotSupportedException ex)
@@ -103,7 +108,7 @@ namespace System.Text.Json.Serialization
                     throw;
                 }
 
-                ThrowHelper.ThrowNotSupportedException(state, reader, ex);
+                ThrowHelper.ThrowNotSupportedException(ref state, reader, ex);
                 return default;
             }
         }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.WriteCore.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.WriteCore.cs
index 1d60dbf..da003ec 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.WriteCore.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.WriteCore.cs
@@ -42,12 +42,17 @@ namespace System.Text.Json.Serialization
             }
             catch (InvalidOperationException ex) when (ex.Source == ThrowHelper.ExceptionSourceValueToRethrowAsJsonException)
             {
-                ThrowHelper.ReThrowWithPath(state, ex);
+                ThrowHelper.ReThrowWithPath(ref state, ex);
                 throw;
             }
-            catch (JsonException ex)
+            catch (JsonException ex) when (ex.Path == null)
             {
-                ThrowHelper.AddJsonExceptionInformation(state, ex);
+                // JsonExceptions where the Path property is already set
+                // typically originate from nested calls to JsonSerializer;
+                // treat these cases as any other exception type and do not
+                // overwrite any exception information.
+
+                ThrowHelper.AddJsonExceptionInformation(ref state, ex);
                 throw;
             }
             catch (NotSupportedException ex)
@@ -59,7 +64,7 @@ namespace System.Text.Json.Serialization
                     throw;
                 }
 
-                ThrowHelper.ThrowNotSupportedException(state, ex);
+                ThrowHelper.ThrowNotSupportedException(ref state, ex);
                 return default;
             }
         }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs
index 8a2abde..b1cee07 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Utf8JsonReader.cs
@@ -399,7 +399,7 @@ namespace System.Text.Json
             {
                 reader = restore;
                 // Re-throw with Path information.
-                ThrowHelper.ReThrowWithPath(state, ex);
+                ThrowHelper.ReThrowWithPath(ref state, ex);
             }
 
             int length = valueSpan.IsEmpty ? checked((int)valueSequence.Length) : valueSpan.Length;

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
index b8df6c1..7a3984b 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
@@ -52,9 +52,7 @@ namespace System.Text.Json
         [MethodImpl(MethodImplOptions.NoInlining)]
         public static void ThrowJsonException_DeserializeUnableToConvertValue(Type propertyType)
         {
-            var ex = new JsonException(SR.Format(SR.DeserializeUnableToConvertValue, propertyType));
-            ex.AppendPathInformation = true;
-            throw ex;
+            throw new JsonException(SR.Format(SR.DeserializeUnableToConvertValue, propertyType)) { AppendPathInformation = true };
         }
 
         [DoesNotReturn]
@@ -75,43 +73,28 @@ namespace System.Text.Json
         [MethodImpl(MethodImplOptions.NoInlining)]
         public static void ThrowJsonException_SerializationConverterRead(JsonConverter? converter)
         {
-            var ex = new JsonException(SR.Format(SR.SerializationConverterRead, converter));
-            ex.AppendPathInformation = true;
-            throw ex;
+            throw new JsonException(SR.Format(SR.SerializationConverterRead, converter)) { AppendPathInformation = true };
         }
 
         [DoesNotReturn]
         [MethodImpl(MethodImplOptions.NoInlining)]
         public static void ThrowJsonException_SerializationConverterWrite(JsonConverter? converter)
         {
-            var ex = new JsonException(SR.Format(SR.SerializationConverterWrite, converter));
-            ex.AppendPathInformation = true;
-            throw ex;
+            throw new JsonException(SR.Format(SR.SerializationConverterWrite, converter)) { AppendPathInformation = true };
         }
 
         [DoesNotReturn]
         [MethodImpl(MethodImplOptions.NoInlining)]
         public static void ThrowJsonException_SerializerCycleDetected(int maxDepth)
         {
-            throw new JsonException(SR.Format(SR.SerializerCycleDetected, maxDepth));
+            throw new JsonException(SR.Format(SR.SerializerCycleDetected, maxDepth)) { AppendPathInformation = true };
         }
 
         [DoesNotReturn]
         [MethodImpl(MethodImplOptions.NoInlining)]
         public static void ThrowJsonException(string? message = null)
         {
-            JsonException ex;
-            if (string.IsNullOrEmpty(message))
-            {
-                ex = new JsonException();
-            }
-            else
-            {
-                ex = new JsonException(message);
-                ex.AppendPathInformation = true;
-            }
-
-            throw ex;
+            throw new JsonException(message) { AppendPathInformation = true };
         }
 
         [DoesNotReturn]
@@ -296,12 +279,12 @@ namespace System.Text.Json
 
             NotSupportedException ex = new NotSupportedException(
                 SR.Format(SR.ObjectWithParameterizedCtorRefMetadataNotHonored, state.Current.JsonTypeInfo.Type));
-            ThrowNotSupportedException(state, reader, ex);
+            ThrowNotSupportedException(ref state, reader, ex);
         }
 
         [DoesNotReturn]
         [MethodImpl(MethodImplOptions.NoInlining)]
-        public static void ReThrowWithPath(in ReadStack state, JsonReaderException ex)
+        public static void ReThrowWithPath(ref ReadStack state, JsonReaderException ex)
         {
             Debug.Assert(ex.Path == null);
 
@@ -328,15 +311,17 @@ namespace System.Text.Json
 
         [DoesNotReturn]
         [MethodImpl(MethodImplOptions.NoInlining)]
-        public static void ReThrowWithPath(in ReadStack state, in Utf8JsonReader reader, Exception ex)
+        public static void ReThrowWithPath(ref ReadStack state, in Utf8JsonReader reader, Exception ex)
         {
             JsonException jsonException = new JsonException(null, ex);
-            AddJsonExceptionInformation(state, reader, jsonException);
+            AddJsonExceptionInformation(ref state, reader, jsonException);
             throw jsonException;
         }
 
-        public static void AddJsonExceptionInformation(in ReadStack state, in Utf8JsonReader reader, JsonException ex)
+        public static void AddJsonExceptionInformation(ref ReadStack state, in Utf8JsonReader reader, JsonException ex)
         {
+            Debug.Assert(ex.Path is null); // do not overwrite existing path information
+
             long lineNumber = reader.CurrentState._lineNumber;
             ex.LineNumber = lineNumber;
 
@@ -370,15 +355,17 @@ namespace System.Text.Json
 
         [DoesNotReturn]
         [MethodImpl(MethodImplOptions.NoInlining)]
-        public static void ReThrowWithPath(in WriteStack state, Exception ex)
+        public static void ReThrowWithPath(ref WriteStack state, Exception ex)
         {
             JsonException jsonException = new JsonException(null, ex);
-            AddJsonExceptionInformation(state, jsonException);
+            AddJsonExceptionInformation(ref state, jsonException);
             throw jsonException;
         }
 
-        public static void AddJsonExceptionInformation(in WriteStack state, JsonException ex)
+        public static void AddJsonExceptionInformation(ref WriteStack state, JsonException ex)
         {
+            Debug.Assert(ex.Path is null); // do not overwrite existing path information
+
             string path = state.PropertyPath();
             ex.Path = path;
 
@@ -432,7 +419,7 @@ namespace System.Text.Json
         }
 
         [DoesNotReturn]
-        public static void ThrowNotSupportedException(in ReadStack state, in Utf8JsonReader reader, NotSupportedException ex)
+        public static void ThrowNotSupportedException(ref ReadStack state, in Utf8JsonReader reader, NotSupportedException ex)
         {
             string message = ex.Message;
 
@@ -464,7 +451,7 @@ namespace System.Text.Json
         }
 
         [DoesNotReturn]
-        public static void ThrowNotSupportedException(in WriteStack state, NotSupportedException ex)
+        public static void ThrowNotSupportedException(ref WriteStack state, NotSupportedException ex)
         {
             string message = ex.Message;
 
@@ -508,14 +495,14 @@ namespace System.Text.Json
                 message = SR.Format(SR.DeserializeNoConstructor, nameof(JsonConstructorAttribute), type);
             }
 
-            ThrowNotSupportedException(state, reader, new NotSupportedException(message));
+            ThrowNotSupportedException(ref state, reader, new NotSupportedException(message));
         }
 
         [DoesNotReturn]
         [MethodImpl(MethodImplOptions.NoInlining)]
         public static void ThrowNotSupportedException_CannotPopulateCollection(Type type, ref Utf8JsonReader reader, ref ReadStack state)
         {
-            ThrowNotSupportedException(state, reader, new NotSupportedException(SR.Format(SR.CannotPopulateCollection, type)));
+            ThrowNotSupportedException(ref state, reader, new NotSupportedException(SR.Format(SR.CannotPopulateCollection, type)));
         }
 
         [DoesNotReturn]

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.Array.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.Array.cs
index 5ea4fe8..2d5f921 100644 (file)
--- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.Array.cs
+++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.Array.cs
@@ -70,22 +70,14 @@ namespace System.Text.Json.Serialization.Tests
         [Fact]
         public static void CustomArrayConverterFail()
         {
-            string json = $"\"{Int64.MaxValue.ToString()}0\"";
+            string json = $"\"{long.MaxValue}0\"";
 
-            var options = new JsonSerializerOptions();
-            options.Converters.Add(new LongArrayConverter());
+            var options = new JsonSerializerOptions { Converters = { new LongArrayConverter() } };
+            JsonException ex = Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<long[]>(json, options));
 
-            try
-            {
-                JsonSerializer.Deserialize<long[]>(json, options);
-                Assert.True(false, "Expected exception");
-            }
-            catch (JsonException ex)
-            {
-                Assert.Null(ex.InnerException);
-                Assert.Equal("$", ex.Path);
-                Assert.Equal("Too big for a long", ex.Message);
-            }
+            Assert.Null(ex.InnerException);
+            Assert.Equal("$", ex.Path);
+            Assert.Equal("Too big for a long", ex.Message);
         }
 
         private class ClassWithProperty

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ExceptionTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ExceptionTests.cs
index 05cc4f4..9346a64 100644 (file)
--- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ExceptionTests.cs
+++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ExceptionTests.cs
@@ -677,5 +677,38 @@ namespace System.Text.Json.Serialization.Tests
             public StructWithBadCtor_WithProps(SerializationInfo info, StreamingContext ctx) =>
                 (Info, Ctx) = (info, ctx);
         }
+
+        [Fact]
+        public static void CustomConverterThrowingJsonException_Serialization_ShouldNotOverwriteMetadata()
+        {
+            JsonException ex = Assert.Throws<JsonException>(() => JsonSerializer.Serialize(new { Value = new PocoUsingCustomConverterThrowingJsonException() }));
+            Assert.Equal(PocoConverterThrowingCustomJsonException.ExceptionMessage, ex.Message);
+            Assert.Equal(PocoConverterThrowingCustomJsonException.ExceptionPath, ex.Path);
+        }
+
+        [Fact]
+        public static void CustomConverterThrowingJsonException_Deserialization_ShouldNotOverwriteMetadata()
+        {
+            JsonException ex = Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<PocoUsingCustomConverterThrowingJsonException[]>(@"[{}]"));
+            Assert.Equal(PocoConverterThrowingCustomJsonException.ExceptionMessage, ex.Message);
+            Assert.Equal(PocoConverterThrowingCustomJsonException.ExceptionPath, ex.Path);
+        }
+
+        [JsonConverter(typeof(PocoConverterThrowingCustomJsonException))]
+        public class PocoUsingCustomConverterThrowingJsonException
+        {
+        }
+
+        public class PocoConverterThrowingCustomJsonException : JsonConverter<PocoUsingCustomConverterThrowingJsonException>
+        {
+            public const string ExceptionMessage = "Custom JsonException mesage";
+            public const string ExceptionPath = "$.CustomPath";
+
+            public override PocoUsingCustomConverterThrowingJsonException? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+                => throw new JsonException(ExceptionMessage, ExceptionPath, 0, 0);
+
+            public override void Write(Utf8JsonWriter writer, PocoUsingCustomConverterThrowingJsonException value, JsonSerializerOptions options)
+                => throw new JsonException(ExceptionMessage, ExceptionPath, 0, 0);
+        }
     }
 }