Merge pull request #1098 from jskeet/wkt-json

Ensure all formatted well-known-type values are valid JSON

2 files changed, 41 insertions, 25 deletions

diff --git a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs
index 8473b4be..2cf2d5fc 100644
--- a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs
+++ b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs
@@ -37,6 +37,8 @@ using UnitTest.Issues.TestProtos;
 using Google.Protobuf.WellKnownTypes;
 using Google.Protobuf.Reflection;
 
+using static Google.Protobuf.JsonParserTest; // For WrapInQuotes
+
 namespace Google.Protobuf
 {
     /// <summary>
@@ -319,23 +321,28 @@ namespace Google.Protobuf
         }
 
         [Test]
-        public void TimestampStandalone()
+        [TestCase("1970-01-01T00:00:00Z", 0)]
+        [TestCase("1970-01-01T00:00:00.100Z", 100000000)]
+        [TestCase("1970-01-01T00:00:00.120Z", 120000000)]
+        [TestCase("1970-01-01T00:00:00.123Z", 123000000)]
+        [TestCase("1970-01-01T00:00:00.123400Z", 123400000)]
+        [TestCase("1970-01-01T00:00:00.123450Z", 123450000)]
+        [TestCase("1970-01-01T00:00:00.123456Z", 123456000)]
+        [TestCase("1970-01-01T00:00:00.123456700Z", 123456700)]
+        [TestCase("1970-01-01T00:00:00.123456780Z", 123456780)]
+        [TestCase("1970-01-01T00:00:00.123456789Z", 123456789)]
+        public void TimestampStandalone(string expected, int nanos)
         {
-            Assert.AreEqual("1970-01-01T00:00:00Z", new Timestamp().ToString());
-            Assert.AreEqual("1970-01-01T00:00:00.100Z", new Timestamp { Nanos = 100000000 }.ToString());
-            Assert.AreEqual("1970-01-01T00:00:00.120Z", new Timestamp { Nanos = 120000000 }.ToString());
-            Assert.AreEqual("1970-01-01T00:00:00.123Z", new Timestamp { Nanos = 123000000 }.ToString());
-            Assert.AreEqual("1970-01-01T00:00:00.123400Z", new Timestamp { Nanos = 123400000 }.ToString());
-            Assert.AreEqual("1970-01-01T00:00:00.123450Z", new Timestamp { Nanos = 123450000 }.ToString());
-            Assert.AreEqual("1970-01-01T00:00:00.123456Z", new Timestamp { Nanos = 123456000 }.ToString());
-            Assert.AreEqual("1970-01-01T00:00:00.123456700Z", new Timestamp { Nanos = 123456700 }.ToString());
-            Assert.AreEqual("1970-01-01T00:00:00.123456780Z", new Timestamp { Nanos = 123456780 }.ToString());
-            Assert.AreEqual("1970-01-01T00:00:00.123456789Z", new Timestamp { Nanos = 123456789 }.ToString());
+            Assert.AreEqual(WrapInQuotes(expected), new Timestamp { Nanos = nanos }.ToString());
+        }
 
-            // One before and one after the Unix epoch
-            Assert.AreEqual("1673-06-19T12:34:56Z",
+        [Test]
+        public void TimestampStandalone_FromDateTime()
+        {
+            // One before and one after the Unix epoch, more easily represented via DateTime.
+            Assert.AreEqual("\"1673-06-19T12:34:56Z\"",
                 new DateTime(1673, 6, 19, 12, 34, 56, DateTimeKind.Utc).ToTimestamp().ToString());
-            Assert.AreEqual("2015-07-31T10:29:34Z",
+            Assert.AreEqual("\"2015-07-31T10:29:34Z\"",
                 new DateTime(2015, 7, 31, 10, 29, 34, DateTimeKind.Utc).ToTimestamp().ToString());
         }
 
@@ -367,7 +374,7 @@ namespace Google.Protobuf
         [TestCase(1, -100000000, "0.900s")]
         public void DurationStandalone(long seconds, int nanoseconds, string expected)
         {
-            Assert.AreEqual(expected, new Duration { Seconds = seconds, Nanos = nanoseconds }.ToString());
+            Assert.AreEqual(WrapInQuotes(expected), new Duration { Seconds = seconds, Nanos = nanoseconds }.ToString());
         }
 
         [Test]
@@ -399,11 +406,11 @@ namespace Google.Protobuf
         public void FieldMaskStandalone()
         {
             var fieldMask = new FieldMask { Paths = { "", "single", "with_underscore", "nested.field.name", "nested..double_dot" } };
-            Assert.AreEqual(",single,withUnderscore,nested.field.name,nested..doubleDot", fieldMask.ToString());
+            Assert.AreEqual("\",single,withUnderscore,nested.field.name,nested..doubleDot\"", fieldMask.ToString());
 
             // Invalid, but we shouldn't create broken JSON...
             fieldMask = new FieldMask { Paths = { "x\\y" } };
-            Assert.AreEqual(@"x\\y", fieldMask.ToString());
+            Assert.AreEqual(@"""x\\y""", fieldMask.ToString());
         }
 
         [Test]
diff --git a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs
index 0fcf3113..303baacc 100644
--- a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs
+++ b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs
@@ -147,7 +147,7 @@ namespace Google.Protobuf
         {
             ByteString data = ByteString.CopyFrom(1, 2, 3);
             // Can't do this with attributes...
-            var parsed = JsonParser.Default.Parse<BytesValue>("\"" + data.ToBase64() + "\"");
+            var parsed = JsonParser.Default.Parse<BytesValue>(WrapInQuotes(data.ToBase64()));
             var expected = new BytesValue { Value = data };
             Assert.AreEqual(expected, parsed);
         }
@@ -588,9 +588,9 @@ namespace Google.Protobuf
         public void Timestamp_Valid(string jsonValue, string expectedFormatted)
         {
             expectedFormatted = expectedFormatted ?? jsonValue;
-            string json = "\"" + jsonValue + "\"";
+            string json = WrapInQuotes(jsonValue);
             var parsed = Timestamp.Parser.ParseJson(json);
-            Assert.AreEqual(expectedFormatted, parsed.ToString());
+            Assert.AreEqual(WrapInQuotes(expectedFormatted), parsed.ToString());
         }
         
         [Test]
@@ -615,7 +615,7 @@ namespace Google.Protobuf
         [TestCase("2100-02-29T14:46:23.123456789Z", Description = "Feb 29th on a non-leap-year")]
         public void Timestamp_Invalid(string jsonValue)
         {
-            string json = "\"" + jsonValue + "\"";
+            string json = WrapInQuotes(jsonValue);
             Assert.Throws<InvalidProtocolBufferException>(() => Timestamp.Parser.ParseJson(json));
         }
 
@@ -689,9 +689,9 @@ namespace Google.Protobuf
         public void Duration_Valid(string jsonValue, string expectedFormatted)
         {
             expectedFormatted = expectedFormatted ?? jsonValue;
-            string json = "\"" + jsonValue + "\"";
+            string json = WrapInQuotes(jsonValue);
             var parsed = Duration.Parser.ParseJson(json);
-            Assert.AreEqual(expectedFormatted, parsed.ToString());
+            Assert.AreEqual(WrapInQuotes(expectedFormatted), parsed.ToString());
         }
 
         // The simplest way of testing that the value has parsed correctly is to reformat it,
@@ -720,7 +720,7 @@ namespace Google.Protobuf
         [TestCase("-3155760000000s", Description = "Integer part too long (negative)")]
         public void Duration_Invalid(string jsonValue)
         {
-            string json = "\"" + jsonValue + "\"";
+            string json = WrapInQuotes(jsonValue);
             Assert.Throws<InvalidProtocolBufferException>(() => Duration.Parser.ParseJson(json));
         }
 
@@ -736,7 +736,7 @@ namespace Google.Protobuf
         [TestCase("fooBar.bazQux", "foo_bar.baz_qux")]
         public void FieldMask_Valid(string jsonValue, params string[] expectedPaths)
         {
-            string json = "\"" + jsonValue + "\"";
+            string json = WrapInQuotes(jsonValue);
             var parsed = FieldMask.Parser.ParseJson(json);
             CollectionAssert.AreEqual(expectedPaths, parsed.Paths);
         }
@@ -812,7 +812,16 @@ namespace Google.Protobuf
 
             var parser63 = new JsonParser(new JsonParser.Settings(63));
             Assert.Throws<InvalidProtocolBufferException>(() => parser63.Parse<TestRecursiveMessage>(data64));
+        }
 
+        /// <summary>
+        /// Various tests use strings which have quotes round them for parsing or as the result
+        /// of formatting, but without those quotes being specified in the tests (for the sake of readability).
+        /// This method simply returns the input, wrapped in double quotes.
+        /// </summary>
+        internal static string WrapInQuotes(string text)
+        {
+            return '"' + text + '"';
         }
     }
 }