Fixed deserialization of negative TimeSpans. (#5075) (#5076)

ChilliCream · Jun 9, 2022 · 5c712c4 · 5c712c4
1 parent 054b87a
commit 5c712c4
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 12 deletions.
diff --git a/src/HotChocolate/Core/src/Types/HotChocolate.Types.csproj b/src/HotChocolate/Core/src/Types/HotChocolate.Types.csproj
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="Current">
+<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="Current">
 
   <PropertyGroup>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
@@ -27,6 +27,7 @@
     <!--Tests-->
     <InternalsVisibleTo Include="HotChocolate.Types.Filters.Tests" />
     <InternalsVisibleTo Include="HotChocolate.Types.Sorting.Tests" />
+    <InternalsVisibleTo Include="HotChocolate.Types.Tests" />
     <InternalsVisibleTo Include="HotChocolate.AspNetCore.Tests" />
     <InternalsVisibleTo Include="HotChocolate.Data.Filters.Tests" />
     <InternalsVisibleTo Include="HotChocolate.Data.Sorting.Tests" />

diff --git a/src/HotChocolate/Core/src/Types/Types/Scalars/Iso8601Duration.cs b/src/HotChocolate/Core/src/Types/Types/Scalars/Iso8601Duration.cs
@@ -8,8 +8,6 @@ namespace HotChocolate.Types;
 /// </summary>
 internal struct Iso8601Duration
 {
-    private static readonly uint NegativeBit = 0x80000000;
-
     [Flags]
     private enum Parts
     {
@@ -36,10 +34,10 @@ private enum Parts
         int minutes,
         int seconds,
         uint nanoseconds,
+        bool isNegative,
         out TimeSpan? result)
     {
         ulong ticks = 0;
-        var isNegative = (nanoseconds & NegativeBit) != 0;
 
         // Throw error if result cannot fit into a long
         try
@@ -115,7 +113,8 @@ internal static bool TryParse(string s, out TimeSpan? result)
         int hours = default;
         int minutes = default;
         int seconds = default;
-        uint nanoseconds; // High bit is used to indicate whether duration is negative
+        uint nanoseconds = default;
+        bool isNegative = false;
 
         Parts parts = Parts.HasNone;
 
@@ -134,7 +133,7 @@ internal static bool TryParse(string s, out TimeSpan? result)
         if (s[pos] == '-')
         {
             pos++;
-            nanoseconds = NegativeBit;
+            isNegative = true;
         }
         else
         {
@@ -341,7 +340,16 @@ internal static bool TryParse(string s, out TimeSpan? result)
                 return false;
             }
 
-            return TryToTimeSpan(years, months, weeks, days, hours, minutes, seconds, nanoseconds, out result);
+            return TryToTimeSpan(years,
+                months,
+                weeks,
+                days,
+                hours,
+                minutes,
+                seconds,
+                nanoseconds,
+                isNegative,
+                out result);
         }
 
         return false;

diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Scalars/Iso8601DurationTests.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Scalars/Iso8601DurationTests.cs
@@ -0,0 +1,27 @@
+using System;
+using System.Collections.Generic;
+using Xunit;
+
+namespace HotChocolate.Types;
+
+public class Iso8601DurationTests
+{
+    public static IEnumerable<object[]> TryParseTests => new List<object[]>
+    {
+        new object[] { "-P1D", TimeSpan.FromDays(-1) },
+        new object[] { "PT0.0000001S", TimeSpan.FromMilliseconds(1) / 1000 / 10 },
+        new object[] { "-PT0.0000001S", TimeSpan.FromMilliseconds(-1) / 1000 / 10 },
+    };
+
+    [Theory]
+    [MemberData(nameof(TryParseTests))]
+    public void TryParse(string duration, TimeSpan? expected)
+    {
+        // act
+        var result = Iso8601Duration.TryParse(duration, out var actual);
+
+        // assert
+        Assert.True(result);
+        Assert.Equal(expected, actual);
+    }
+}
diff --git a/src/StrawberryShake/Client/src/Core/Serialization/Iso8601Duration.cs b/src/StrawberryShake/Client/src/Core/Serialization/Iso8601Duration.cs
@@ -8,8 +8,6 @@ namespace StrawberryShake.Serialization;
 /// </summary>
 internal struct Iso8601Duration
 {
-    private static readonly uint NegativeBit = 0x80000000;
-
     [Flags]
     private enum Parts
     {
@@ -36,10 +34,10 @@ private enum Parts
         int minutes,
         int seconds,
         uint nanoseconds,
+        bool isNegative,
         out TimeSpan? result)
     {
         ulong ticks = 0;
-        var isNegative = (nanoseconds & NegativeBit) != 0;
 
         // Throw error if result cannot fit into a long
         try
@@ -116,7 +114,8 @@ internal static bool TryParse(string s, out TimeSpan? result)
         int hours = default;
         int minutes = default;
         int seconds = default;
-        uint nanoseconds; // High bit is used to indicate whether duration is negative
+        uint nanoseconds = default;
+        bool isNegative = false;
 
         Parts parts = Parts.HasNone;
 
@@ -135,7 +134,7 @@ internal static bool TryParse(string s, out TimeSpan? result)
         if (s[pos] == '-')
         {
             pos++;
-            nanoseconds = NegativeBit;
+            isNegative = true;
         }
         else
         {
@@ -354,6 +353,7 @@ internal static bool TryParse(string s, out TimeSpan? result)
                 minutes,
                 seconds,
                 nanoseconds,
+                isNegative,
                 out result);
         }
 

diff --git a/src/StrawberryShake/Client/src/Core/StrawberryShake.Core.csproj b/src/StrawberryShake/Client/src/Core/StrawberryShake.Core.csproj
@@ -8,6 +8,10 @@
     <Description>Abstractions and foundational APIs for StrawberryShake GraphQL clients.</Description>
   </PropertyGroup>
 
+  <ItemGroup>
+    <InternalsVisibleTo Include="StrawberryShake.Core.Tests" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="3.1.0" />
     <PackageReference Include="System.Collections.Immutable" Version="5.0.0" />

diff --git a/src/StrawberryShake/Client/test/Core.Tests/Serialization/Iso8601DurationTests.cs b/src/StrawberryShake/Client/test/Core.Tests/Serialization/Iso8601DurationTests.cs
@@ -0,0 +1,28 @@
+using System;
+using System.Collections.Generic;
+using Xunit;
+
+namespace StrawberryShake.Serialization
+{
+    public class Iso8601DurationTests
+    {
+        public static IEnumerable<object[]> TryParseTests => new List<object[]>
+        {
+            new object[] { "-P1D", TimeSpan.FromDays(-1) },
+            new object[] { "PT0.0000001S", TimeSpan.FromMilliseconds(1) / 1000 / 10 },
+            new object[] { "-PT0.0000001S", TimeSpan.FromMilliseconds(-1) / 1000 / 10 }
+        };
+
+        [Theory]
+        [MemberData(nameof(TryParseTests))]
+        public void TryParse(string duration, TimeSpan? expected)
+        {
+            // act
+            var result = Iso8601Duration.TryParse(duration, out var actual);
+
+            // assert
+            Assert.True(result);
+            Assert.Equal(expected, actual);
+        }
+    }
+}