Ignore non-items in message sets (#857)

This removes the remaining runtime error case in message set parser: when parsing a message set message we expect tag 1 for the items. Ignore other tags. With this message set parser no longer fails in runtime. Anything unexpected is ignored.
google · Jun 29, 2023 · acc0462 · acc0462
1 parent 0eb3796
commit acc0462
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 11 deletions.
diff --git a/protobuf/lib/src/protobuf/message_set.dart b/protobuf/lib/src/protobuf/message_set.dart
@@ -58,16 +58,18 @@ abstract class $_MessageSet extends GeneratedMessage {
     outer:
     while (true) {
       final tag = input.readTag();
-      final wireType = getTagWireType(tag);
       final tagNumber = getTagFieldNumber(tag);
 
       if (tag == 0) {
         break;
       }
 
       if (tagNumber != _messageSetItemsTag) {
-        throw UnsupportedError(
-            'Invalid message set (type = $wireType, tag = $tagNumber)');
+        if (!input.skipField(tag)) {
+          break; // End of group.
+        } else {
+          continue;
+        }
       }
 
       // Parse an item. An item is a message with two fields:

diff --git a/protoc_plugin/test/message_set_test.dart b/protoc_plugin/test/message_set_test.dart
@@ -52,14 +52,10 @@ void main() {
     expect(msg.unknownFields.isEmpty, true);
   });
 
-  test('Ignore extra item tags', () {
+  test('Ignore extra tags in items', () {
     // Generate a message set encoding using unknown fields. Message set item
     // will have extra tags.
 
-    final messageSetUnknownFields = UnknownFieldSet();
-
-    final itemField = UnknownFieldSetField();
-
     final itemFieldGroup = UnknownFieldSet();
 
     // Invalid field with tag 1.
@@ -83,9 +79,9 @@ void main() {
     itemFieldGroup.addField(
         2, UnknownFieldSetField()..addVarint(Int64(1758024)));
 
-    itemField.addGroup(itemFieldGroup);
-
-    messageSetUnknownFields.addField(1, itemField);
+    final messageSetUnknownFields = UnknownFieldSet();
+    messageSetUnknownFields.addField(
+        1, UnknownFieldSetField()..addGroup(itemFieldGroup));
 
     final messageSetEncoded = CodedBufferWriter();
     messageSetUnknownFields.writeToCodedBufferWriter(messageSetEncoded);
@@ -105,4 +101,26 @@ void main() {
     expect(extensionValue.b, 'test');
     expect(msg.unknownFields.isEmpty, true);
   });
+
+  test('Ignore invalid tags in repeated items', () {
+    // Extra fields other than `repeated Item items = 1` in the outer message.
+
+    final messageSetUnknownFields = UnknownFieldSet();
+    messageSetUnknownFields.addField(
+        2, UnknownFieldSetField()..addVarint(Int64(987)));
+
+    final messageSetEncoded = CodedBufferWriter();
+    messageSetUnknownFields.writeToCodedBufferWriter(messageSetEncoded);
+
+    final encoded = (Empty()
+          ..unknownFields.addField(
+              123,
+              UnknownFieldSetField()
+                ..lengthDelimited.add(messageSetEncoded.toBuffer())))
+        .writeToBuffer();
+
+    final registry = ExtensionRegistry()..add(TestMessage.messageSetExtension);
+    final msg = TestMessage.fromBuffer(encoded, registry);
+    expect(msg.info.hasExtension(TestMessage.messageSetExtension), false);
+  });
 }