From b1f954e639f7cfd2c8768df62347f1e97a85aa7c Mon Sep 17 00:00:00 2001 From: Sergio Campama Date: Thu, 5 Oct 2017 17:47:22 -0400 Subject: Improves coverage of GPBCodedInputStream --- objectivec/GPBCodedInputStream.m | 61 ++------ objectivec/Tests/GPBCodedInputStreamTests.m | 185 +++++++++++++++++++---- objectivec/Tests/GPBMessageTests+Serialization.m | 14 +- objectivec/Tests/GPBTestUtilities.h | 3 + objectivec/Tests/GPBUnknownFieldSetTest.m | 4 - 5 files changed, 182 insertions(+), 85 deletions(-) diff --git a/objectivec/GPBCodedInputStream.m b/objectivec/GPBCodedInputStream.m index 22859e77..0759640d 100644 --- a/objectivec/GPBCodedInputStream.m +++ b/objectivec/GPBCodedInputStream.m @@ -68,6 +68,12 @@ static void RaiseException(NSInteger code, NSString *reason) { userInfo:exceptionInfo] raise]; } +static void CheckRecursionLimit(GPBCodedInputStreamState *state) { + if (state->recursionDepth >= kDefaultRecursionLimit) { + RaiseException(GPBCodedInputStreamErrorRecursionDepthExceeded, nil); + } +} + static void CheckSize(GPBCodedInputStreamState *state, size_t size) { size_t newSize = state->bufferPos + size; if (newSize > state->bufferSize) { @@ -99,41 +105,6 @@ static int64_t ReadRawLittleEndian64(GPBCodedInputStreamState *state) { return value; } -static int32_t ReadRawVarint32(GPBCodedInputStreamState *state) { - int8_t tmp = ReadRawByte(state); - if (tmp >= 0) { - return tmp; - } - int32_t result = tmp & 0x7f; - if ((tmp = ReadRawByte(state)) >= 0) { - result |= tmp << 7; - } else { - result |= (tmp & 0x7f) << 7; - if ((tmp = ReadRawByte(state)) >= 0) { - result |= tmp << 14; - } else { - result |= (tmp & 0x7f) << 14; - if ((tmp = ReadRawByte(state)) >= 0) { - result |= tmp << 21; - } else { - result |= (tmp & 0x7f) << 21; - result |= (tmp = ReadRawByte(state)) << 28; - if (tmp < 0) { - // Discard upper 32 bits. - for (int i = 0; i < 5; i++) { - if (ReadRawByte(state) >= 0) { - return result; - } - } - RaiseException(GPBCodedInputStreamErrorInvalidVarInt, - @"Invalid VarInt32"); - } - } - } - } - return result; -} - static int64_t ReadRawVarint64(GPBCodedInputStreamState *state) { int32_t shift = 0; int64_t result = 0; @@ -149,6 +120,10 @@ static int64_t ReadRawVarint64(GPBCodedInputStreamState *state) { return 0; } +static int32_t ReadRawVarint32(GPBCodedInputStreamState *state) { + return (int32_t)ReadRawVarint64(state); +} + static void SkipRawData(GPBCodedInputStreamState *state, size_t size) { CheckSize(state, size); state->bufferPos += size; @@ -452,9 +427,7 @@ void GPBCodedInputStreamCheckLastTagWas(GPBCodedInputStreamState *state, - (void)readGroup:(int32_t)fieldNumber message:(GPBMessage *)message extensionRegistry:(GPBExtensionRegistry *)extensionRegistry { - if (state_.recursionDepth >= kDefaultRecursionLimit) { - RaiseException(GPBCodedInputStreamErrorRecursionDepthExceeded, nil); - } + CheckRecursionLimit(&state_); ++state_.recursionDepth; [message mergeFromCodedInputStream:self extensionRegistry:extensionRegistry]; GPBCodedInputStreamCheckLastTagWas( @@ -464,9 +437,7 @@ void GPBCodedInputStreamCheckLastTagWas(GPBCodedInputStreamState *state, - (void)readUnknownGroup:(int32_t)fieldNumber message:(GPBUnknownFieldSet *)message { - if (state_.recursionDepth >= kDefaultRecursionLimit) { - RaiseException(GPBCodedInputStreamErrorRecursionDepthExceeded, nil); - } + CheckRecursionLimit(&state_); ++state_.recursionDepth; [message mergeFromCodedInputStream:self]; GPBCodedInputStreamCheckLastTagWas( @@ -476,10 +447,8 @@ void GPBCodedInputStreamCheckLastTagWas(GPBCodedInputStreamState *state, - (void)readMessage:(GPBMessage *)message extensionRegistry:(GPBExtensionRegistry *)extensionRegistry { + CheckRecursionLimit(&state_); int32_t length = ReadRawVarint32(&state_); - if (state_.recursionDepth >= kDefaultRecursionLimit) { - RaiseException(GPBCodedInputStreamErrorRecursionDepthExceeded, nil); - } size_t oldLimit = GPBCodedInputStreamPushLimit(&state_, length); ++state_.recursionDepth; [message mergeFromCodedInputStream:self extensionRegistry:extensionRegistry]; @@ -492,10 +461,8 @@ void GPBCodedInputStreamCheckLastTagWas(GPBCodedInputStreamState *state, extensionRegistry:(GPBExtensionRegistry *)extensionRegistry field:(GPBFieldDescriptor *)field parentMessage:(GPBMessage *)parentMessage { + CheckRecursionLimit(&state_); int32_t length = ReadRawVarint32(&state_); - if (state_.recursionDepth >= kDefaultRecursionLimit) { - RaiseException(GPBCodedInputStreamErrorRecursionDepthExceeded, nil); - } size_t oldLimit = GPBCodedInputStreamPushLimit(&state_, length); ++state_.recursionDepth; GPBDictionaryReadEntry(mapDictionary, self, extensionRegistry, field, diff --git a/objectivec/Tests/GPBCodedInputStreamTests.m b/objectivec/Tests/GPBCodedInputStreamTests.m index cc402156..d8e128f7 100644 --- a/objectivec/Tests/GPBCodedInputStreamTests.m +++ b/objectivec/Tests/GPBCodedInputStreamTests.m @@ -62,52 +62,101 @@ #define bytes(...) [self bytes_with_sentinel:0, __VA_ARGS__, 256] - (void)testDecodeZigZag { - XCTAssertEqual(0, GPBDecodeZigZag32(0)); - XCTAssertEqual(-1, GPBDecodeZigZag32(1)); - XCTAssertEqual(1, GPBDecodeZigZag32(2)); - XCTAssertEqual(-2, GPBDecodeZigZag32(3)); - XCTAssertEqual((int32_t)0x3FFFFFFF, GPBDecodeZigZag32(0x7FFFFFFE)); - XCTAssertEqual((int32_t)0xC0000000, GPBDecodeZigZag32(0x7FFFFFFF)); - XCTAssertEqual((int32_t)0x7FFFFFFF, GPBDecodeZigZag32(0xFFFFFFFE)); - XCTAssertEqual((int32_t)0x80000000, GPBDecodeZigZag32(0xFFFFFFFF)); - - XCTAssertEqual((int64_t)0, GPBDecodeZigZag64(0)); - XCTAssertEqual((int64_t)-1, GPBDecodeZigZag64(1)); - XCTAssertEqual((int64_t)1, GPBDecodeZigZag64(2)); - XCTAssertEqual((int64_t)-2, GPBDecodeZigZag64(3)); - XCTAssertEqual((int64_t)0x000000003FFFFFFFL, - GPBDecodeZigZag64(0x000000007FFFFFFEL)); - XCTAssertEqual((int64_t)0xFFFFFFFFC0000000L, - GPBDecodeZigZag64(0x000000007FFFFFFFL)); - XCTAssertEqual((int64_t)0x000000007FFFFFFFL, - GPBDecodeZigZag64(0x00000000FFFFFFFEL)); - XCTAssertEqual((int64_t)0xFFFFFFFF80000000L, - GPBDecodeZigZag64(0x00000000FFFFFFFFL)); - XCTAssertEqual((int64_t)0x7FFFFFFFFFFFFFFFL, - GPBDecodeZigZag64(0xFFFFFFFFFFFFFFFEL)); - XCTAssertEqual((int64_t)0x8000000000000000L, - GPBDecodeZigZag64(0xFFFFFFFFFFFFFFFFL)); + [self assertReadZigZag32:bytes(0x0) value:0]; + [self assertReadZigZag32:bytes(0x1) value:-1]; + [self assertReadZigZag32:bytes(0x2) value:1]; + [self assertReadZigZag32:bytes(0x3) value:-2]; + + [self assertReadZigZag32:bytes(0xFE, 0xFF, 0xFF, 0xFF, 0x07) value:(int32_t)0x3FFFFFFF]; + [self assertReadZigZag32:bytes(0xFF, 0xFF, 0xFF, 0xFF, 0x07) value:(int32_t)0xC0000000]; + [self assertReadZigZag32:bytes(0xFE, 0xFF, 0xFF, 0xFF, 0x0F) value:(int32_t)0x7FFFFFFF]; + [self assertReadZigZag32:bytes(0xFF, 0xFF, 0xFF, 0xFF, 0x0F) value:(int32_t)0x80000000]; + + [self assertReadZigZag64:bytes(0x0) value:0]; + [self assertReadZigZag64:bytes(0x1) value:-1]; + [self assertReadZigZag64:bytes(0x2) value:1]; + [self assertReadZigZag64:bytes(0x3) value:-2]; + + [self assertReadZigZag64:bytes(0xFE, 0xFF, 0xFF, 0xFF, 0x07) value:(int32_t)0x3FFFFFFF]; + [self assertReadZigZag64:bytes(0xFF, 0xFF, 0xFF, 0xFF, 0x07) value:(int32_t)0xC0000000]; + [self assertReadZigZag64:bytes(0xFE, 0xFF, 0xFF, 0xFF, 0x0F) value:(int32_t)0x7FFFFFFF]; + [self assertReadZigZag64:bytes(0xFF, 0xFF, 0xFF, 0xFF, 0x0F) value:(int32_t)0x80000000]; + + [self assertReadZigZag64:bytes(0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x01) value:0x7FFFFFFFFFFFFFFFL]; + [self assertReadZigZag64:bytes(0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x01) value:0x8000000000000000L]; } - (void)assertReadVarint:(NSData*)data value:(int64_t)value { - { + if (value <= INT32_MAX && value >= INT32_MIN) { + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual((int32_t)value, [input readInt32]); + } + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual((int32_t)value, [input readEnum]); + } + } + if (value <= UINT32_MAX && value >= 0) { GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; - XCTAssertEqual((int32_t)value, [input readInt32]); + XCTAssertEqual((uint32_t)value, [input readUInt32]); } { GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; XCTAssertEqual(value, [input readInt64]); } + if (value >= 0) { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual((uint64_t)value, [input readUInt64]); + } } - (void)assertReadLittleEndian32:(NSData*)data value:(int32_t)value { - GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; - XCTAssertEqual(value, [input readSFixed32]); + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(value, [input readSFixed32]); + } + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(GPBConvertInt32ToFloat(value), [input readFloat]); + } + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual((uint32_t)value, [input readFixed32]); + } + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(value, [input readSFixed32]); + } } - (void)assertReadLittleEndian64:(NSData*)data value:(int64_t)value { + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(value, [input readSFixed64]); + } + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(GPBConvertInt64ToDouble(value), [input readDouble]); + } + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual((uint64_t)value, [input readFixed64]); + } + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(value, [input readSFixed64]); + } +} + +- (void)assertReadZigZag32:(NSData*)data value:(int64_t)value { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual((int32_t)value, [input readSInt32]); +} + +- (void)assertReadZigZag64:(NSData*)data value:(int64_t)value { GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; - XCTAssertEqual(value, [input readSFixed64]); + XCTAssertEqual(value, [input readSInt64]); } - (void)assertReadVarintFailure:(NSData*)data { @@ -128,12 +177,28 @@ XCTAssertEqual(((uint8_t*)data.bytes)[1], (uint8_t)0x74); } +- (void)testReadBool { + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:bytes(0x00)]; + XCTAssertEqual(NO, [input readBool]); + } + { + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:bytes(0x01)]; + XCTAssertEqual(YES, [input readBool]); + } +} + - (void)testReadVarint { [self assertReadVarint:bytes(0x00) value:0]; [self assertReadVarint:bytes(0x01) value:1]; [self assertReadVarint:bytes(0x7f) value:127]; // 14882 [self assertReadVarint:bytes(0xa2, 0x74) value:(0x22 << 0) | (0x74 << 7)]; + // 1904930 + [self assertReadVarint:bytes(0xa2, 0xa2, 0x74) value:(0x22 << 0) | (0x22 << 7) | (0x74 << 14)]; + // 243831074 + [self assertReadVarint:bytes(0xa2, 0xa2, 0xa2, 0x74) + value:(0x22 << 0) | (0x22 << 7) | (0x22 << 14) | (0x74 << 21)]; // 2961488830 [self assertReadVarint:bytes(0xbe, 0xf7, 0x92, 0x84, 0x0b) value:(0x3e << 0) | (0x77 << 7) | (0x12 << 14) | @@ -163,6 +228,45 @@ [self assertReadVarintFailure:bytes(0x80)]; } +- (void)testReadVarint32FromVarint64 { + { + // Turn on lower 31 bits of the upper half on a 64 bit varint. + NSData* data = bytes(0x80, 0x80, 0x80, 0x80, 0xF0, 0xFF, 0xFF, 0xFF, 0xFF, 0x7E); + + int32_t value32 = 0x0; + GPBCodedInputStream* input32 = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(value32, [input32 readInt32]); + + int64_t value64 = INT64_MAX & 0xFFFFFFFF00000000; + GPBCodedInputStream* input64 = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(value64, [input64 readInt64]); + } + { + // Turn on lower 31 bits and lower 31 bits on upper half on a 64 bit varint. + NSData* data = bytes(0xFF, 0xFF, 0xFF, 0xFF, 0xF7, 0xFF, 0xFF, 0xFF, 0xFF, 0x7E); + + int32_t value32 = INT32_MAX; + GPBCodedInputStream* input32 = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(value32, [input32 readInt32]); + + int64_t value64 = INT64_MAX & 0xFFFFFFFF7FFFFFFF; + GPBCodedInputStream* input64 = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(value64, [input64 readInt64]); + } + { + // Turn on bits 32 and 64 bit on a 64 bit varint. + NSData* data = bytes(0x80, 0x80, 0x80, 0x80, 0x88, 0x80, 0x80, 0x80, 0x80, 0x01); + + int32_t value32 = INT32_MIN; + GPBCodedInputStream* input32 = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(value32, [input32 readInt32]); + + int64_t value64 = INT64_MIN | (0x01L << 31); + GPBCodedInputStream* input64 = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqual(value64, [input64 readInt64]); + } +} + - (void)testReadLittleEndian { [self assertReadLittleEndian32:bytes(0x78, 0x56, 0x34, 0x12) value:0x12345678]; @@ -265,6 +369,27 @@ XCTAssertThrows([input readBytes]); } +- (void)testReadEmptyString { + NSData *data = bytes(0x00); + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertEqualObjects(@"", [input readString]); +} + +- (void)testInvalidGroupEndTagThrows { + NSData *data = bytes(0x0B, 0x1A, 0x02, 0x4B, 0x50, 0x14); + GPBCodedInputStream* input = [GPBCodedInputStream streamWithData:data]; + XCTAssertThrowsSpecificNamed([input skipMessage], + NSException, + GPBCodedInputStreamException, + @"should throw a GPBCodedInputStreamException exception "); +} + +- (void)testBytesWithNegativeSize { + NSData *data = bytes(0xFF, 0xFF, 0xFF, 0xFF, 0x0F); + GPBCodedInputStream *input = [GPBCodedInputStream streamWithData:data]; + XCTAssertNil([input readBytes]); +} + // Verifies fix for b/10315336. // Note: Now that there isn't a custom string class under the hood, this test // isn't as critical, but it does cover bad input and if a custom class is added diff --git a/objectivec/Tests/GPBMessageTests+Serialization.m b/objectivec/Tests/GPBMessageTests+Serialization.m index 4a4c5447..55d77a1f 100644 --- a/objectivec/Tests/GPBMessageTests+Serialization.m +++ b/objectivec/Tests/GPBMessageTests+Serialization.m @@ -42,10 +42,6 @@ #import "google/protobuf/UnittestRuntimeProto2.pbobjc.h" #import "google/protobuf/UnittestRuntimeProto3.pbobjc.h" -static NSData *DataFromCStr(const char *str) { - return [NSData dataWithBytes:str length:strlen(str)]; -} - @interface MessageSerializationTests : GPBTestCase @end @@ -980,6 +976,16 @@ static NSData *DataFromCStr(const char *str) { XCTAssertEqual(error.code, GPBCodedInputStreamErrorRecursionDepthExceeded); } +- (void)testParseDelimitedDataWithNegativeSize { + NSData *data = DataFromCStr("\xFF\xFF\xFF\xFF\x0F"); + GPBCodedInputStream *input = [GPBCodedInputStream streamWithData:data]; + NSError *error; + [GPBMessage parseDelimitedFromCodedInputStream:input + extensionRegistry:nil + error:&error]; + XCTAssertNil(error); +} + #ifdef DEBUG - (void)testErrorMissingRequiredField { NSData *data = DataFromCStr(""); diff --git a/objectivec/Tests/GPBTestUtilities.h b/objectivec/Tests/GPBTestUtilities.h index 44c80844..780184b0 100644 --- a/objectivec/Tests/GPBTestUtilities.h +++ b/objectivec/Tests/GPBTestUtilities.h @@ -39,6 +39,9 @@ @class TestUnpackedExtensions; @class GPBExtensionRegistry; +static inline NSData *DataFromCStr(const char *str) { + return [NSData dataWithBytes:str length:strlen(str)]; +} // Helper for uses of C arrays in tests cases. #ifndef GPBARRAYSIZE diff --git a/objectivec/Tests/GPBUnknownFieldSetTest.m b/objectivec/Tests/GPBUnknownFieldSetTest.m index 64cbd2d2..8e03a427 100644 --- a/objectivec/Tests/GPBUnknownFieldSetTest.m +++ b/objectivec/Tests/GPBUnknownFieldSetTest.m @@ -34,10 +34,6 @@ #import "GPBUnknownFieldSet_PackagePrivate.h" #import "google/protobuf/Unittest.pbobjc.h" -static NSData *DataFromCStr(const char *str) { - return [NSData dataWithBytes:str length:strlen(str)]; -} - @interface GPBUnknownFieldSet (GPBUnknownFieldSetTest) - (void)getTags:(int32_t*)tags; @end -- cgit v1.2.3