Skip to content

Commit

Permalink
[ObjC] Put out of range closed enum extension values in unknown fields.
Browse files Browse the repository at this point in the history 
For normal fields, closed enums get their out of range values put into unknown
fields, but that wasn't happening for extension fields, this fixes that by
adding the validation during parsing.

Also document on the getExtension API what happens with enums.

Add tests to confirm expected behaviors.

PiperOrigin-RevId: 491356730
  • Loading branch information
@protobuf-github-bot @copybara-github
protobuf-github-bot authored and copybara-github committed Nov 28, 2022
1 parent 7a7bdde commit 903639c
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 11 deletions.
6 changes: 6 additions & 0 deletions objectivec/GPBMessage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,12 @@ CF_EXTERN_C_END
* repeated fields. If the extension is a Message one will be auto created for
* you and returned similar to fields.
*
* NOTE: For Enum extensions, if the enum was _closed_, then unknown values
* were parsed into the message's unknown fields instead of ending up in the
* extensions, just like what happens with singular/repeated fields. For open
* enums, the _raw_ value will be in the NSNumber, meaning if one does a
* `switch` on the values, a `default` case should also be included.
*
* @param extension The extension descriptor of the extension to fetch.
*
* @return The extension matching the given descriptor, or nil if none found.
Expand Down
42 changes: 31 additions & 11 deletions objectivec/GPBMessage.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ static id CreateMapForField(GPBFieldDescriptor *field, GPBMessage *autocreator)
static id GetMapIvarWithField(GPBMessage *self, GPBFieldDescriptor *field);
static NSMutableDictionary *CloneExtensionMap(NSDictionary *extensionMap, NSZone *zone)
__attribute__((ns_returns_retained));
static GPBUnknownFieldSet *GetOrMakeUnknownFields(GPBMessage *self);

#ifdef DEBUG
static NSError *MessageError(NSInteger code, NSDictionary *userInfo) {
Expand Down Expand Up @@ -638,13 +639,15 @@ static id GetMapIvarWithField(GPBMessage *self, GPBFieldDescriptor *field) {
#endif // !defined(__clang_analyzer__)

static id NewSingleValueFromInputStream(GPBExtensionDescriptor *extension,
GPBMessage *messageToGetExtension,
GPBCodedInputStream *input,
id<GPBExtensionRegistry> extensionRegistry,
GPBMessage *existingValue)
__attribute__((ns_returns_retained));

// Note that this returns a retained value intentionally.
static id NewSingleValueFromInputStream(GPBExtensionDescriptor *extension,
GPBMessage *messageToGetExtension,
GPBCodedInputStream *input,
id<GPBExtensionRegistry> extensionRegistry,
GPBMessage *existingValue) {
Expand Down Expand Up @@ -681,8 +684,20 @@ static id NewSingleValueFromInputStream(GPBExtensionDescriptor *extension,
return GPBCodedInputStreamReadRetainedBytes(state);
case GPBDataTypeString:
return GPBCodedInputStreamReadRetainedString(state);
case GPBDataTypeEnum:
return [[NSNumber alloc] initWithInt:GPBCodedInputStreamReadEnum(state)];
case GPBDataTypeEnum: {
int32_t val = GPBCodedInputStreamReadEnum(&input->state_);
GPBEnumDescriptor *enumDescriptor = extension.enumDescriptor;
// If run with source generated before the closed enum support, all enums
// will be considers not closed, so casing to the enum type for a switch
// could cause things to fall off the end of a switch.
if (!enumDescriptor.isClosed || enumDescriptor.enumVerifier(val)) {
return [[NSNumber alloc] initWithInt:val];
} else {
GPBUnknownFieldSet *unknownFields = GetOrMakeUnknownFields(messageToGetExtension);
[unknownFields mergeVarintField:extension->description_->fieldNumber value:val];
return nil;
}
}
case GPBDataTypeGroup:
case GPBDataTypeMessage: {
GPBMessage *message;
Expand Down Expand Up @@ -726,9 +741,11 @@ static void ExtensionMergeFromInputStream(GPBExtensionDescriptor *extension, BOO
int32_t length = GPBCodedInputStreamReadInt32(state);
size_t limit = GPBCodedInputStreamPushLimit(state, length);
while (GPBCodedInputStreamBytesUntilLimit(state) > 0) {
id value = NewSingleValueFromInputStream(extension, input, extensionRegistry, nil);
[message addExtension:extension value:value];
[value release];
id value = NewSingleValueFromInputStream(extension, message, input, extensionRegistry, nil);
if (value) {
[message addExtension:extension value:value];
[value release];
}
}
GPBCodedInputStreamPopLimit(state, limit);
} else {
Expand All @@ -737,13 +754,16 @@ static void ExtensionMergeFromInputStream(GPBExtensionDescriptor *extension, BOO
if (!isRepeated && GPBDataTypeIsMessage(description->dataType)) {
existingValue = [message getExistingExtension:extension];
}
id value = NewSingleValueFromInputStream(extension, input, extensionRegistry, existingValue);
if (isRepeated) {
[message addExtension:extension value:value];
} else {
[message setExtension:extension value:value];
id value =
NewSingleValueFromInputStream(extension, message, input, extensionRegistry, existingValue);
if (value) {
if (isRepeated) {
[message addExtension:extension value:value];
} else {
[message setExtension:extension value:value];
}
[value release];
}
[value release];
}
}

Expand Down
76 changes: 76 additions & 0 deletions objectivec/Tests/GPBMessageTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1481,6 +1481,82 @@ - (void)testExtensionsMergeFrom {
XCTAssertEqualObjects(message, message2);
}

- (void)testClosedEnumsInExtensions {
// Only unknown values.

NSData *data =
DataFromCStr("\xA8\x01\x0A" // optional_nested_enum_extension set to 10
"\x98\x03\x0B" // repeated_nested_enum_extension set to 11
"\xA2\x03\x01\x0C" // repeated_foreign_enum_extension set to 12 (packed)
);
NSError *error = nil;

TestAllExtensions *msg = [TestAllExtensions parseFromData:data
extensionRegistry:[self extensionRegistry]
error:&error];
XCTAssertNil(error);

XCTAssertFalse([msg hasExtension:[UnittestRoot optionalNestedEnumExtension]]);
XCTAssertFalse([msg hasExtension:[UnittestRoot repeatedNestedEnumExtension]]);
XCTAssertFalse([msg hasExtension:[UnittestRoot repeatedForeignEnumExtension]]);

GPBUnknownFieldSet *unknownFields = msg.unknownFields;
GPBUnknownField *field =
[unknownFields getField:[UnittestRoot optionalNestedEnumExtension].fieldNumber];
XCTAssertNotNil(field);
XCTAssertEqual(field.varintList.count, 1);
XCTAssertEqual([field.varintList valueAtIndex:0], 10);
field = [unknownFields getField:[UnittestRoot repeatedNestedEnumExtension].fieldNumber];
XCTAssertNotNil(field);
XCTAssertEqual(field.varintList.count, 1);
XCTAssertEqual([field.varintList valueAtIndex:0], 11);
field = [unknownFields getField:[UnittestRoot repeatedForeignEnumExtension].fieldNumber];
XCTAssertNotNil(field);
XCTAssertEqual(field.varintList.count, 1);
XCTAssertEqual([field.varintList valueAtIndex:0], 12);

// Unknown and known, the known come though an unknown go to unknown fields.

data = DataFromCStr(
"\xA8\x01\x01" // optional_nested_enum_extension set to 1
"\xA8\x01\x0A" // optional_nested_enum_extension set to 10
"\xA8\x01\x02" // optional_nested_enum_extension set to 2
"\x98\x03\x02" // repeated_nested_enum_extension set to 2
"\x98\x03\x0B" // repeated_nested_enum_extension set to 11
"\x98\x03\x03" // repeated_nested_enum_extension set to 3
"\xA2\x03\x03\x04\x0C\x06" // repeated_foreign_enum_extension set to 4, 12, 6 (packed)
);
error = nil;

msg = [TestAllExtensions parseFromData:data
extensionRegistry:[self extensionRegistry]
error:&error];
XCTAssertNil(error);

XCTAssertTrue([msg hasExtension:[UnittestRoot optionalNestedEnumExtension]]);
XCTAssertEqualObjects([msg getExtension:[UnittestRoot optionalNestedEnumExtension]], @2);
XCTAssertTrue([msg hasExtension:[UnittestRoot repeatedNestedEnumExtension]]);
id expected = @[ @2, @3 ];
XCTAssertEqualObjects([msg getExtension:[UnittestRoot repeatedNestedEnumExtension]], expected);
XCTAssertTrue([msg hasExtension:[UnittestRoot repeatedForeignEnumExtension]]);
expected = @[ @4, @6 ];
XCTAssertEqualObjects([msg getExtension:[UnittestRoot repeatedForeignEnumExtension]], expected);

unknownFields = msg.unknownFields;
field = [unknownFields getField:[UnittestRoot optionalNestedEnumExtension].fieldNumber];
XCTAssertNotNil(field);
XCTAssertEqual(field.varintList.count, 1);
XCTAssertEqual([field.varintList valueAtIndex:0], 10);
field = [unknownFields getField:[UnittestRoot repeatedNestedEnumExtension].fieldNumber];
XCTAssertNotNil(field);
XCTAssertEqual(field.varintList.count, 1);
XCTAssertEqual([field.varintList valueAtIndex:0], 11);
field = [unknownFields getField:[UnittestRoot repeatedForeignEnumExtension].fieldNumber];
XCTAssertNotNil(field);
XCTAssertEqual(field.varintList.count, 1);
XCTAssertEqual([field.varintList valueAtIndex:0], 12);
}

- (void)testDefaultingExtensionMessages {
TestAllExtensions *message = [TestAllExtensions message];

Expand Down

0 comments on commit 903639c

Please sign in to comment.