Fix unknown enum handling

Fixes google#849.
osa1 · Jun 22, 2023 · bef02ba · bef02ba
1 parent e76bd74
commit bef02ba
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 12 deletions.
diff --git a/protobuf/lib/src/protobuf/proto3_json.dart b/protobuf/lib/src/protobuf/proto3_json.dart
@@ -164,6 +164,8 @@ void _mergeFromProto3Json(
       ignoreUnknownFields, supportNamesWithUnderscores, permissiveEnums);
 
   void recursionHelper(Object? json, _FieldSet fieldSet) {
+    // Convert a JSON object to proto object. Returns `null` on unknown enum
+    // values when [ignoreUnknownFields] is [true].
     Object? convertProto3JsonValue(Object value, FieldInfo fieldInfo) {
       final fieldType = fieldInfo.type;
       switch (PbFieldType._baseType(fieldType)) {
@@ -174,14 +176,12 @@ void _mergeFromProto3Json(
           throw context.parseException('Expected bool value', json);
         case PbFieldType._BYTES_BIT:
           if (value is String) {
-            Uint8List result;
             try {
-              result = base64Decode(value);
+              return base64Decode(value);
             } on FormatException {
               throw context.parseException(
                   'Expected bytes encoded as base64 String', json);
             }
-            return result;
           }
           throw context.parseException(
               'Expected bytes encoded as base64 String', value);
@@ -264,14 +264,12 @@ void _mergeFromProto3Json(
         case PbFieldType._SFIXED64_BIT:
           if (value is int) return Int64(value);
           if (value is String) {
-            Int64 result;
             try {
-              result = Int64.parseInt(value);
+              return Int64.parseInt(value);
             } on FormatException {
               throw context.parseException(
                   'Expected int or stringified int', value);
             }
-            return result;
           }
           throw context.parseException(
               'Expected int or stringified int', value);
@@ -368,9 +366,12 @@ void _mergeFromProto3Json(
                   throw context.parseException('Expected a String key', subKey);
                 }
                 context.addMapIndex(subKey);
-                fieldValues[decodeMapKey(subKey, mapFieldInfo.keyFieldType)] =
-                    convertProto3JsonValue(
-                        subValue, mapFieldInfo.valueFieldInfo);
+                final key = decodeMapKey(subKey, mapFieldInfo.keyFieldType);
+                final value = convertProto3JsonValue(
+                    subValue, mapFieldInfo.valueFieldInfo);
+                if (value != null) {
+                  fieldValues[key] = value;
+                }
                 context.popIndex();
               });
             } else {
@@ -382,7 +383,10 @@ void _mergeFromProto3Json(
               for (var i = 0; i < value.length; i++) {
                 final entry = value[i];
                 context.addListIndex(i);
-                values.add(convertProto3JsonValue(entry, fieldInfo));
+                final parsedValue = convertProto3JsonValue(entry, fieldInfo);
+                if (parsedValue != null) {
+                  values.add(parsedValue);
+                }
                 context.popIndex();
               }
             } else {
@@ -402,8 +406,15 @@ void _mergeFromProto3Json(
               original.mergeFromMessage(parsedSubMessage);
             }
           } else {
-            fieldSet._setFieldUnchecked(
-                meta, fieldInfo, convertProto3JsonValue(value, fieldInfo));
+            final parsedValue = convertProto3JsonValue(value, fieldInfo);
+            if (parsedValue == null) {
+              // Unknown enum
+              if (!ignoreUnknownFields) {
+                throw context.parseException('Unknown enum value', value);
+              }
+            } else {
+              fieldSet._setFieldUnchecked(meta, fieldInfo, parsedValue);
+            }
           }
           context.popIndex();
         });

diff --git a/protoc_plugin/Makefile b/protoc_plugin/Makefile
@@ -31,6 +31,7 @@ TEST_PROTO_LIST = \
 	entity \
 	enum_extension \
 	enum_name \
+	enum_test \
 	extend_unittest \
 	ExtensionEnumNameConflict \
 	ExtensionNameConflict \

diff --git a/protoc_plugin/test/protos/enum_test.proto b/protoc_plugin/test/protos/enum_test.proto
@@ -0,0 +1,16 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+syntax = "proto3";
+
+enum A {
+  X = 0;
+  Y = 1;
+}
+
+message Message {
+  A enum_field = 1;
+  map<int32, A> map_value_field = 2;
+  repeated A repeated_enum_field = 3;
+}
diff --git a/protoc_plugin/test/unknown_enums_test.dart b/protoc_plugin/test/unknown_enums_test.dart
@@ -0,0 +1,40 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:test/test.dart';
+
+import '../out/protos/enum_test.pb.dart';
+
+void main() {
+  group('Enum parsing in maps, lists, messages', () {
+    test('Parse known fields', () {
+      final json = {
+        'enumField': 'Y',
+        'mapValueField': {'1': 'Y'},
+        'repeatedEnumField': ['Y'],
+      };
+
+      final msg = Message();
+      msg.mergeFromProto3Json(json);
+      expect(msg.enumField, A.Y);
+      expect(msg.mapValueField.values.toList(), [A.Y]);
+      expect(msg.repeatedEnumField, [A.Y]);
+    });
+
+    test('Skip unknown fields', () {
+      final json = {
+        'enumField': 'Z',
+        'mapValueField': {'1': 'X', '2': 'Z', '3': 'Y'},
+        'repeatedEnumField': ['X', 'Z', 'Y'],
+      };
+
+      final msg = Message();
+      msg.enumField = A.Y;
+      msg.mergeFromProto3Json(json, ignoreUnknownFields: true);
+      expect(msg.enumField, A.Y);
+      expect(msg.mapValueField.values.toList(), [A.X, A.Y]);
+      expect(msg.repeatedEnumField, [A.X, A.Y]);
+    });
+  });
+}