Improves coverage of GPBCodedInputStream

sergiocampama · sergiocampama · commit b1f954e639f7 · 2017-10-24T10:44:41.000-04:00
diff --git 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 @@ - (NSString *)readString {
 - (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)readGroup:(int32_t)fieldNumber
 
 - (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)readUnknownGroup:(int32_t)fieldNumber
 
 - (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)readMapEntry:(id)mapDictionary
     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
@@ -62,52 +62,101 @@ - (NSData*)bytes_with_sentinel:(int32_t)unused, ... {
 #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 @@ - (void)testBytes {
   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 @@ - (void)testReadVarint {
   [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 @@ - (void)testReadMaliciouslyLargeBlob {
   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
@@ -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 @@ - (void)testErrorRecursionDepthReached {
   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
@@ -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
@@ -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
