[ObjC] Support errors when merging unknown fields to a message.

PiperOrigin-RevId: 658782615

Author: thomasvl

objectivec/GPBMessage.h

@@ -507,13 +507,25 @@ CF_EXTERN_C_END
  * Merges in the data from an `GPBUnknownFields`, meaning the data from the unknown fields gets
  * re-parsed so any known fields will be propertly set.
  *
- * If the intent is to replace the message's unknown fields, call `-clearUnknownFields` first.
+ * If the intent is to *replace* the message's unknown fields, call `-clearUnknownFields` first.
+ *
+ * Since the data from the GPBUnknownFields will always be well formed, this call will almost never
+ * fail. What could cause it to fail is if the GPBUnknownFields contains a field values it is
+ * and error for the message's schema - i.e.: if it contains a length delimited field where the
+ * field number for the message is defined to be a _string_ field, however the length delimited
+ * data provide is not a valid UTF8 string.
  *
  * @param unknownFields     The unknown fields to merge the data from.
  * @param extensionRegistry The extension registry to use to look up extensions, can be `nil`.
+ * @param errorPtr          An optional error pointer to fill in with a failure
+ *                          reason if the data can not be parsed. Will only be
+ *                          filled in if the data failed to be parsed.
+ *
+ * @return Boolean indicating success. errorPtr will only be fill in on failure.
  **/
-- (void)mergeUnknownFields:(GPBUnknownFields *)unknownFields
-         extensionRegistry:(nullable id<GPBExtensionRegistry>)extensionRegistry;
+- (BOOL)mergeUnknownFields:(GPBUnknownFields *)unknownFields
+         extensionRegistry:(nullable id<GPBExtensionRegistry>)extensionRegistry
+                     error:(NSError **)errorPtr;
 
 @end
 

objectivec/GPBMessage.m

@@ -1484,14 +1484,12 @@ - (void)clearUnknownFields {
   self.unknownFields = nil;
 }
 
-- (void)mergeUnknownFields:(GPBUnknownFields *)unknownFields
-         extensionRegistry:(nullable id<GPBExtensionRegistry>)extensionRegistry {
-  NSData *data = [unknownFields serializeAsData];
-  if (![self mergeFromData:data extensionRegistry:extensionRegistry error:NULL]) {
-#if defined(DEBUG) && DEBUG
-    NSAssert(0, @"Internal error within the library, failed to parse data from unknown fields.");
-#endif
-  };
+- (BOOL)mergeUnknownFields:(GPBUnknownFields *)unknownFields
+         extensionRegistry:(nullable id<GPBExtensionRegistry>)extensionRegistry
+                     error:(NSError **)errorPtr {
+  return [self mergeFromData:[unknownFields serializeAsData]
+           extensionRegistry:extensionRegistry
+                       error:errorPtr];
 }
 
 - (BOOL)isInitialized {

objectivec/GPBUnknownFields.h

@@ -33,7 +33,7 @@ __attribute__((objc_subclassing_restricted))
  *
  * Note: The instance is not linked to the message, any change will not be
  * reflected on the message the changes have to be pushed back to the message
- * with `-[GPBMessage mergeUnknownFields:error:]`.
+ * with `-[GPBMessage mergeUnknownFields:extensionRegistry:error:]`.
  **/
 - (instancetype)initFromMessage:(nonnull GPBMessage *)message;
 

objectivec/Tests/GPBMessageTests.m

@@ -506,7 +506,7 @@ - (void)testDescription {
   GPBUnknownFields *ufs = [[[GPBUnknownFields alloc] init] autorelease];
   [ufs addFieldNumber:1234 fixed32:1234];
   [ufs addFieldNumber:2345 varint:54321];
-  [message mergeUnknownFields:ufs extensionRegistry:nil];
+  XCTAssertTrue([message mergeUnknownFields:ufs extensionRegistry:nil error:NULL]);
 
   NSString *description = [message description];
   XCTAssertGreaterThan([description length], 0U);
@@ -997,13 +997,17 @@ - (void)testAutocreatedUnknownFields {
   XCTAssertFalse([message hasOptionalNestedMessage]);
   GPBUnknownFields *ufs = [[[GPBUnknownFields alloc] init] autorelease];
   [ufs addFieldNumber:1 varint:1];
-  [message.optionalNestedMessage mergeUnknownFields:ufs extensionRegistry:nil];
+  XCTAssertTrue([message.optionalNestedMessage mergeUnknownFields:ufs
+                                                extensionRegistry:nil
+                                                            error:NULL]);
   XCTAssertTrue([message hasOptionalNestedMessage]);
 
   message.optionalNestedMessage = nil;
   XCTAssertFalse([message hasOptionalNestedMessage]);
   [ufs clear];  // Also make sure merging zero length forces it to become visible.
-  [message.optionalNestedMessage mergeUnknownFields:ufs extensionRegistry:nil];
+  XCTAssertTrue([message.optionalNestedMessage mergeUnknownFields:ufs
+                                                extensionRegistry:nil
+                                                            error:NULL]);
   XCTAssertTrue([message hasOptionalNestedMessage]);
 }
 
@@ -1887,7 +1891,7 @@ - (void)testGenerateAndParseUnknownMessage {
   [message setUnknownFields:unknowns];
   GPBUnknownFields *ufs = [[[GPBUnknownFields alloc] init] autorelease];
   [ufs addFieldNumber:1234 varint:5678];
-  [message mergeUnknownFields:ufs extensionRegistry:nil];
+  XCTAssertTrue([message mergeUnknownFields:ufs extensionRegistry:nil error:NULL]);
   NSData *data = [message data];
   GPBMessage *message2 = [GPBMessage parseFromData:data extensionRegistry:nil error:NULL];
   XCTAssertEqualObjects(message, message2);
@@ -1900,7 +1904,7 @@ - (void)testDelimitedWriteAndParseMultipleMessages {
   [message1 setUnknownFields:unknowns1];
   GPBUnknownFields *ufs1 = [[[GPBUnknownFields alloc] init] autorelease];
   [ufs1 addFieldNumber:1234 varint:5678];
-  [message1 mergeUnknownFields:ufs1 extensionRegistry:nil];
+  XCTAssertTrue([message1 mergeUnknownFields:ufs1 extensionRegistry:nil error:NULL]);
 
   GPBUnknownFieldSet *unknowns2 = [[[GPBUnknownFieldSet alloc] init] autorelease];
   [unknowns2 mergeVarintField:789 value:987];
@@ -1910,7 +1914,7 @@ - (void)testDelimitedWriteAndParseMultipleMessages {
   GPBUnknownFields *ufs2 = [[[GPBUnknownFields alloc] init] autorelease];
   [ufs2 addFieldNumber:2345 fixed32:6789];
   [ufs2 addFieldNumber:3456 fixed32:7890];
-  [message2 mergeUnknownFields:ufs2 extensionRegistry:nil];
+  XCTAssertTrue([message2 mergeUnknownFields:ufs2 extensionRegistry:nil error:NULL]);
 
   NSMutableData *delimitedData = [NSMutableData data];
   [delimitedData appendData:[message1 delimitedData]];

objectivec/Tests/GPBUnknownFieldsTest.m

@@ -811,7 +811,7 @@ - (void)testMessageMergeUnknowns {
   [group addFieldNumber:123456 varint:5432];
 
   TestAllTypes* msg = [TestAllTypes message];
-  [msg mergeUnknownFields:ufs extensionRegistry:nil];
+  XCTAssertTrue([msg mergeUnknownFields:ufs extensionRegistry:nil error:NULL]);
   XCTAssertEqual(msg.optionalInt64, 100);
   XCTAssertEqual(msg.optionalFixed32, 200);
   XCTAssertEqual(msg.optionalFixed64, 300);
@@ -829,7 +829,7 @@ - (void)testMessageMergeUnknowns {
   XCTAssertEqual(varint, 5432);
 
   TestEmptyMessage* emptyMessage = [TestEmptyMessage message];
-  [emptyMessage mergeUnknownFields:ufs extensionRegistry:nil];
+  XCTAssertTrue([emptyMessage mergeUnknownFields:ufs extensionRegistry:nil error:NULL]);
   GPBUnknownFields* ufs3 = [[[GPBUnknownFields alloc] initFromMessage:emptyMessage] autorelease];
   XCTAssertEqualObjects(ufs3, ufs);  // Round trip through an empty message got us same fields back.
   XCTAssertTrue(ufs3 != ufs);        // But they are different objects.
@@ -843,7 +843,7 @@ - (void)testRoundTripLotsOfFields {
   TestEmptyMessage* emptyMessage = [TestEmptyMessage parseFromData:allFieldsData error:NULL];
   GPBUnknownFields* ufs = [[[GPBUnknownFields alloc] initFromMessage:emptyMessage] autorelease];
   TestAllTypes* allFields2 = [TestAllTypes message];
-  [allFields2 mergeUnknownFields:ufs extensionRegistry:nil];
+  XCTAssertTrue([allFields2 mergeUnknownFields:ufs extensionRegistry:nil error:NULL]);
   XCTAssertEqualObjects(allFields2, allFields);
 
   // Confirm that the they still all end up in unknowns when parsed into a message with extensions
@@ -891,7 +891,7 @@ - (void)testMismatchedFieldTypes {
   // unknown fields again.
   {
     TestAllTypes* msg = [TestAllTypes message];
-    [msg mergeUnknownFields:ufsWrongTypes extensionRegistry:nil];
+    XCTAssertTrue([msg mergeUnknownFields:ufsWrongTypes extensionRegistry:nil error:NULL]);
     GPBUnknownFields* ufs2 = [[[GPBUnknownFields alloc] initFromMessage:msg] autorelease];
     XCTAssertFalse(ufs2.empty);
     XCTAssertEqualObjects(ufs2, ufsWrongTypes);  // All back as unknown fields.
@@ -901,19 +901,46 @@ - (void)testMismatchedFieldTypes {
   // into unknown fields.
   {
     TestAllExtensions* msg = [TestAllExtensions message];
-    [msg mergeUnknownFields:ufsWrongTypes extensionRegistry:[UnittestRoot extensionRegistry]];
+    XCTAssertTrue([msg mergeUnknownFields:ufsWrongTypes
+                        extensionRegistry:[UnittestRoot extensionRegistry]
+                                    error:NULL]);
     GPBUnknownFields* ufs2 = [[[GPBUnknownFields alloc] initFromMessage:msg] autorelease];
     XCTAssertFalse(ufs2.empty);
     XCTAssertEqualObjects(ufs2, ufsWrongTypes);  // All back as unknown fields.
   }
 }
 
+- (void)testMergeFailures {
+  // Valid data, pushes to the string just fine.
+  {
+    GPBUnknownFields* ufs = [[[GPBUnknownFields alloc] init] autorelease];
+    [ufs addFieldNumber:TestAllTypes_FieldNumber_OptionalString
+        lengthDelimited:DataFromCStr("abc")];
+    TestAllTypes* msg = [TestAllTypes message];
+    NSError* error = nil;
+    XCTAssertTrue([msg mergeUnknownFields:ufs extensionRegistry:nil error:&error]);
+    XCTAssertNil(error);
+    XCTAssertEqualObjects(msg.optionalString, @"abc");
+  }
+
+  // Invalid UTF-8 causes a failure when pushed to the message.
+  {
+    GPBUnknownFields* ufs = [[[GPBUnknownFields alloc] init] autorelease];
+    [ufs addFieldNumber:TestAllTypes_FieldNumber_OptionalString
+        lengthDelimited:DataFromBytes(0xC2, 0xF2, 0x0, 0x0, 0x0)];
+    TestAllTypes* msg = [TestAllTypes message];
+    NSError* error = nil;
+    XCTAssertFalse([msg mergeUnknownFields:ufs extensionRegistry:nil error:&error]);
+    XCTAssertNotNil(error);
+  }
+}
+
 - (void)testLargeVarint {
   GPBUnknownFields* ufs = [[[GPBUnknownFields alloc] init] autorelease];
   [ufs addFieldNumber:1 varint:0x7FFFFFFFFFFFFFFFL];
 
   TestEmptyMessage* emptyMessage = [TestEmptyMessage message];
-  [emptyMessage mergeUnknownFields:ufs extensionRegistry:nil];
+  XCTAssertTrue([emptyMessage mergeUnknownFields:ufs extensionRegistry:nil error:NULL]);
 
   GPBUnknownFields* ufsParsed =
       [[[GPBUnknownFields alloc] initFromMessage:emptyMessage] autorelease];

objectivec/Tests/GPBUtilitiesTests.m

@@ -188,7 +188,7 @@ - (void)testTextFormatUnknownFields {
   [group addFieldNumber:3 fixed32:0x3];
   [group addFieldNumber:2 fixed64:0x2];
   TestEmptyMessage *message = [TestEmptyMessage message];
-  [message mergeUnknownFields:ufs extensionRegistry:nil];
+  XCTAssertTrue([message mergeUnknownFields:ufs extensionRegistry:nil error:NULL]);
 
   NSString *expected = @"# --- Unknown fields ---\n"
                        @"10: 1\n"
@@ -255,7 +255,8 @@ - (void)testSetRepeatedFields {
 static void AddUnknownFields(GPBMessage *message, int num) {
   GPBUnknownFields *ufs = [[GPBUnknownFields alloc] init];
   [ufs addFieldNumber:num varint:num];
-  [message mergeUnknownFields:ufs extensionRegistry:nil];
+  // Can't fail since it is a varint.
+  [message mergeUnknownFields:ufs extensionRegistry:nil error:NULL];
   [ufs release];
 }
 

objectivec/Tests/GPBWireFormatTests.m

@@ -131,7 +131,9 @@ - (void)testSerializeMessageSet {
   GPBUnknownFields* group = [ufs addGroupWithFieldNumber:GPBWireFormatMessageSetItem];
   [group addFieldNumber:GPBWireFormatMessageSetTypeId varint:kUnknownTypeId2];
   [group addFieldNumber:GPBWireFormatMessageSetMessage lengthDelimited:DataFromCStr("baz")];
-  [message_set mergeUnknownFields:ufs extensionRegistry:[MSetUnittestMsetRoot extensionRegistry]];
+  XCTAssertTrue([message_set mergeUnknownFields:ufs
+                              extensionRegistry:[MSetUnittestMsetRoot extensionRegistry]
+                                          error:NULL]);
 
   NSData* data = [message_set data];