Encoder: handle invalid types gracefully (#592)

* Test: JSON encoder using invalid types * Test: should handle and emit the invalid data error * Fix: handle and emit the invalid data encoder error * Update existing tests * Test: should decode data with malformed JSON * fixup! Fix: handle and emit the invalid data encoder error * Test: should decode data with malformed MsgPack * Test: incoming invalid/non-encodeable payloads and messages * Add error argument on every encode/decode method * Update tests * fixup! Add error argument on every encode/decode method * TestProxyTransport: sendWithData needs to capture protocol messages * Add missing authDetailsFromDictionary * fixup! Add error argument on every encode/decode method * Fix: invalid type cast * Fix: bad access crash * Fix test * fixup! Fix: bad access crash * Fix: missing encoder fields * If message error is empty then use the reason prop * Keep the original error domain to give more context * Transport: use only one way to send data
ably · Apr 13, 2017 · 43baf48 · 43baf48
1 parent 3015161
commit 43baf48
Show file tree

Hide file tree

Showing 27 changed files with 519 additions and 181 deletions.
diff --git a/Source/ARTAuth.m b/Source/ARTAuth.m
@@ -296,8 +296,13 @@ - (void)executeTokenRequest:(ARTTokenRequest *)tokenRequest callback:(void (^)(A
 
     NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:requestUrl];
     request.HTTPMethod = @"POST";
-
-    request.HTTPBody = [encoder encodeTokenRequest:tokenRequest];
+
+    NSError *encodeError = nil;
+    request.HTTPBody = [encoder encodeTokenRequest:tokenRequest error:&encodeError];
+    if (encodeError) {
+        callback(nil, encodeError);
+        return;
+    }
     [request setValue:[encoder mimeType] forHTTPHeaderField:@"Accept"];
     [request setValue:[encoder mimeType] forHTTPHeaderField:@"Content-Type"];
 

diff --git a/Source/ARTBaseMessage.m b/Source/ARTBaseMessage.m
@@ -37,7 +37,7 @@ - (id)decodeWithEncoder:(ARTDataEncoder*)encoder error:(NSError **)error {
     ARTDataEncoderOutput *decoded = [encoder decode:self.data encoding:self.encoding];
     if (decoded.errorInfo && error) {
         *error = [NSError errorWithDomain:ARTAblyErrorDomain code:0 userInfo:@{NSLocalizedDescriptionKey: @"decoding failed",
-                                                                               NSLocalizedFailureReasonErrorKey: decoded.errorInfo}];
+                                                                               NSLocalizedFailureReasonErrorKey: decoded.errorInfo.message}];
     }
     id ret = [self copy];
     ((ARTBaseMessage *)ret).data = decoded.data;
@@ -49,7 +49,7 @@ - (id)encodeWithEncoder:(ARTDataEncoder*)encoder error:(NSError **)error {
     ARTDataEncoderOutput *encoded = [encoder encode:self.data];
     if (encoded.errorInfo && error) {
         *error = [NSError errorWithDomain:ARTAblyErrorDomain code:0 userInfo:@{NSLocalizedDescriptionKey: @"encoding failed",
-                                                                               NSLocalizedFailureReasonErrorKey: encoded.errorInfo}];
+                                                                               NSLocalizedFailureReasonErrorKey: encoded.errorInfo.message}];
     }
     id ret = [self copy];
     ((ARTBaseMessage *)ret).data = encoded.data;

diff --git a/Source/ARTChannel+Private.h b/Source/ARTChannel+Private.h
@@ -17,7 +17,6 @@ ART_ASSUME_NONNULL_BEGIN
 @property (nonatomic, strong, art_null_resettable) ARTChannelOptions *options;
 @property (nonatomic, strong, readonly) ARTDataEncoder *dataEncoder;
 
-- (ARTMessage *__art_nonnull)encodeMessageIfNeeded:(ARTMessage *__art_nonnull)message;
 - (void)internalPostMessages:(id)data callback:(art_nullable void (^)(ARTErrorInfo *__art_nullable error))callback;
 
 @end

diff --git a/Source/ARTChannel.m b/Source/ARTChannel.m
@@ -45,38 +45,57 @@ - (void)publish:(NSString *)name data:(id)data {
 }
 
 - (void)publish:(art_nullable NSString *)name data:(art_nullable id)data callback:(art_nullable void (^)(ARTErrorInfo *__art_nullable error))callback {
-    [self internalPostMessages:[self encodeMessageIfNeeded:[[ARTMessage alloc] initWithName:name data:data]]
-                      callback:callback];
+    NSError *error;
+    ARTMessage *messagesWithDataEncoded = [self encodeMessageIfNeeded:[[ARTMessage alloc] initWithName:name data:data] error:&error];
+    if (error) {
+        if (callback) callback([ARTErrorInfo createWithNSError:error]);
+        return;
+    }
+    [self internalPostMessages:messagesWithDataEncoded callback:callback];
 }
 
 - (void)publish:(NSString *)name data:(id)data clientId:(NSString *)clientId {
     [self publish:name data:data clientId:clientId callback:nil];
 }
 
 - (void)publish:(NSString *)name data:(id)data clientId:(NSString *)clientId callback:(void (^)(ARTErrorInfo * _Nullable))callback {
-    [self internalPostMessages:[self encodeMessageIfNeeded:[[ARTMessage alloc] initWithName:name data:data clientId:clientId]]
-                      callback:callback];
+    NSError *error;
+    ARTMessage *messagesWithDataEncoded = [self encodeMessageIfNeeded:[[ARTMessage alloc] initWithName:name data:data clientId:clientId] error:&error];
+    if (error) {
+        if (callback) callback([ARTErrorInfo createWithNSError:error]);
+        return;
+    }
+    [self internalPostMessages:messagesWithDataEncoded callback:callback];
 }
 
 - (void)publish:(NSArray<ARTMessage *> *)messages {
     [self publish:messages callback:nil];
 }
 
 - (void)publish:(__GENERIC(NSArray, ARTMessage *) *)messages callback:(art_nullable void (^)(ARTErrorInfo *__art_nullable error))callback {
-    [self internalPostMessages:[messages artMap:^id(ARTMessage *message) {
-        return [self encodeMessageIfNeeded:message];
-    }] callback:callback];
+    NSError *error;
+    NSMutableArray<ARTMessage *> *messagesWithDataEncoded = [NSMutableArray new];
+    for (ARTMessage *message in messages) {
+        [messagesWithDataEncoded addObject:[self encodeMessageIfNeeded:message error:&error]];
+    }
+    if (error) {
+        callback([ARTErrorInfo createWithNSError:error]);
+        return;
+    }
+    [self internalPostMessages:messagesWithDataEncoded callback:callback];
 }
 
-- (ARTMessage *)encodeMessageIfNeeded:(ARTMessage *)message {
+- (ARTMessage *)encodeMessageIfNeeded:(ARTMessage *)message error:(NSError **)error {
     if (!self.dataEncoder) {
         return message;
     }
-    NSError *error = nil;
-    message = [message encodeWithEncoder:self.dataEncoder error:&error];
-    if (error != nil) {
-        [self.logger error:@"ARTChannel: error encoding data: %@", error];
-        [NSException raise:NSInvalidArgumentException format:@"ARTChannel: error encoding data: %@", error];
+    NSError *e = nil;
+    message = [message encodeWithEncoder:self.dataEncoder error:&e];
+    if (e) {
+        [self.logger error:@"ARTChannel: error encoding data: %@", e];
+    }
+    if (error) {
+        *error = e;
     }
     return message;
 }

diff --git a/Source/ARTEncoder.h b/Source/ARTEncoder.h
@@ -28,27 +28,27 @@ ART_ASSUME_NONNULL_BEGIN
 - (ARTEncoderFormat)format;
 - (NSString *)formatAsString;
 
-- (art_nullable NSData *)encodeTokenRequest:(ARTTokenRequest *)request;
-- (art_nullable NSData *)encodeTokenDetails:(ARTTokenDetails *)tokenDetails;
-
-- (art_nullable ARTTokenDetails *)decodeTokenDetails:(NSData *)data error:(NSError * __autoreleasing *)error;
-- (art_nullable ARTTokenRequest *)decodeTokenRequest:(NSData *)data error:(NSError * __autoreleasing *)error;
-- (art_nullable ARTMessage *)decodeMessage:(NSData *)data;
-- (art_nullable NSArray *)decodeMessages:(NSData *)data;
-- (art_nullable NSData *)encodeMessage:(ARTMessage *)message;
-- (art_nullable NSData *)encodeMessages:(NSArray *)messages;
-
-- (art_nullable ARTPresenceMessage *)decodePresenceMessage:(NSData *)data;
-- (art_nullable NSArray *)decodePresenceMessages:(NSData *)data;
-- (art_nullable NSData *)encodePresenceMessage:(ARTPresenceMessage *)message;
-- (art_nullable NSData *)encodePresenceMessages:(NSArray *)messages;
-
-- (art_nullable NSData *)encodeProtocolMessage:(ARTProtocolMessage *)message;
-- (art_nullable ARTProtocolMessage *)decodeProtocolMessage:(NSData *)data;
-
-- (art_nullable NSDate *)decodeTime:(NSData *)data;
-- (art_nullable NSError *)decodeError:(NSData *)error;
-- (art_nullable NSArray *)decodeStats:(NSData *)data;
+- (nullable NSData *)encodeTokenRequest:(ARTTokenRequest *)request error:(NSError *_Nullable *_Nullable)error;
+- (nullable NSData *)encodeTokenDetails:(ARTTokenDetails *)tokenDetails error:(NSError *_Nullable *_Nullable)error;
+
+- (nullable ARTTokenDetails *)decodeTokenDetails:(NSData *)data error:(NSError *_Nullable *_Nullable)error;
+- (nullable ARTTokenRequest *)decodeTokenRequest:(NSData *)data error:(NSError *_Nullable *_Nullable)error;
+- (nullable ARTMessage *)decodeMessage:(NSData *)data error:(NSError *_Nullable *_Nullable)error;
+- (nullable NSArray *)decodeMessages:(NSData *)data error:(NSError *_Nullable *_Nullable)error;
+- (nullable NSData *)encodeMessage:(ARTMessage *)message error:(NSError *_Nullable *_Nullable)error;
+- (nullable NSData *)encodeMessages:(NSArray *)messages error:(NSError *_Nullable *_Nullable)error;
+
+- (nullable ARTPresenceMessage *)decodePresenceMessage:(NSData *)data error:(NSError *_Nullable *_Nullable)error;
+- (nullable NSArray *)decodePresenceMessages:(NSData *)data error:(NSError *_Nullable *_Nullable)error;
+- (nullable NSData *)encodePresenceMessage:(ARTPresenceMessage *)message error:(NSError *_Nullable *_Nullable)error;
+- (nullable NSData *)encodePresenceMessages:(NSArray *)messages error:(NSError *_Nullable *_Nullable)error;
+
+- (nullable NSData *)encodeProtocolMessage:(ARTProtocolMessage *)message error:(NSError *_Nullable *_Nullable)error;
+- (nullable ARTProtocolMessage *)decodeProtocolMessage:(NSData *)data error:(NSError *_Nullable *_Nullable)error;
+
+- (nullable NSDate *)decodeTime:(NSData *)data error:(NSError *_Nullable *_Nullable)error;
+- (nullable NSError *)decodeError:(NSData *)error error:(NSError *_Nullable *_Nullable)error;
+- (nullable NSArray *)decodeStats:(NSData *)data error:(NSError *_Nullable *_Nullable)error;
 
 @end
 

diff --git a/Source/ARTJsonEncoder.m b/Source/ARTJsonEncoder.m
@@ -22,12 +22,20 @@ - (NSString *)formatAsString {
     return @"json";
 }
 
-- (id)decode:(NSData *)data {
-    return [NSJSONSerialization JSONObjectWithData:data options:0 error:nil];
+- (id)decode:(NSData *)data error:(NSError **)error {
+    return [NSJSONSerialization JSONObjectWithData:data options:0 error:error];
 }
 
-- (NSData *)encode:(id)obj {
-    return [NSJSONSerialization dataWithJSONObject:obj options:0 error:nil];
+- (NSData *)encode:(id)obj error:(NSError **)error {
+    @try {
+        return [NSJSONSerialization dataWithJSONObject:obj options:0 error:error];
+    }
+    @catch (NSException *exception) {
+        if (error) {
+            *error = [[NSError alloc] initWithDomain:ARTAblyErrorDomain code:ARTClientCodeErrorInvalidType userInfo:@{NSLocalizedDescriptionKey: exception.reason}];
+        }
+        return nil;
+    }
 }
 
 @end
diff --git a/Source/ARTJsonLikeEncoder.h b/Source/ARTJsonLikeEncoder.h
@@ -21,8 +21,8 @@ ART_ASSUME_NONNULL_BEGIN
 - (ARTEncoderFormat)format;
 - (NSString *)formatAsString;
 
-- (id)decode:(NSData *)data;
-- (NSData *)encode:(id)obj;
+- (nullable id)decode:(NSData *)data error:(NSError * _Nullable __autoreleasing * _Nullable)error;
+- (nullable NSData *)encode:(id)obj error:(NSError * _Nullable __autoreleasing * _Nullable)error;
 
 @end