[FIX] changed "copy" to "mutableCopy" for NSMutableArray types

Albite PBAppendableArray uses the same backing as PBArray it is okay to just call "copy" for those. For NSMutableArray a call to "copy" creates an immutable NSArray which is assigned to the mutable typed variable. This will create unrecognised selector crashes (see added test).
alexeyxo · Sep 9, 2015 · c293c4f · c293c4f
1 parent 71fa48d
commit c293c4f
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 2 deletions.
diff --git a/src/runtime/Classes/MutableField.m b/src/runtime/Classes/MutableField.m
@@ -65,15 +65,15 @@ - (PBMutableField *)mergeFromField:(PBField *)other {
 
  if (other.lengthDelimitedArray.count > 0) {
  if (_lengthDelimitedArray == nil) {
- _lengthDelimitedArray = [other.lengthDelimitedArray copy];
+ _lengthDelimitedArray = [other.lengthDelimitedArray mutableCopy];
  } else {
  [_lengthDelimitedArray addObjectsFromArray:other.lengthDelimitedArray];
  }
  }
 
  if (other.groupArray.count > 0) {
  if (_groupArray == nil) {
- _groupArray = [other.groupArray copy];
+ _groupArray = [other.groupArray mutableCopy];
  } else {
  [_groupArray addObjectsFromArray:other.groupArray];
  }

diff --git a/src/runtime/Tests/UnknownFieldSetTest.m b/src/runtime/Tests/UnknownFieldSetTest.m
@@ -121,6 +121,38 @@ - (void) testMergeFrom {
  XCTAssertEqualObjects(destination1.data, destination2.data, @"");
 }
 
+- (void) testMergeFrom_LengthDelimited {
+ PBUnknownFieldSet* set1 =
+ [[[[PBUnknownFieldSet builder]
+ addField:[[PBMutableField field] addLengthDelimited:[@"2" dataUsingEncoding:NSUTF8StringEncoding]] forNumber:2]
+ addField:[[PBMutableField field] addLengthDelimited:[@"4" dataUsingEncoding:NSUTF8StringEncoding]] forNumber:3] build];
+
+ PBUnknownFieldSet* set2 =
+ [[[[PBUnknownFieldSet builder]
+ addField:[[PBMutableField field] addLengthDelimited:[@"1" dataUsingEncoding:NSUTF8StringEncoding]] forNumber:1]
+ addField:[[PBMutableField field] addLengthDelimited:[@"3" dataUsingEncoding:NSUTF8StringEncoding]] forNumber:3] build];
+
+ PBUnknownFieldSet* set3 =
+ [[[[PBUnknownFieldSet builder]
+ addField:[[PBMutableField field] addLengthDelimited:[@"1" dataUsingEncoding:NSUTF8StringEncoding]] forNumber:1]
+ addField:[[PBMutableField field] addLengthDelimited:[@"4" dataUsingEncoding:NSUTF8StringEncoding]] forNumber:3] build];
+
+ PBUnknownFieldSet* set4 =
+ [[[[PBUnknownFieldSet builder]
+ addField:[[PBMutableField field] addLengthDelimited:[@"2" dataUsingEncoding:NSUTF8StringEncoding]] forNumber:2]
+ addField:[[PBMutableField field] addLengthDelimited:[@"3" dataUsingEncoding:NSUTF8StringEncoding]] forNumber:3] build];
+
+ TestEmptyMessage* source1 = (id)[[[TestEmptyMessage builder] setUnknownFields:set1] build];
+ TestEmptyMessage* source2 = (id)[[[TestEmptyMessage builder] setUnknownFields:set2] build];
+ TestEmptyMessage* source3 = (id)[[[TestEmptyMessage builder] setUnknownFields:set3] build];
+ TestEmptyMessage* source4 = (id)[[[TestEmptyMessage builder] setUnknownFields:set4] build];
+
+ TestEmptyMessage* destination1 = (id)[[[[TestEmptyMessage builder] mergeFrom:source1] mergeFrom:source2] build];
+ TestEmptyMessage* destination2 = (id)[[[[TestEmptyMessage builder] mergeFrom:source3] mergeFrom:source4] build];
+
+ XCTAssertEqualObjects(destination1.data, destination2.data, @"");
+}
+
 
 - (void) testClear {
  PBUnknownFieldSet* fields =