Merge pull request #17702 from thomasvl/patch_objc_to_28

CherryPick the lastest work for Objective-C over the the 28 branch

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 @@ static void MergeUnknownFieldDataIntoFieldSet(GPBMessage *self, NSData *data,
   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;
 
@@ -109,6 +109,26 @@ __attribute__((objc_subclassing_restricted))
  **/
 - (nonnull GPBUnknownFields *)addGroupWithFieldNumber:(int32_t)fieldNumber;
 
+/**
+ * Removes the given field from the set.
+ *
+ * It is a programming error to attempt to remove a field that is not in this collection.
+ *
+ * Reminder: it is not save to mutate the collection while also using fast enumeration on it.
+ *
+ * @param field The field to remove.
+ **/
+- (void)removeField:(nonnull GPBUnknownField *)field;
+
+/**
+ * Removes all of the fields from the collection that have the given field number.
+ *
+ * If there are no fields with the given field number, this is a no-op.
+ *
+ * @param fieldNumber The field number to remove.
+ **/
+- (void)clearFieldNumber:(int32_t)fieldNumber;
+
 @end
 
 @interface GPBUnknownFields (AccessHelpers)

objectivec/GPBUnknownFields.m

@@ -318,6 +318,34 @@ static BOOL MergeFromInputStream(GPBUnknownFields *self, GPBCodedInputStream *in
   return [group autorelease];
 }
 
+- (void)removeField:(nonnull GPBUnknownField *)field {
+  NSUInteger count = fields_.count;
+  [fields_ removeObjectIdenticalTo:field];
+  if (count == fields_.count) {
+    [NSException raise:NSInvalidArgumentException format:@"The field was not present."];
+  }
+}
+
+- (void)clearFieldNumber:(int32_t)fieldNumber {
+  CHECK_FIELD_NUMBER(fieldNumber);
+  NSMutableIndexSet *toRemove = nil;
+  NSUInteger idx = 0;
+  for (GPBUnknownField *field in fields_) {
+    if (field->number_ == fieldNumber) {
+      if (toRemove == nil) {
+        toRemove = [[NSMutableIndexSet alloc] initWithIndex:idx];
+      } else {
+        [toRemove addIndex:idx];
+      }
+    }
+    ++idx;
+  }
+  if (toRemove) {
+    [fields_ removeObjectsAtIndexes:toRemove];
+    [toRemove release];
+  }
+}
+
 #pragma mark - NSFastEnumeration protocol
 
 - (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state

objectivec/Tests/GPBMessageTests.m

@@ -506,7 +506,7 @@
   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 @@
   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 @@
   [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 @@
   [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 @@
   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

@@ -510,6 +510,82 @@
   XCTAssertNil([ufs fields:99]);  // Not present
 }
 
+- (void)testRemoveField {
+  GPBUnknownFields* ufs = [[[GPBUnknownFields alloc] init] autorelease];
+  [ufs addFieldNumber:1 varint:1];
+  [ufs addFieldNumber:1 fixed32:1];
+  [ufs addFieldNumber:1 fixed64:1];
+  XCTAssertEqual(ufs.count, 3);
+
+  NSArray<GPBUnknownField*>* fields = [ufs fields:1];
+  XCTAssertEqual(fields.count, 3);
+  GPBUnknownField* field = fields[0];
+  XCTAssertEqual(field.number, 1);
+  XCTAssertEqual(field.type, GPBUnknownFieldTypeVarint);
+  XCTAssertEqual(field.varint, 1);
+  [ufs removeField:field];  // Remove first (varint)
+  XCTAssertEqual(ufs.count, 2);
+
+  fields = [ufs fields:1];
+  XCTAssertEqual(fields.count, 2);
+  field = fields[0];
+  XCTAssertEqual(field.number, 1);
+  XCTAssertEqual(field.type, GPBUnknownFieldTypeFixed32);
+  field = fields[1];
+  XCTAssertEqual(field.number, 1);
+  XCTAssertEqual(field.type, GPBUnknownFieldTypeFixed64);
+  [ufs removeField:field];  // Remove the second (fixed64)
+  XCTAssertEqual(ufs.count, 1);
+
+  fields = [ufs fields:1];
+  XCTAssertEqual(fields.count, 1);
+  field = fields[0];
+  XCTAssertEqual(field.number, 1);
+  XCTAssertEqual(field.type, GPBUnknownFieldTypeFixed32);
+
+  field = [[field retain] autorelease];  // Hold on to this last one.
+  [ufs removeField:field];               // Remove the last one (fixed32)
+  XCTAssertEqual(ufs.count, 0);
+
+  // Trying to remove something not in the set should fail.
+  XCTAssertThrowsSpecificNamed([ufs removeField:field], NSException, NSInvalidArgumentException);
+}
+
+- (void)testClearFieldNumber {
+  GPBUnknownFields* ufs = [[[GPBUnknownFields alloc] init] autorelease];
+  [ufs addFieldNumber:1 varint:1];
+  [ufs addFieldNumber:2 fixed32:2];
+  [ufs addFieldNumber:1 fixed64:1];
+  [ufs addFieldNumber:3 varint:3];
+  XCTAssertEqual(ufs.count, 4);
+
+  [ufs clearFieldNumber:999];  // Not present, noop.
+  XCTAssertEqual(ufs.count, 4);
+
+  [ufs clearFieldNumber:1];  // Should remove slot zero and slot two.
+  XCTAssertEqual(ufs.count, 2);
+  NSArray<GPBUnknownField*>* fields = [ufs fields:2];
+  XCTAssertEqual(fields.count, 1);
+  GPBUnknownField* field = fields[0];
+  XCTAssertEqual(field.number, 2);
+  XCTAssertEqual(field.type, GPBUnknownFieldTypeFixed32);
+  XCTAssertEqual(field.fixed32, 2);
+  fields = [ufs fields:3];
+  XCTAssertEqual(fields.count, 1);
+  field = fields[0];
+  XCTAssertEqual(field.number, 3);
+  XCTAssertEqual(field.type, GPBUnknownFieldTypeVarint);
+  XCTAssertEqual(field.varint, 3);
+
+  [ufs clearFieldNumber:2];  // Should remove slot one.
+  fields = [ufs fields:3];
+  XCTAssertEqual(fields.count, 1);
+  field = fields[0];
+  XCTAssertEqual(field.number, 3);
+  XCTAssertEqual(field.type, GPBUnknownFieldTypeVarint);
+  XCTAssertEqual(field.varint, 3);
+}
+
 - (void)testFastEnumeration {
   GPBUnknownFields* ufs = [[[GPBUnknownFields alloc] init] autorelease];
   [ufs addFieldNumber:1 varint:1];
@@ -735,7 +811,7 @@
   [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);
@@ -753,7 +829,7 @@
   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.
@@ -767,7 +843,7 @@
   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
@@ -815,7 +891,7 @@
   // 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.
@@ -825,19 +901,46 @@
   // 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 @@
   [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 @@ static GPBUnknownFieldSet *UnknownFieldsSetHelper(int num) {
 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 @@ const int kUnknownTypeId2 = 1550056;
   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];