Fixes to JSON timestamp/duration representations

1 files changed, 17 insertions, 12 deletions

diff --git a/csharp/src/Google.Protobuf/JsonFormatter.cs b/csharp/src/Google.Protobuf/JsonFormatter.cs
index bde17903..563b834e 100644
--- a/csharp/src/Google.Protobuf/JsonFormatter.cs
+++ b/csharp/src/Google.Protobuf/JsonFormatter.cs
@@ -485,13 +485,14 @@ namespace Google.Protobuf
             int nanos = (int) value.Descriptor.Fields[Timestamp.NanosFieldNumber].Accessor.GetValue(value);
             long seconds = (long) value.Descriptor.Fields[Timestamp.SecondsFieldNumber].Accessor.GetValue(value);
 
-            // Even if the original message isn't using the built-in classes, we can still build one... and then
-            // rely on it being normalized.
-            Timestamp normalized = Timestamp.Normalize(seconds, nanos);
+            // Even if the original message isn't using the built-in classes, we can still build one... and its
+            // conversion will check whether or not it's normalized.
+            // TODO: Perhaps the diagnostic-only formatter should not throw for non-normalized values?
+            Timestamp ts = new Timestamp { Seconds = seconds, Nanos = nanos };
             // Use .NET's formatting for the value down to the second, including an opening double quote (as it's a string value)
-            DateTime dateTime = normalized.ToDateTime();
+            DateTime dateTime = ts.ToDateTime();
             builder.Append(dateTime.ToString("yyyy'-'MM'-'dd'T'HH:mm:ss", CultureInfo.InvariantCulture));
-            AppendNanoseconds(builder, Math.Abs(normalized.Nanos));
+            AppendNanoseconds(builder, Math.Abs(ts.Nanos));
             builder.Append("Z\"");
         }
 
@@ -502,18 +503,22 @@ namespace Google.Protobuf
             int nanos = (int) value.Descriptor.Fields[Duration.NanosFieldNumber].Accessor.GetValue(value);
             long seconds = (long) value.Descriptor.Fields[Duration.SecondsFieldNumber].Accessor.GetValue(value);
 
+            // TODO: Perhaps the diagnostic-only formatter should not throw for non-normalized values?
             // Even if the original message isn't using the built-in classes, we can still build one... and then
             // rely on it being normalized.
-            Duration normalized = Duration.Normalize(seconds, nanos);
+            if (!Duration.IsNormalized(seconds, nanos))
+            {
+                throw new InvalidOperationException("Non-normalized duration value");
+            }
 
             // The seconds part will normally provide the minus sign if we need it, but not if it's 0...
-            if (normalized.Seconds == 0 && normalized.Nanos < 0)
+            if (seconds == 0 && nanos < 0)
             {
                 builder.Append('-');
             }
 
-            builder.Append(normalized.Seconds.ToString("d", CultureInfo.InvariantCulture));
-            AppendNanoseconds(builder, Math.Abs(normalized.Nanos));
+            builder.Append(seconds.ToString("d", CultureInfo.InvariantCulture));
+            AppendNanoseconds(builder, Math.Abs(nanos));
             builder.Append("s\"");
         }
 
@@ -598,15 +603,15 @@ namespace Google.Protobuf
                 // Output to 3, 6 or 9 digits.
                 if (nanos % 1000000 == 0)
                 {
-                    builder.Append((nanos / 1000000).ToString("d", CultureInfo.InvariantCulture));
+                    builder.Append((nanos / 1000000).ToString("d3", CultureInfo.InvariantCulture));
                 }
                 else if (nanos % 1000 == 0)
                 {
-                    builder.Append((nanos / 1000).ToString("d", CultureInfo.InvariantCulture));
+                    builder.Append((nanos / 1000).ToString("d6", CultureInfo.InvariantCulture));
                 }
                 else
                 {
-                    builder.Append(nanos.ToString("d", CultureInfo.InvariantCulture));
+                    builder.Append(nanos.ToString("d9", CultureInfo.InvariantCulture));
                 }
             }
         }