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/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