Improving the granularity parsing errors (#1623)
Add more context to GPBCodedInputStream failures.
Have GPBMessage parsing apis extract out the GPBCodedInputStream information and expose it.
Update HeaderDocs with pointers to all error domains/codes.
Expand the unittests to cover the full set of errors reported.
Fixes https://github.com/google/protobuf/issues/1618
diff --git a/objectivec/GPBCodedInputStream.h b/objectivec/GPBCodedInputStream.h
index 44f6990..df9d97b 100644
--- a/objectivec/GPBCodedInputStream.h
+++ b/objectivec/GPBCodedInputStream.h
@@ -35,6 +35,39 @@
NS_ASSUME_NONNULL_BEGIN
+CF_EXTERN_C_BEGIN
+
+/// GPBCodedInputStream exception name. Exceptions raised from
+/// GPBCodedInputStream contain an underlying error in the userInfo dictionary
+/// under the GPBCodedInputStreamUnderlyingErrorKey key.
+extern NSString *const GPBCodedInputStreamException;
+
+/// The key under which the underlying NSError from the exception is stored.
+extern NSString *const GPBCodedInputStreamUnderlyingErrorKey;
+
+/// NSError domain used for GPBCodedInputStream errors.
+extern NSString *const GPBCodedInputStreamErrorDomain;
+
+/// Error code for NSError with GPBCodedInputStreamErrorDomain.
+typedef NS_ENUM(NSInteger, GPBCodedInputStreamErrorCode) {
+ /// The size does not fit in the remaining bytes to be read.
+ GPBCodedInputStreamErrorInvalidSize = -100,
+ /// Attempted to read beyond the subsection limit.
+ GPBCodedInputStreamErrorSubsectionLimitReached = -101,
+ /// The requested subsection limit is invalid.
+ GPBCodedInputStreamErrorInvalidSubsectionLimit = -102,
+ /// Invalid tag read.
+ GPBCodedInputStreamErrorInvalidTag = -103,
+ /// Invalid UTF-8 character in a string.
+ GPBCodedInputStreamErrorInvalidUTF8 = -104,
+ /// Invalid VarInt read.
+ GPBCodedInputStreamErrorInvalidVarInt = -105,
+ /// The maximum recursion depth of messages was exceeded.
+ GPBCodedInputStreamErrorRecursionDepthExceeded = -106,
+};
+
+CF_EXTERN_C_END
+
/// Reads and decodes protocol message fields.
///
/// The common uses of protocol buffers shouldn't need to use this class.
diff --git a/objectivec/GPBCodedInputStream.m b/objectivec/GPBCodedInputStream.m
index 4022717..acba715 100644
--- a/objectivec/GPBCodedInputStream.m
+++ b/objectivec/GPBCodedInputStream.m
@@ -36,17 +36,42 @@
#import "GPBUtilities_PackagePrivate.h"
#import "GPBWireFormat.h"
+NSString *const GPBCodedInputStreamException =
+ GPBNSStringifySymbol(GPBCodedInputStreamException);
+
+NSString *const GPBCodedInputStreamUnderlyingErrorKey =
+ GPBNSStringifySymbol(GPBCodedInputStreamUnderlyingErrorKey);
+
+NSString *const GPBCodedInputStreamErrorDomain =
+ GPBNSStringifySymbol(GPBCodedInputStreamErrorDomain);
+
static const NSUInteger kDefaultRecursionLimit = 64;
+static void RaiseException(NSInteger code, NSString *reason) {
+ NSDictionary *errorInfo = nil;
+ if ([reason length]) {
+ errorInfo = @{ GPBErrorReasonKey: reason };
+ }
+ NSError *error = [NSError errorWithDomain:GPBCodedInputStreamErrorDomain
+ code:code
+ userInfo:errorInfo];
+
+ NSDictionary *exceptionInfo =
+ @{ GPBCodedInputStreamUnderlyingErrorKey: error };
+ [[[NSException alloc] initWithName:GPBCodedInputStreamException
+ reason:reason
+ userInfo:exceptionInfo] raise];
+}
+
static void CheckSize(GPBCodedInputStreamState *state, size_t size) {
size_t newSize = state->bufferPos + size;
if (newSize > state->bufferSize) {
- [NSException raise:NSParseErrorException format:@""];
+ RaiseException(GPBCodedInputStreamErrorInvalidSize, nil);
}
if (newSize > state->currentLimit) {
// Fast forward to end of currentLimit;
state->bufferPos = state->currentLimit;
- [NSException raise:NSParseErrorException format:@""];
+ RaiseException(GPBCodedInputStreamErrorSubsectionLimitReached, nil);
}
}
@@ -95,8 +120,8 @@
return result;
}
}
- [NSException raise:NSParseErrorException
- format:@"Unable to read varint32"];
+ RaiseException(GPBCodedInputStreamErrorInvalidVarInt,
+ @"Invalid VarInt32");
}
}
}
@@ -115,7 +140,7 @@
}
shift += 7;
}
- [NSException raise:NSParseErrorException format:@"Unable to read varint64"];
+ RaiseException(GPBCodedInputStreamErrorInvalidVarInt, @"Invalid VarInt64");
return 0;
}
@@ -202,8 +227,7 @@
state->lastTag = ReadRawVarint32(state);
if (state->lastTag == 0) {
// If we actually read zero, that's not a valid tag.
- [NSException raise:NSParseErrorException
- format:@"Invalid last tag %d", state->lastTag];
+ RaiseException(GPBCodedInputStreamErrorInvalidTag, @"Last tag can't be 0");
}
return state->lastTag;
}
@@ -226,8 +250,7 @@
NSLog(@"UTF-8 failure, is some field type 'string' when it should be "
@"'bytes'?");
#endif
- [NSException raise:NSParseErrorException
- format:@"Invalid UTF-8 for a 'string'"];
+ RaiseException(GPBCodedInputStreamErrorInvalidUTF8, nil);
}
}
return result;
@@ -262,8 +285,7 @@
byteLimit += state->bufferPos;
size_t oldLimit = state->currentLimit;
if (byteLimit > oldLimit) {
- [NSException raise:NSInvalidArgumentException
- format:@"byteLimit > oldLimit: %tu > %tu", byteLimit, oldLimit];
+ RaiseException(GPBCodedInputStreamErrorInvalidSubsectionLimit, nil);
}
state->currentLimit = byteLimit;
return oldLimit;
@@ -286,8 +308,7 @@
void GPBCodedInputStreamCheckLastTagWas(GPBCodedInputStreamState *state,
int32_t value) {
if (state->lastTag != value) {
- [NSException raise:NSParseErrorException
- format:@"Last tag: %d should be %d", state->lastTag, value];
+ RaiseException(GPBCodedInputStreamErrorInvalidTag, @"Unexpected tag read");
}
}
@@ -353,7 +374,7 @@
SkipRawData(&state_, sizeof(int32_t));
return YES;
}
- [NSException raise:NSParseErrorException format:@"Invalid tag %d", tag];
+ RaiseException(GPBCodedInputStreamErrorInvalidTag, nil);
return NO;
}
@@ -414,9 +435,7 @@
message:(GPBMessage *)message
extensionRegistry:(GPBExtensionRegistry *)extensionRegistry {
if (state_.recursionDepth >= kDefaultRecursionLimit) {
- [NSException raise:NSParseErrorException
- format:@"recursionDepth(%tu) >= %tu", state_.recursionDepth,
- kDefaultRecursionLimit];
+ RaiseException(GPBCodedInputStreamErrorRecursionDepthExceeded, nil);
}
++state_.recursionDepth;
[message mergeFromCodedInputStream:self extensionRegistry:extensionRegistry];
@@ -428,9 +447,7 @@
- (void)readUnknownGroup:(int32_t)fieldNumber
message:(GPBUnknownFieldSet *)message {
if (state_.recursionDepth >= kDefaultRecursionLimit) {
- [NSException raise:NSParseErrorException
- format:@"recursionDepth(%tu) >= %tu", state_.recursionDepth,
- kDefaultRecursionLimit];
+ RaiseException(GPBCodedInputStreamErrorRecursionDepthExceeded, nil);
}
++state_.recursionDepth;
[message mergeFromCodedInputStream:self];
@@ -443,9 +460,7 @@
extensionRegistry:(GPBExtensionRegistry *)extensionRegistry {
int32_t length = ReadRawVarint32(&state_);
if (state_.recursionDepth >= kDefaultRecursionLimit) {
- [NSException raise:NSParseErrorException
- format:@"recursionDepth(%tu) >= %tu", state_.recursionDepth,
- kDefaultRecursionLimit];
+ RaiseException(GPBCodedInputStreamErrorRecursionDepthExceeded, nil);
}
size_t oldLimit = GPBCodedInputStreamPushLimit(&state_, length);
++state_.recursionDepth;
@@ -461,9 +476,7 @@
parentMessage:(GPBMessage *)parentMessage {
int32_t length = ReadRawVarint32(&state_);
if (state_.recursionDepth >= kDefaultRecursionLimit) {
- [NSException raise:NSParseErrorException
- format:@"recursionDepth(%tu) >= %tu", state_.recursionDepth,
- kDefaultRecursionLimit];
+ RaiseException(GPBCodedInputStreamErrorRecursionDepthExceeded, nil);
}
size_t oldLimit = GPBCodedInputStreamPushLimit(&state_, length);
++state_.recursionDepth;
diff --git a/objectivec/GPBMessage.h b/objectivec/GPBMessage.h
index 2249829..b3b0779 100644
--- a/objectivec/GPBMessage.h
+++ b/objectivec/GPBMessage.h
@@ -49,12 +49,15 @@
/// Error code for NSError with GPBMessageErrorDomain.
typedef NS_ENUM(NSInteger, GPBMessageErrorCode) {
- /// The data being parsed is bad and a message can not be created from it.
- GPBMessageErrorCodeMalformedData = -100,
+ /// Uncategorized error.
+ GPBMessageErrorCodeOther = -100,
/// A message can't be serialized because it is missing required fields.
GPBMessageErrorCodeMissingRequiredField = -101,
};
+/// Key under which the error's reason is stored inside the userInfo dictionary.
+extern NSString *const GPBErrorReasonKey;
+
CF_EXTERN_C_END
/// Base class for all of the generated message classes.
@@ -86,6 +89,9 @@
/// @note In DEBUG builds, the parsed message is checked to be sure all required
/// fields were provided, and the parse will fail if some are missing.
///
+/// @note The errors returned are likely coming from the domain and codes listed
+/// at the top of this file and GPBCodedInputStream.h.
+///
/// @param data The data to parse.
/// @param errorPtr An optional error pointer to fill in with a failure reason if
/// the data can not be parsed.
@@ -101,6 +107,9 @@
/// @note In DEBUG builds, the parsed message is checked to be sure all required
/// fields were provided, and the parse will fail if some are missing.
///
+/// @note The errors returned are likely coming from the domain and codes listed
+/// at the top of this file and GPBCodedInputStream.h.
+///
/// @param data The data to parse.
/// @param extensionRegistry The extension registry to use to look up extensions.
/// @param errorPtr An optional error pointer to fill in with a failure
@@ -119,6 +128,9 @@
/// @note In DEBUG builds, the parsed message is checked to be sure all required
/// fields were provided, and the parse will fail if some are missing.
///
+/// @note The errors returned are likely coming from the domain and codes listed
+/// at the top of this file and GPBCodedInputStream.h.
+///
/// @param input The stream to read data from.
/// @param extensionRegistry The extension registry to use to look up extensions.
/// @param errorPtr An optional error pointer to fill in with a failure
@@ -139,6 +151,9 @@
/// the required fields are set. So this method can be used to reload
/// messages that may not be complete.
///
+/// @note The errors returned are likely coming from the domain and codes listed
+/// at the top of this file and GPBCodedInputStream.h.
+///
/// @param input The stream to read data from.
/// @param extensionRegistry The extension registry to use to look up extensions.
/// @param errorPtr An optional error pointer to fill in with a failure
@@ -158,6 +173,9 @@
/// @note In DEBUG builds, the parsed message is checked to be sure all required
/// fields were provided, and the parse will fail if some are missing.
///
+/// @note The errors returned are likely coming from the domain and codes listed
+/// at the top of this file and GPBCodedInputStream.h.
+///
/// @param data The data to parse.
/// @param errorPtr An optional error pointer to fill in with a failure reason if
/// the data can not be parsed.
@@ -171,6 +189,9 @@
/// @note In DEBUG builds, the parsed message is checked to be sure all required
/// fields were provided, and the parse will fail if some are missing.
///
+/// @note The errors returned are likely coming from the domain and codes listed
+/// at the top of this file and GPBCodedInputStream.h.
+///
/// @param data The data to parse.
/// @param extensionRegistry The extension registry to use to look up extensions.
/// @param errorPtr An optional error pointer to fill in with a failure
@@ -188,6 +209,9 @@
/// the required fields are set. So this method can be used to reload
/// messages that may not be complete.
///
+/// @note The errors returned are likely coming from the domain and codes listed
+/// at the top of this file and GPBCodedInputStream.h.
+///
/// @param input The stream to read data from.
/// @param extensionRegistry The extension registry to use to look up extensions.
/// @param errorPtr An optional error pointer to fill in with a failure
diff --git a/objectivec/GPBMessage.m b/objectivec/GPBMessage.m
index f1cb392..919fb93 100644
--- a/objectivec/GPBMessage.m
+++ b/objectivec/GPBMessage.m
@@ -53,6 +53,8 @@
NSString *const GPBMessageErrorDomain =
GPBNSStringifySymbol(GPBMessageErrorDomain);
+NSString *const GPBErrorReasonKey = @"Reason";
+
static NSString *const kGPBDataCoderKey = @"GPBData";
//
@@ -96,20 +98,35 @@
NSZone *zone)
__attribute__((ns_returns_retained));
+#ifdef DEBUG
static NSError *MessageError(NSInteger code, NSDictionary *userInfo) {
return [NSError errorWithDomain:GPBMessageErrorDomain
code:code
userInfo:userInfo];
}
+#endif
-static NSError *MessageErrorWithReason(NSInteger code, NSString *reason) {
- NSDictionary *userInfo = nil;
- if ([reason length]) {
- userInfo = @{ @"Reason" : reason };
+static NSError *ErrorFromException(NSException *exception) {
+ NSError *error = nil;
+
+ if ([exception.name isEqual:GPBCodedInputStreamException]) {
+ NSDictionary *exceptionInfo = exception.userInfo;
+ error = exceptionInfo[GPBCodedInputStreamUnderlyingErrorKey];
}
- return MessageError(code, userInfo);
-}
+ if (!error) {
+ NSString *reason = exception.reason;
+ NSDictionary *userInfo = nil;
+ if ([reason length]) {
+ userInfo = @{ GPBErrorReasonKey : reason };
+ }
+
+ error = [NSError errorWithDomain:GPBMessageErrorDomain
+ code:GPBMessageErrorCodeOther
+ userInfo:userInfo];
+ }
+ return error;
+}
static void CheckExtension(GPBMessage *self,
GPBExtensionDescriptor *extension) {
@@ -817,8 +834,7 @@
[self release];
self = nil;
if (errorPtr) {
- *errorPtr = MessageErrorWithReason(GPBMessageErrorCodeMalformedData,
- exception.reason);
+ *errorPtr = ErrorFromException(exception);
}
}
#ifdef DEBUG
@@ -849,8 +865,7 @@
[self release];
self = nil;
if (errorPtr) {
- *errorPtr = MessageErrorWithReason(GPBMessageErrorCodeMalformedData,
- exception.reason);
+ *errorPtr = ErrorFromException(exception);
}
}
#ifdef DEBUG
@@ -1923,8 +1938,7 @@
@catch (NSException *exception) {
message = nil;
if (errorPtr) {
- *errorPtr = MessageErrorWithReason(GPBMessageErrorCodeMalformedData,
- exception.reason);
+ *errorPtr = ErrorFromException(exception);
}
}
#ifdef DEBUG
diff --git a/objectivec/Tests/GPBMessageTests+Serialization.m b/objectivec/Tests/GPBMessageTests+Serialization.m
index 0d811a9..fad9773 100644
--- a/objectivec/Tests/GPBMessageTests+Serialization.m
+++ b/objectivec/Tests/GPBMessageTests+Serialization.m
@@ -881,6 +881,103 @@
XCTAssertEqualObjects(extsParse, extsOrig);
}
+- (void)testErrorSubsectionInvalidLimit {
+ NSData *data = DataFromCStr(
+ "\x0A\x08\x0A\x07\x12\x04\x72\x02\x4B\x50\x12\x04\x72\x02\x4B\x50");
+ NSError *error = nil;
+ NestedTestAllTypes *msg = [NestedTestAllTypes parseFromData:data
+ error:&error];
+ XCTAssertNil(msg);
+ XCTAssertNotNil(error);
+ XCTAssertEqualObjects(error.domain, GPBCodedInputStreamErrorDomain);
+ XCTAssertEqual(error.code, GPBCodedInputStreamErrorInvalidSubsectionLimit);
+}
+
+- (void)testErrorSubsectionLimitReached {
+ NSData *data = DataFromCStr("\x0A\x06\x12\x03\x72\x02\x4B\x50");
+ NSError *error = nil;
+ NestedTestAllTypes *msg = [NestedTestAllTypes parseFromData:data
+ error:&error];
+ XCTAssertNil(msg);
+ XCTAssertNotNil(error);
+ XCTAssertEqualObjects(error.domain, GPBCodedInputStreamErrorDomain);
+ XCTAssertEqual(error.code, GPBCodedInputStreamErrorSubsectionLimitReached);
+}
+
+- (void)testErrorInvalidVarint {
+ NSData *data = DataFromCStr("\x72\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF");
+ NSError *error = nil;
+ TestAllTypes *msg = [TestAllTypes parseFromData:data error:&error];
+ XCTAssertNil(msg);
+ XCTAssertNotNil(error);
+ XCTAssertEqualObjects(error.domain, GPBCodedInputStreamErrorDomain);
+ XCTAssertEqual(error.code, GPBCodedInputStreamErrorInvalidVarInt);
+}
+
+- (void)testErrorInvalidUTF8 {
+ NSData *data = DataFromCStr("\x72\x04\xF4\xFF\xFF\xFF");
+ NSError *error = nil;
+ TestAllTypes *msg = [TestAllTypes parseFromData:data error:&error];
+ XCTAssertNil(msg);
+ XCTAssertNotNil(error);
+ XCTAssertEqualObjects(error.domain, GPBCodedInputStreamErrorDomain);
+ XCTAssertEqual(error.code, GPBCodedInputStreamErrorInvalidUTF8);
+}
+
+- (void)testErrorInvalidSize {
+ NSData *data = DataFromCStr("\x72\x03\x4B\x50");
+ NSError *error = nil;
+ NestedTestAllTypes *msg = [NestedTestAllTypes parseFromData:data
+ error:&error];
+ XCTAssertNil(msg);
+ XCTAssertNotNil(error);
+ XCTAssertEqualObjects(error.domain, GPBCodedInputStreamErrorDomain);
+ XCTAssertEqual(error.code, GPBCodedInputStreamErrorInvalidSize);
+}
+
+- (void)testErrorInvalidTag {
+ NSData *data = DataFromCStr("\x0F");
+ NSError *error = nil;
+ NestedTestAllTypes *msg = [NestedTestAllTypes parseFromData:data
+ error:&error];
+ XCTAssertNil(msg);
+ XCTAssertNotNil(error);
+ XCTAssertEqualObjects(error.domain, GPBCodedInputStreamErrorDomain);
+ XCTAssertEqual(error.code, GPBCodedInputStreamErrorInvalidTag);
+}
+
+- (void)testErrorRecursionDepthReached {
+ NSData *data = DataFromCStr(
+ "\x0A\x86\x01\x0A\x83\x01\x0A\x80\x01\x0A\x7E\x0A\x7C\x0A\x7A\x0A\x78"
+ "\x0A\x76\x0A\x74\x0A\x72\x0A\x70\x0A\x6E\x0A\x6C\x0A\x6A\x0A\x68"
+ "\x0A\x66\x0A\x64\x0A\x62\x0A\x60\x0A\x5E\x0A\x5C\x0A\x5A\x0A\x58"
+ "\x0A\x56\x0A\x54\x0A\x52\x0A\x50\x0A\x4E\x0A\x4C\x0A\x4A\x0A\x48"
+ "\x0A\x46\x0A\x44\x0A\x42\x0A\x40\x0A\x3E\x0A\x3C\x0A\x3A\x0A\x38"
+ "\x0A\x36\x0A\x34\x0A\x32\x0A\x30\x0A\x2E\x0A\x2C\x0A\x2A\x0A\x28"
+ "\x0A\x26\x0A\x24\x0A\x22\x0A\x20\x0A\x1E\x0A\x1C\x0A\x1A\x0A\x18"
+ "\x0A\x16\x0A\x14\x0A\x12\x0A\x10\x0A\x0E\x0A\x0C\x0A\x0A\x0A\x08"
+ "\x0A\x06\x12\x04\x72\x02\x4B\x50");
+ NSError *error = nil;
+ NestedTestAllTypes *msg = [NestedTestAllTypes parseFromData:data
+ error:&error];
+ XCTAssertNil(msg);
+ XCTAssertNotNil(error);
+ XCTAssertEqualObjects(error.domain, GPBCodedInputStreamErrorDomain);
+ XCTAssertEqual(error.code, GPBCodedInputStreamErrorRecursionDepthExceeded);
+}
+
+#ifdef DEBUG
+- (void)testErrorMissingRequiredField {
+ NSData *data = DataFromCStr("");
+ NSError *error = nil;
+ TestRequired *msg = [TestRequired parseFromData:data error:&error];
+ XCTAssertNil(msg);
+ XCTAssertNotNil(error);
+ XCTAssertEqualObjects(error.domain, GPBMessageErrorDomain);
+ XCTAssertEqual(error.code, GPBMessageErrorCodeMissingRequiredField);
+}
+#endif
+
#pragma mark - Subset from from map_tests.cc
// TEST(GeneratedMapFieldTest, StandardWireFormat)
@@ -989,8 +1086,8 @@
TestMap *msg = [TestMap parseFromData:data error:&error];
XCTAssertNil(msg);
XCTAssertNotNil(error);
- XCTAssertEqualObjects(error.domain, GPBMessageErrorDomain);
- XCTAssertEqual(error.code, GPBMessageErrorCodeMalformedData);
+ XCTAssertEqualObjects(error.domain, GPBCodedInputStreamErrorDomain);
+ XCTAssertEqual(error.code, GPBCodedInputStreamErrorInvalidSubsectionLimit);
}
// TEST(GeneratedMapFieldTest, Proto2UnknownEnum)
