THRIFT-2180 Integer types issues in Cocoa lib on ARM64
CLient: Cocoa
Patch: Jeremy W. Sherman
This closes #372
diff --git a/lib/cocoa/src/protocol/TBinaryProtocol.m b/lib/cocoa/src/protocol/TBinaryProtocol.m
index 1508acc..e79bd57 100644
--- a/lib/cocoa/src/protocol/TBinaryProtocol.m
+++ b/lib/cocoa/src/protocol/TBinaryProtocol.m
@@ -21,9 +21,60 @@
#import "TProtocolException.h"
#import "TObjective-C.h"
-int32_t VERSION_1 = 0x80010000;
-int32_t VERSION_MASK = 0xffff0000;
+static const uint16_t VERSION_1 = 0x8001;
+union versioned_size {
+ int32_t i32;
+ struct {
+ uint16_t version;
+ int16_t size;
+ } packed;
+};
+
+NS_INLINE size_t
+CheckedCastInt32ToSizeT(int32_t size)
+{
+ if (size < 0) {
+ NSString *reason = [NSString stringWithFormat:
+ @"%s: refusing to read data with negative size: %"PRId32,
+ __func__, size];
+ @throw [TProtocolException
+ exceptionWithName: @"TProtocolException"
+ reason: reason];
+ }
+ size_t checkedSize = (size_t)size;
+ return checkedSize;
+}
+
+NS_INLINE int32_t
+CheckedCastSizeTToInt32(size_t size)
+{
+ if (size > INT32_MAX) {
+ NSString *reason = [NSString stringWithFormat:
+ @"%s: data size exceeds values representable by a 32-bit signed integer: %zu",
+ __func__, size];
+ @throw [TProtocolException
+ exceptionWithName: @"TProtocolException"
+ reason: reason];
+ }
+ int32_t checkedSize = (int32_t)size;
+ return checkedSize;
+}
+
+NS_INLINE uint8_t
+CheckedCastIntToUInt8(int size)
+{
+ if (size > UINT8_MAX) {
+ NSString *reason = [NSString stringWithFormat:
+ @"%s: data size exceeds values representable by a 8-bit unsigned integer: %d",
+ __func__, size];
+ @throw [TProtocolException
+ exceptionWithName: @"TProtocolException"
+ reason: reason];
+ }
+ uint8_t checkedSize = (uint8_t)size;
+ return checkedSize;
+}
static TBinaryProtocolFactory * gSharedFactory = nil;
@@ -89,12 +140,13 @@
}
-- (NSString *) readStringBody: (int) size
+- (NSString *) readStringBody: (int) rawSize
{
+ size_t size = CheckedCastInt32ToSizeT(rawSize);
char * buffer = malloc(size+1);
if (!buffer) {
@throw [TProtocolException exceptionWithName: @"TProtocolException"
- reason: [NSString stringWithFormat: @"Unable to allocate memory in %s, size: %i",
+ reason: [NSString stringWithFormat: @"Unable to allocate memory in %s, size: %zu",
__PRETTY_FUNCTION__,
size]];;
}
@@ -112,10 +164,15 @@
{
int32_t size = [self readI32];
if (size < 0) {
- int version = size & VERSION_MASK;
+ union versioned_size vsize;
+ vsize.i32 = size;
+ uint16_t version = vsize.packed.version;
if (version != VERSION_1) {
+ NSString *reason = [NSString stringWithFormat:
+ @"%s: Expected version %"PRIu16", instead found: %"PRIu16,
+ __func__, VERSION_1, version];
@throw [TProtocolException exceptionWithName: @"TProtocolException"
- reason: @"Bad version in readMessageBegin"];
+ reason: reason];
}
if (type != NULL) {
*type = size & 0x00FF;
@@ -206,7 +263,7 @@
- (NSString *) readString
{
- int size = [self readI32];
+ int32_t size = [self readI32];
return [self readStringBody: size];
}
@@ -230,10 +287,9 @@
return (short)
(((buff[0] & 0xff) << 8) |
((buff[1] & 0xff)));
- return 0;
}
-- (int64_t) readI64;
+- (int64_t) readI64
{
uint8_t i64rd[8];
[mTransport readAll: i64rd offset: 0 length: 8];
@@ -248,7 +304,7 @@
((int64_t)(i64rd[7] & 0xff));
}
-- (double) readDouble;
+- (double) readDouble
{
// FIXME - will this get us into trouble on PowerPC?
int64_t ieee754 = [self readI64];
@@ -259,15 +315,16 @@
- (NSData *) readBinary
{
int32_t size = [self readI32];
- uint8_t * buff = malloc(size);
+ size_t binarySize = CheckedCastInt32ToSizeT(size);
+ uint8_t * buff = malloc(binarySize);
if (buff == NULL) {
@throw [TProtocolException
exceptionWithName: @"TProtocolException"
reason: [NSString stringWithFormat: @"Out of memory. Unable to allocate %d bytes trying to read binary data.",
size]];
}
- [mTransport readAll: buff offset: 0 length: size];
- return [NSData dataWithBytesNoCopy: buff length: size];
+ [mTransport readAll: buff offset: 0 length: binarySize];
+ return [NSData dataWithBytesNoCopy: buff length: binarySize];
}
@@ -343,7 +400,7 @@
[self writeI32: sequenceID];
} else {
[self writeString: name];
- [self writeByte: messageType];
+ [self writeByte: CheckedCastIntToUInt8(messageType)];
[self writeI32: sequenceID];
}
}
@@ -362,8 +419,8 @@
type: (int) fieldType
fieldID: (int) fieldID
{
- [self writeByte: fieldType];
- [self writeI16: fieldID];
+ [self writeByte: CheckedCastIntToUInt8(fieldType)];
+ [self writeI16: CheckedCastIntToUInt8(fieldID)];
}
@@ -413,7 +470,8 @@
if (value != nil) {
const char * utf8Bytes = [value UTF8String];
size_t length = strlen(utf8Bytes);
- [self writeI32: length];
+ int32_t size = CheckedCastSizeTToInt32(length);
+ [self writeI32: size];
[mTransport write: (uint8_t *) utf8Bytes offset: 0 length: length];
} else {
// instead of crashing when we get null, let's write out a zero
@@ -425,7 +483,8 @@
- (void) writeBinary: (NSData *) data
{
- [self writeI32: [data length]];
+ int32_t size = CheckedCastSizeTToInt32([data length]);
+ [self writeI32: size];
[mTransport write: [data bytes] offset: 0 length: [data length]];
}
@@ -442,8 +501,8 @@
valueType: (int) valueType
size: (int) size
{
- [self writeByte: keyType];
- [self writeByte: valueType];
+ [self writeByte: CheckedCastIntToUInt8(keyType)];
+ [self writeByte: CheckedCastIntToUInt8(valueType)];
[self writeI32: size];
}
@@ -453,7 +512,7 @@
- (void) writeSetBeginWithElementType: (int) elementType
size: (int) size
{
- [self writeByte: elementType];
+ [self writeByte: CheckedCastIntToUInt8(elementType)];
[self writeI32: size];
}
@@ -463,7 +522,7 @@
- (void) writeListBeginWithElementType: (int) elementType
size: (int) size
{
- [self writeByte: elementType];
+ [self writeByte: CheckedCastIntToUInt8(elementType)];
[self writeI32: size];
}
diff --git a/lib/cocoa/src/server/TSocketServer.m b/lib/cocoa/src/server/TSocketServer.m
index 4941445..07bc829 100644
--- a/lib/cocoa/src/server/TSocketServer.m
+++ b/lib/cocoa/src/server/TSocketServer.m
@@ -37,7 +37,7 @@
- (id) initWithPort: (int) port
protocolFactory: (id <TProtocolFactory>) protocolFactory
- processorFactory: (id <TProcessorFactory>) processorFactory;
+ processorFactory: (id <TProcessorFactory>) processorFactory
{
self = [super init];
@@ -49,7 +49,9 @@
int fd = -1;
CFSocketRef socket = CFSocketCreate(kCFAllocatorDefault, PF_INET, SOCK_STREAM, IPPROTO_TCP, 0, NULL, NULL);
if (socket) {
- CFSocketSetSocketFlags(socket, CFSocketGetSocketFlags(socket) & ~kCFSocketCloseOnInvalidate);
+ CFOptionFlags flagsToClear = kCFSocketCloseOnInvalidate;
+ CFSocketSetSocketFlags(socket, CFSocketGetSocketFlags(socket) & ~flagsToClear);
+
fd = CFSocketGetNative(socket);
int yes = 1;
setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void *)&yes, sizeof(yes));
@@ -137,6 +139,7 @@
} while (result);
}
@catch (TTransportException * te) {
+ (void)te;
//NSLog(@"Caught transport exception, abandoning client connection: %@", te);
}
diff --git a/lib/cocoa/src/transport/TFramedTransport.m b/lib/cocoa/src/transport/TFramedTransport.m
index 085f9b6..2148806 100644
--- a/lib/cocoa/src/transport/TFramedTransport.m
+++ b/lib/cocoa/src/transport/TFramedTransport.m
@@ -52,20 +52,21 @@
- (void)flush
{
- int len = [writeBuffer length];
- int data_len = len - HEADER_SIZE;
- if (data_len < 0)
+ size_t headerAndDataLength = [writeBuffer length];
+ if (headerAndDataLength < HEADER_SIZE) {
@throw [TTransportException exceptionWithReason:@"Framed transport buffer has no header"];
+ }
+ size_t dataLength = headerAndDataLength - HEADER_SIZE;
uint8_t i32rd[HEADER_SIZE];
- i32rd[0] = (uint8_t)(0xff & (data_len >> 24));
- i32rd[1] = (uint8_t)(0xff & (data_len >> 16));
- i32rd[2] = (uint8_t)(0xff & (data_len >> 8));
- i32rd[3] = (uint8_t)(0xff & (data_len));
+ i32rd[0] = (uint8_t)(0xff & (dataLength >> 24));
+ i32rd[1] = (uint8_t)(0xff & (dataLength >> 16));
+ i32rd[2] = (uint8_t)(0xff & (dataLength >> 8));
+ i32rd[3] = (uint8_t)(0xff & (dataLength));
// should we make a copy of the writeBuffer instead? Better for threaded operations!
[writeBuffer replaceBytesInRange:NSMakeRange(0, HEADER_SIZE) withBytes:i32rd length:HEADER_SIZE];
- [mTransport write:[writeBuffer mutableBytes] offset:0 length:len];
+ [mTransport write:[writeBuffer mutableBytes] offset:0 length:headerAndDataLength];
[mTransport flush];
// reuse old memory buffer
@@ -73,54 +74,70 @@
[writeBuffer appendBytes:dummy_header length:HEADER_SIZE];
}
-- (void)write:(const uint8_t *)data offset:(unsigned int)offset length:(unsigned int)length
+- (void) write: (const uint8_t *) data offset: (size_t) offset length: (size_t) length
{
[writeBuffer appendBytes:data+offset length:length];
}
-- (int)readAll:(uint8_t *)buf offset:(int)off length:(int)len {
+- (size_t) readAll: (uint8_t *) buf offset: (size_t) offset length: (size_t) length
+{
if (readBuffer == nil) {
[self readFrame];
}
if (readBuffer != nil) {
- int buffer_len = [readBuffer length];
- if (buffer_len-readOffset >= len) {
- [readBuffer getBytes:buf range:NSMakeRange(readOffset,len)]; // copy data
- readOffset += len;
+ size_t bufferLength = [readBuffer length];
+ if (bufferLength - readOffset >= length) {
+ [readBuffer getBytes:buf range:NSMakeRange(readOffset,length)]; // copy data
+ readOffset += length;
} else {
// void the previous readBuffer data and request a new frame
[self readFrame];
- [readBuffer getBytes:buf range:NSMakeRange(0,len)]; // copy data
- readOffset = len;
+ [readBuffer getBytes:buf range:NSMakeRange(0,length)]; // copy data
+ readOffset = length;
}
}
- return len;
+ return length;
}
- (void)readFrame
{
uint8_t i32rd[HEADER_SIZE];
[mTransport readAll: i32rd offset: 0 length: HEADER_SIZE];
- int size =
+ int32_t headerValue =
((i32rd[0] & 0xff) << 24) |
((i32rd[1] & 0xff) << 16) |
((i32rd[2] & 0xff) << 8) |
((i32rd[3] & 0xff));
+ if (headerValue < 0) {
+ NSString *reason = [NSString stringWithFormat:
+ @"Frame header reports negative frame size: %"PRId32,
+ headerValue];
+ @throw [TTransportException exceptionWithReason:reason];
+ }
+ /* Cast should be safe:
+ * Have verified headerValue non-negative and of lesser or equal bitwidth to size_t. */
+ size_t frameSize = (size_t)headerValue;
+ [self ensureReadBufferHasLength:frameSize];
+
+ [mTransport readAll:[readBuffer mutableBytes] offset:0 length:frameSize];
+}
+
+- (void)ensureReadBufferHasLength:(size_t)length
+{
if (readBuffer == nil) {
- readBuffer = [[NSMutableData alloc] initWithLength:size];
+ readBuffer = [[NSMutableData alloc] initWithLength:length];
} else {
- int len = [readBuffer length];
- if (len >= size) {
- [readBuffer setLength:size];
+ size_t currentLength = [readBuffer length];
+ BOOL isTooLong = (currentLength >= length);
+ if (isTooLong) {
+ [readBuffer setLength:length];
} else {
- // increase length of data buffer
- [readBuffer increaseLengthBy:size-len];
+ size_t lengthToAdd = length - currentLength;
+ [readBuffer increaseLengthBy:lengthToAdd];
}
}
- // copy into internal memory buffer
- [mTransport readAll:[readBuffer mutableBytes] offset:0 length:size];
}
@end
diff --git a/lib/cocoa/src/transport/THTTPClient.h b/lib/cocoa/src/transport/THTTPClient.h
index 4d57840..78935fb 100644
--- a/lib/cocoa/src/transport/THTTPClient.h
+++ b/lib/cocoa/src/transport/THTTPClient.h
@@ -25,7 +25,7 @@
NSMutableURLRequest * mRequest;
NSMutableData * mRequestData;
NSData * mResponseData;
- int mResponseDataOffset;
+ size_t mResponseDataOffset;
NSString * mUserAgent;
int mTimeout;
}
diff --git a/lib/cocoa/src/transport/THTTPClient.m b/lib/cocoa/src/transport/THTTPClient.m
index 5617d45..169927c 100644
--- a/lib/cocoa/src/transport/THTTPClient.m
+++ b/lib/cocoa/src/transport/THTTPClient.m
@@ -102,20 +102,20 @@
}
-- (int) readAll: (uint8_t *) buf offset: (int) off length: (int) len
+- (size_t) readAll: (uint8_t *) buf offset: (size_t) offset length: (size_t) length
{
NSRange r;
r.location = mResponseDataOffset;
- r.length = len;
+ r.length = length;
- [mResponseData getBytes: buf+off range: r];
- mResponseDataOffset += len;
+ [mResponseData getBytes: buf+offset range: r];
+ mResponseDataOffset += length;
- return len;
+ return length;
}
-- (void) write: (const uint8_t *) data offset: (unsigned int) offset length: (unsigned int) length
+- (void) write: (const uint8_t *) data offset: (size_t) offset length: (size_t) length
{
[mRequestData appendBytes: data+offset length: length];
}
@@ -147,8 +147,8 @@
NSHTTPURLResponse * httpResponse = (NSHTTPURLResponse *) response;
if ([httpResponse statusCode] != 200) {
@throw [TTransportException exceptionWithName: @"TTransportException"
- reason: [NSString stringWithFormat: @"Bad response from HTTP server: %d",
- [httpResponse statusCode]]];
+ reason: [NSString stringWithFormat: @"Bad response from HTTP server: %ld",
+ (long)[httpResponse statusCode]]];
}
// phew!
diff --git a/lib/cocoa/src/transport/TMemoryBuffer.m b/lib/cocoa/src/transport/TMemoryBuffer.m
index c3801c7..4513ab8 100644
--- a/lib/cocoa/src/transport/TMemoryBuffer.m
+++ b/lib/cocoa/src/transport/TMemoryBuffer.m
@@ -25,7 +25,7 @@
@implementation TMemoryBuffer
- (id)init {
- if (self = [super init]) {
+ if ((self = [super init])) {
mBuffer = [[NSMutableData alloc] init];
mOffset = 0;
}
@@ -33,27 +33,29 @@
}
- (id)initWithData:(NSData *)data {
- if (self = [super init]) {
+ if ((self = [super init])) {
mBuffer = [data mutableCopy];
mOffset = 0;
}
return self;
}
-- (int)readAll:(uint8_t *)buf offset:(int)off length:(int)len {
- if ([mBuffer length] - mOffset < len) {
+- (size_t) readAll: (uint8_t *) buf offset: (size_t) offset length: (size_t) length
+{
+ if ([mBuffer length] - mOffset < length) {
@throw [TTransportException exceptionWithReason:@"Not enough bytes remain in buffer"];
}
- [mBuffer getBytes:buf range:NSMakeRange(mOffset, len)];
- mOffset += len;
+ [mBuffer getBytes:buf range:NSMakeRange(mOffset, length)];
+ mOffset += length;
if (mOffset >= GARBAGE_BUFFER_SIZE) {
[mBuffer replaceBytesInRange:NSMakeRange(0, mOffset) withBytes:NULL length:0];
mOffset = 0;
}
- return len;
+ return length;
}
-- (void)write:(const uint8_t *)data offset:(unsigned int)offset length:(unsigned int)length {
+- (void) write: (const uint8_t *) data offset: (size_t) offset length: (size_t) length
+{
[mBuffer appendBytes:data+offset length:length];
}
diff --git a/lib/cocoa/src/transport/TNSFileHandleTransport.m b/lib/cocoa/src/transport/TNSFileHandleTransport.m
index 0ff200b..c2b18ca 100644
--- a/lib/cocoa/src/transport/TNSFileHandleTransport.m
+++ b/lib/cocoa/src/transport/TNSFileHandleTransport.m
@@ -51,26 +51,26 @@
}
-- (int) readAll: (uint8_t *) buf offset: (int) off length: (int) len
+- (size_t) readAll: (uint8_t *) buf offset: (size_t) offset length: (size_t) length
{
- int got = 0;
- while (got < len) {
- NSData * d = [mInputFileHandle readDataOfLength: len-got];
- if ([d length] == 0) {
+ size_t totalBytesRead = 0;
+ while (totalBytesRead < length) {
+ NSData * data = [mInputFileHandle readDataOfLength: length-totalBytesRead];
+ if ([data length] == 0) {
@throw [TTransportException exceptionWithName: @"TTransportException"
reason: @"Cannot read. No more data."];
}
- [d getBytes: buf+got];
- got += [d length];
+ [data getBytes: buf+totalBytesRead];
+ totalBytesRead += [data length];
}
- return got;
+ return totalBytesRead;
}
-- (void) write: (const uint8_t *) data offset: (unsigned int) offset length: (unsigned int) length
+- (void) write: (const uint8_t *) data offset: (size_t) offset length: (size_t) length
{
- void *pos = (void *) data + offset;
- NSData * dataObject = [[NSData alloc] initWithBytesNoCopy: pos // data+offset
+ const void *pos = data + offset;
+ NSData * dataObject = [[NSData alloc] initWithBytesNoCopy: (void *)pos
length: length
freeWhenDone: NO];
diff --git a/lib/cocoa/src/transport/TNSStreamTransport.m b/lib/cocoa/src/transport/TNSStreamTransport.m
index e2bc249..7ac1cdc 100644
--- a/lib/cocoa/src/transport/TNSStreamTransport.m
+++ b/lib/cocoa/src/transport/TNSStreamTransport.m
@@ -51,34 +51,40 @@
}
-- (int) readAll: (uint8_t *) buf offset: (int) off length: (int) len
+- (size_t) readAll: (uint8_t *) buf offset: (size_t) offset length: (size_t) length
{
- int got = 0;
- int ret = 0;
- while (got < len) {
- ret = [self.mInput read: buf+off+got maxLength: len-got];
- if (ret <= 0) {
+ size_t totalBytesRead = 0;
+ ssize_t bytesRead = 0;
+ while (totalBytesRead < length) {
+ bytesRead = [self.mInput read: buf+offset+totalBytesRead maxLength: length-totalBytesRead];
+
+ BOOL encounteredErrorOrEOF = (bytesRead <= 0);
+ if (encounteredErrorOrEOF) {
@throw [TTransportException exceptionWithReason: @"Cannot read. Remote side has closed."];
+ } else {
+ /* bytesRead is guaranteed to be positive and within the range representable by size_t. */
+ totalBytesRead += (size_t)bytesRead;
}
- got += ret;
}
- return got;
+ return totalBytesRead;
}
-- (void) write: (const uint8_t *) data offset: (unsigned int) offset length: (unsigned int) length
+- (void) write: (const uint8_t *) data offset: (size_t) offset length: (size_t) length
{
- int got = 0;
- int result = 0;
- while (got < length) {
- result = [self.mOutput write: data+offset+got maxLength: length-got];
- if (result == -1) {
+ size_t totalBytesWritten = 0;
+ ssize_t bytesWritten = 0;
+ while (totalBytesWritten < length) {
+ bytesWritten = [self.mOutput write: data+offset+totalBytesWritten maxLength: length-totalBytesWritten];
+ if (bytesWritten < 0) {
@throw [TTransportException exceptionWithReason: @"Error writing to transport output stream."
error: [self.mOutput streamError]];
- } else if (result == 0) {
+ } else if (bytesWritten == 0) {
@throw [TTransportException exceptionWithReason: @"End of output stream."];
+ } else {
+ /* bytesWritten is guaranteed to be positive and within the range representable by size_t. */
+ totalBytesWritten += (size_t)bytesWritten;
}
- got += result;
}
}
diff --git a/lib/cocoa/src/transport/TSSLSocketClient.m b/lib/cocoa/src/transport/TSSLSocketClient.m
index 5be04ef..d8c55d6 100644
--- a/lib/cocoa/src/transport/TSSLSocketClient.m
+++ b/lib/cocoa/src/transport/TSSLSocketClient.m
@@ -59,10 +59,10 @@
break;
}
}
-
+
memset (&pin, 0, sizeof(pin));
pin.sin_family = AF_INET;
- pin.sin_addr.s_addr = ((struct in_addr *) (hp->h_addr))->s_addr;
+ memcpy(&pin.sin_addr, hp->h_addr, sizeof(struct in_addr));
pin.sin_port = htons (port);
/* create the socket */
@@ -199,8 +199,6 @@
}
case NSStreamEventEndEncountered:
break;
- default:
- break;
}
}
diff --git a/lib/cocoa/src/transport/TSocketClient.h b/lib/cocoa/src/transport/TSocketClient.h
index 372850f..81a0247 100644
--- a/lib/cocoa/src/transport/TSocketClient.h
+++ b/lib/cocoa/src/transport/TSocketClient.h
@@ -28,7 +28,7 @@
}
- (id) initWithHostname: (NSString *) hostname
- port: (int) port;
+ port: (UInt32) port;
@end
diff --git a/lib/cocoa/src/transport/TSocketClient.m b/lib/cocoa/src/transport/TSocketClient.m
index 1a7eea8..b0bac74 100644
--- a/lib/cocoa/src/transport/TSocketClient.m
+++ b/lib/cocoa/src/transport/TSocketClient.m
@@ -35,7 +35,7 @@
@implementation TSocketClient
- (id) initWithHostname: (NSString *) hostname
- port: (int) port
+ port: (UInt32) port
{
inputStream = NULL;
outputStream = NULL;
diff --git a/lib/cocoa/src/transport/TTransport.h b/lib/cocoa/src/transport/TTransport.h
index 61ebbd2..83aad9e 100644
--- a/lib/cocoa/src/transport/TTransport.h
+++ b/lib/cocoa/src/transport/TTransport.h
@@ -23,14 +23,14 @@
* Guarantees that all of len bytes are read
*
* @param buf Buffer to read into
- * @param off Index in buffer to start storing bytes at
- * @param len Maximum number of bytes to read
+ * @param offset Index in buffer to start storing bytes at
+ * @param length Maximum number of bytes to read
* @return The number of bytes actually read, which must be equal to len
* @throws TTransportException if there was an error reading data
*/
-- (int) readAll: (uint8_t *) buf offset: (int) off length: (int) len;
+- (size_t) readAll: (uint8_t *) buf offset: (size_t) offset length: (size_t) length;
-- (void) write: (const uint8_t *) data offset: (unsigned int) offset length: (unsigned int) length;
+- (void) write: (const uint8_t *) data offset: (size_t) offset length: (size_t) length;
- (void) flush;
@end