Merge pull request #1082 from ably/fix-1074

Fixes 40012 "Malformed message; invalid clientId" when message has no clientId and credentials can assume any clientId

Source/ARTHttp.m

@@ -47,14 +47,14 @@ - (void)dealloc {
 }
 
 - (NSObject<ARTCancellable> *)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTTPURLResponse *_Nullable, NSData *_Nullable, NSError *_Nullable))callback {
-    [self.logger debug:@"--> %@ %@\n  Body: %@\n  Headers: %@", request.HTTPMethod, request.URL.absoluteString, [[NSString alloc] initWithData:request.HTTPBody encoding:NSUTF8StringEncoding], request.allHTTPHeaderFields];
+    [self.logger debug:@"--> %@ %@\n  Body: %@\n  Headers: %@", request.HTTPMethod, request.URL.absoluteString, [self debugDescriptionOfBodyWithData:request.HTTPBody], request.allHTTPHeaderFields];
 
     return [_urlSession get:request completion:^(NSHTTPURLResponse *response, NSData *data, NSError *error) {
         NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response;
         if (error) {
             [self.logger error:@"<-- %@ %@: error %@", request.HTTPMethod, request.URL.absoluteString, error];
         } else {
-            [self.logger debug:@"<-- %@ %@: statusCode %ld\n  Data: %@\n  Headers: %@\n", request.HTTPMethod, request.URL.absoluteString, (long)httpResponse.statusCode, [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding], httpResponse.allHeaderFields];
+            [self.logger debug:@"<-- %@ %@: statusCode %ld\n  Data: %@\n  Headers: %@\n", request.HTTPMethod, request.URL.absoluteString, (long)httpResponse.statusCode, [self debugDescriptionOfBodyWithData:data], httpResponse.allHeaderFields];
             NSString *headerErrorMessage = httpResponse.allHeaderFields[ARTHttpHeaderFieldErrorMessageKey];
             if (headerErrorMessage && ![headerErrorMessage isEqualToString:@""]) {
                 [self.logger warn:@"%@", headerErrorMessage];
@@ -64,4 +64,15 @@ - (void)dealloc {
     }];
 }
 
+- (NSString *)debugDescriptionOfBodyWithData:(NSData *)data {
+    if (self.logger.logLevel <= ARTLogLevelDebug) {
+        NSString *requestBodyStr = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding];
+        if (!requestBodyStr) {
+            requestBodyStr = [data base64EncodedStringWithOptions:NSDataBase64EncodingEndLineWithCarriageReturn];
+        }
+        return requestBodyStr;
+    }
+    return nil;
+}
+
 @end

Source/ARTRestChannel.m

@@ -247,9 +247,6 @@ - (void)internalPostMessages:(id)data callback:(void (^)(ARTErrorInfo *__art_nul
             callback([ARTErrorInfo createWithCode:ARTStateMismatchedClientId message:@"attempted to publish message with an invalid clientId"]);
             return;
         }
-        else {
-            message.clientId = self.rest.auth.clientId_nosync;
-        }
 
         NSError *encodeError = nil;
         encodedMessage = [self.rest.defaultEncoder encodeMessage:message error:&encodeError];

Spec/Auth.swift

@@ -1103,37 +1103,32 @@ class Auth : QuickSpec {
 
             // RSA7
             context("clientId and authenticated clients") {
-                // RAS7a1
-                it("should use assigned clientId on all operations") {
-                    let expectedClientId = "client_string"
-                    let options = AblyTests.setupOptions(AblyTests.jsonRestOptions)
-                    options.clientId = expectedClientId
 
-                    let client = ARTRest(options: options)
+                // RSA7a1
+                it("should not pass clientId with published message") {
+                    let options = AblyTests.commonAppSetup()
+                    options.clientId = "mary"
+                    let rest = ARTRest(options: options)
                     testHTTPExecutor = TestProxyHTTPExecutor(options.logHandler)
-                    client.internal.httpExecutor = testHTTPExecutor
-
+                    rest.internal.httpExecutor = testHTTPExecutor
+                    let channel = rest.channels.get("RSA7a1")
                     waitUntil(timeout: testTimeout) { done in
-                        client.channels.get("test").publish(nil, data: "message") { error in
-                            if let e = error {
-                                XCTFail((e ).description)
-                            }
+                        channel.publish("foo", data: nil) { error in
+                            expect(error).to(beNil())
                             done()
                         }
                     }
-
                     switch extractBodyAsMsgPack(testHTTPExecutor.requests.last) {
                     case .failure(let error):
-              XCTFail(error)
+                        fail(error)
                     case .success(let httpBody):
-                        guard let requestedClientId = httpBody.unbox["clientId"] as? String else { XCTFail("No clientId field in .tTPBody"); return }
-                        expect(requestedClientId).to(equal(expectedClientId))
+                        let message = httpBody.unbox
+                        expect(message["clientId"]).to(beNil())
+                        expect(message["name"] as? String).to(equal("foo"))
                     }
-
-                    // .oDO: add more operations
                 }
 
-                // .sA7a2
+                // RSA7a2
                 it("should obtain a token if clientId is assigned") {
                     let options = AblyTests.setupOptions(AblyTests.jsonRestOptions)
                     options.clientId = "client_string"

Spec/RestClientChannel.swift

@@ -319,6 +319,34 @@ class RestClientChannel: QuickSpec {
 
             }
 
+            // https://github.com/ably/ably-cocoa/issues/1074 and related with RSL1m
+            it("should not fail sending a message with no clientId in the client options and credentials that can assume any clientId") {
+                let options = AblyTests.clientOptions()
+                options.authCallback = { _, callback in
+                    getTestTokenDetails(clientId: "*") { token, error in
+                        callback(token, error)
+                    }
+                }
+
+                let rest = ARTRest(options: options)
+                let channel = rest.channels.get("#1074")
+
+                waitUntil(timeout: testTimeout) { done in
+                    // The first attempt encodes the message before requesting auth credentials so there's no clientId
+                    channel.publish("first message", data: nil) { error in
+                        expect(error).to(beNil())
+                        done()
+                    }
+                }
+
+                waitUntil(timeout: testTimeout) { done in
+                    channel.publish("second message", data: nil) { error in
+                        expect(error).to(beNil())
+                        done()
+                    }
+                }
+            }
+
             // RSL1h
             it("should provide an optional argument that allows the clientId value to be specified") {
                 let options = AblyTests.commonAppSetup()