Browse Source

Extra strictness for FieldMask conversion

Jon Skeet 9 years ago
parent
commit
f437b67f60

+ 10 - 0
csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs

@@ -417,6 +417,16 @@ namespace Google.Protobuf
             AssertJson("{ 'a': null, 'b': false, 'c': 10.5, 'd': 'text', 'e': [ 't1', 5 ], 'f': { 'nested': 'value' } }", message.ToString());
             AssertJson("{ 'a': null, 'b': false, 'c': 10.5, 'd': 'text', 'e': [ 't1', 5 ], 'f': { 'nested': 'value' } }", message.ToString());
         }
         }
 
 
+        [Test]
+        [TestCase("foo__bar")]
+        [TestCase("foo_3_ar")]
+        [TestCase("fooBar")]
+        public void FieldMaskInvalid(string input)
+        {
+            var mask = new FieldMask { Paths = { input } };
+            Assert.Throws<InvalidOperationException>(() => JsonFormatter.Default.Format(mask));
+        }
+
         [Test]
         [Test]
         public void FieldMaskStandalone()
         public void FieldMaskStandalone()
         {
         {

+ 8 - 0
csharp/src/Google.Protobuf.Test/JsonParserTest.cs

@@ -778,6 +778,14 @@ namespace Google.Protobuf
             CollectionAssert.AreEqual(expectedPaths, parsed.Paths);
             CollectionAssert.AreEqual(expectedPaths, parsed.Paths);
         }
         }
 
 
+        [Test]
+        [TestCase("foo_bar")]
+        public void FieldMask_Invalid(string jsonValue)
+        {
+            string json = WrapInQuotes(jsonValue);
+            Assert.Throws<InvalidProtocolBufferException>(() => FieldMask.Parser.ParseJson(json));
+        }
+
         [Test]
         [Test]
         public void Any_RegularMessage()
         public void Any_RegularMessage()
         {
         {

+ 26 - 1
csharp/src/Google.Protobuf/JsonFormatter.cs

@@ -224,6 +224,31 @@ namespace Google.Protobuf
             return !first;
             return !first;
         }
         }
 
 
+        /// <summary>
+        /// Camel-case converter with added strictness for field mask formatting.
+        /// </summary>
+        /// <exception cref="InvalidOperationException">The field mask is invalid for JSON representation</exception>
+        private static string ToCamelCaseForFieldMask(string input)
+        {
+            for (int i = 0; i < input.Length; i++)
+            {
+                char c = input[i];
+                if (c >= 'A' && c <= 'Z')
+                {
+                    throw new InvalidOperationException($"Invalid field mask to be converted to JSON: {input}");
+                }
+                if (c == '_' && i < input.Length - 1)
+                {
+                    char next = input[i + 1];
+                    if (next < 'a' || next > 'z')
+                    {
+                        throw new InvalidOperationException($"Invalid field mask to be converted to JSON: {input}");
+                    }
+                }
+            }
+            return ToCamelCase(input);
+        }
+
         // Converted from src/google/protobuf/util/internal/utility.cc ToCamelCase
         // Converted from src/google/protobuf/util/internal/utility.cc ToCamelCase
         // TODO: Use the new field in FieldDescriptor.
         // TODO: Use the new field in FieldDescriptor.
         internal static string ToCamelCase(string input)
         internal static string ToCamelCase(string input)
@@ -525,7 +550,7 @@ namespace Google.Protobuf
         private void WriteFieldMask(StringBuilder builder, IMessage value)
         private void WriteFieldMask(StringBuilder builder, IMessage value)
         {
         {
             IList paths = (IList) value.Descriptor.Fields[FieldMask.PathsFieldNumber].Accessor.GetValue(value);
             IList paths = (IList) value.Descriptor.Fields[FieldMask.PathsFieldNumber].Accessor.GetValue(value);
-            WriteString(builder, string.Join(",", paths.Cast<string>().Select(ToCamelCase)));
+            WriteString(builder, string.Join(",", paths.Cast<string>().Select(ToCamelCaseForFieldMask)));
         }
         }
 
 
         private void WriteAny(StringBuilder builder, IMessage value)
         private void WriteAny(StringBuilder builder, IMessage value)

+ 7 - 1
csharp/src/Google.Protobuf/JsonParser.cs

@@ -894,6 +894,8 @@ namespace Google.Protobuf
         private static string ToSnakeCase(string text)
         private static string ToSnakeCase(string text)
         {
         {
             var builder = new StringBuilder(text.Length * 2);
             var builder = new StringBuilder(text.Length * 2);
+            // Note: this is probably unnecessary now, but currently retained to be as close as possible to the
+            // C++, whilst still throwing an exception on underscores.
             bool wasNotUnderscore = false;  // Initialize to false for case 1 (below)
             bool wasNotUnderscore = false;  // Initialize to false for case 1 (below)
             bool wasNotCap = false;
             bool wasNotCap = false;
 
 
@@ -927,7 +929,11 @@ namespace Google.Protobuf
                 else
                 else
                 {
                 {
                     builder.Append(c);
                     builder.Append(c);
-                    wasNotUnderscore = c != '_';
+                    if (c == '_')
+                    {
+                        throw new InvalidProtocolBufferException($"Invalid field mask: {text}");
+                    }
+                    wasNotUnderscore = true;
                     wasNotCap = true;
                     wasNotCap = true;
                 }
                 }
             }
             }