From 32aaf9eeb53f6afb146da2acac360fb5eb3d6db3 Mon Sep 17 00:00:00 2001
From: Protobuf Team Bot <protobuf-github-bot@google.com>
Date: Fri, 17 Feb 2023 06:35:02 -0800
Subject: [PATCH] [ObjC] Ensure objects aren't leaked on parsing failures.

Add the objects to the object graph and balance the retain count before
doing the parsing. This ensure if an error is hit (and a throw happens),
the object(s) won't be leaked.

Parsing will always mutate the graph, so yes this includes more mutations
in failure cases, but other fields could always be modified before the
bad data is encountered. But even then, that edge case *only* apples to api
users that are explicitly *merge* (-mergeFrom...), the majority of the calls
are to +parseFromData:error:, so the entire graph is released on failure.

PiperOrigin-RevId: 510417377
---
 objectivec/GPBMessage.m         | 21 ++++++++++++++-------
 objectivec/GPBUnknownFieldSet.m |  4 +++-
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/objectivec/GPBMessage.m b/objectivec/GPBMessage.m
index b29540a30f..0bdf4bb8a7 100644
--- a/objectivec/GPBMessage.m
+++ b/objectivec/GPBMessage.m
@@ -1996,9 +1996,12 @@ static GPBUnknownFieldSet *GetOrMakeUnknownFields(GPBMessage *self) {
 
 - (void)mergeFromData:(NSData *)data extensionRegistry:(id<GPBExtensionRegistry>)extensionRegistry {
   GPBCodedInputStream *input = [[GPBCodedInputStream alloc] initWithData:data];
-  [self mergeFromCodedInputStream:input extensionRegistry:extensionRegistry];
-  [input checkLastTagWas:0];
-  [input release];
+  @try {
+    [self mergeFromCodedInputStream:input extensionRegistry:extensionRegistry];
+    [input checkLastTagWas:0];
+  } @finally {
+    [input release];
+  }
 }
 
 #pragma mark - Parse From Data Support
@@ -2204,8 +2207,8 @@ static void MergeSingleFieldFromCodedInputStream(GPBMessage *self, GPBFieldDescr
         [input readMessage:message extensionRegistry:extensionRegistry];
       } else {
         GPBMessage *message = [[field.msgClass alloc] init];
-        [input readMessage:message extensionRegistry:extensionRegistry];
         GPBSetRetainedObjectIvarWithFieldPrivate(self, field, message);
+        [input readMessage:message extensionRegistry:extensionRegistry];
       }
       break;
     }
@@ -2218,8 +2221,8 @@ static void MergeSingleFieldFromCodedInputStream(GPBMessage *self, GPBFieldDescr
         [input readGroup:GPBFieldNumber(field) message:message extensionRegistry:extensionRegistry];
       } else {
         GPBMessage *message = [[field.msgClass alloc] init];
-        [input readGroup:GPBFieldNumber(field) message:message extensionRegistry:extensionRegistry];
         GPBSetRetainedObjectIvarWithFieldPrivate(self, field, message);
+        [input readGroup:GPBFieldNumber(field) message:message extensionRegistry:extensionRegistry];
       }
       break;
     }
@@ -2327,16 +2330,20 @@ static void MergeRepeatedNotPackedFieldFromCodedInputStream(
 #undef CASE_NOT_PACKED_OBJECT
     case GPBDataTypeMessage: {
       GPBMessage *message = [[field.msgClass alloc] init];
-      [input readMessage:message extensionRegistry:extensionRegistry];
       [(NSMutableArray *)genericArray addObject:message];
+      // The array will now retain message, so go ahead and release it in case
+      // -readMessage:extensionRegistry: throws so it won't be leaked.
       [message release];
+      [input readMessage:message extensionRegistry:extensionRegistry];
       break;
     }
     case GPBDataTypeGroup: {
       GPBMessage *message = [[field.msgClass alloc] init];
-      [input readGroup:GPBFieldNumber(field) message:message extensionRegistry:extensionRegistry];
       [(NSMutableArray *)genericArray addObject:message];
+      // The array will now retain message, so go ahead and release it in case
+      // -readGroup:extensionRegistry: throws so it won't be leaked.
       [message release];
+      [input readGroup:GPBFieldNumber(field) message:message extensionRegistry:extensionRegistry];
       break;
     }
     case GPBDataTypeEnum: {
diff --git a/objectivec/GPBUnknownFieldSet.m b/objectivec/GPBUnknownFieldSet.m
index 1dce738934..fd36935480 100644
--- a/objectivec/GPBUnknownFieldSet.m
+++ b/objectivec/GPBUnknownFieldSet.m
@@ -318,10 +318,12 @@ static void GPBUnknownFieldSetMergeUnknownFields(__unused const void *key, const
     }
     case GPBWireFormatStartGroup: {
       GPBUnknownFieldSet *unknownFieldSet = [[GPBUnknownFieldSet alloc] init];
-      [input readUnknownGroup:number message:unknownFieldSet];
       GPBUnknownField *field = [self mutableFieldForNumber:number create:YES];
       [field addGroup:unknownFieldSet];
+      // The field will now retain unknownFieldSet, so go ahead and release it in case
+      // -readUnknownGroup:message: throws so it won't be leaked.
       [unknownFieldSet release];
+      [input readUnknownGroup:number message:unknownFieldSet];
       return YES;
     }
     case GPBWireFormatEndGroup: