From 6cee7c7b44ea4b223418fb74a4285eab1989fd4f Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 22 Oct 2020 19:21:01 +0100 Subject: [PATCH 1/6] Fix weird typos --- Spec/Auth.swift | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/Spec/Auth.swift b/Spec/Auth.swift index 4d011fc0d..c407168d1 100644 --- a/Spec/Auth.swift +++ b/Spec/Auth.swift @@ -1103,7 +1103,7 @@ class Auth : QuickSpec { // RSA7 context("clientId and authenticated clients") { - // RAS7a1 + // RSA7a1 it("should use assigned clientId on all operations") { let expectedClientId = "client_string" let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) @@ -1124,16 +1124,21 @@ class Auth : QuickSpec { switch extractBodyAsMsgPack(testHTTPExecutor.requests.last) { case .failure(let error): - XCTFail(error) + XCTFail(error) case .success(let httpBody): - guard let requestedClientId = httpBody.unbox["clientId"] as? String else { XCTFail("No clientId field in .tTPBody"); return } + guard + let requestedClientId = httpBody.unbox["clientId"] as? String + else { + XCTFail("No clientId field in HTTPBody") + return + } expect(requestedClientId).to(equal(expectedClientId)) } - // .oDO: add more operations + // TODO: add more operations } - // .sA7a2 + // RSA7a2 it("should obtain a token if clientId is assigned") { let options = AblyTests.setupOptions(AblyTests.jsonRestOptions) options.clientId = "client_string" From a6ef712794984ecefe39b507d02d183593c74b57 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 22 Oct 2020 19:29:12 +0100 Subject: [PATCH 2/6] Test that reproduces #1074 --- Spec/RestClientChannel.swift | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Spec/RestClientChannel.swift b/Spec/RestClientChannel.swift index f295f6609..086f2129e 100644 --- a/Spec/RestClientChannel.swift +++ b/Spec/RestClientChannel.swift @@ -319,6 +319,34 @@ class RestClientChannel: QuickSpec { } + // https://github.com/ably/ably-cocoa/issues/1074 and related with RSL1m + fit("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() From 51b308f4fbb6b5adec73477144c451b7826e1e75 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 22 Oct 2020 19:35:57 +0100 Subject: [PATCH 3/6] Logger: if the body is MsgPack encoded then it should not show nil but Base64 encoded string instead. --- Source/ARTHttp.m | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Source/ARTHttp.m b/Source/ARTHttp.m index e501ae40f..a24d078c2 100644 --- a/Source/ARTHttp.m +++ b/Source/ARTHttp.m @@ -47,14 +47,22 @@ - (void)dealloc { } - (NSObject *)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]; + NSString *requestBodyStr = [[NSString alloc] initWithData:request.HTTPBody encoding:NSUTF8StringEncoding]; + if (!requestBodyStr) { + requestBodyStr = [request.HTTPBody base64EncodedStringWithOptions:NSDataBase64EncodingEndLineWithCarriageReturn]; + } + [self.logger debug:@"--> %@ %@\n Body: %@\n Headers: %@", request.HTTPMethod, request.URL.absoluteString, requestBodyStr, 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]; + NSString *responseDataStr = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; + if (!responseDataStr) { + responseDataStr = [data base64EncodedStringWithOptions:NSDataBase64EncodingEndLineWithCarriageReturn]; + } + [self.logger debug:@"<-- %@ %@: statusCode %ld\n Data: %@\n Headers: %@\n", request.HTTPMethod, request.URL.absoluteString, (long)httpResponse.statusCode, responseDataStr, httpResponse.allHeaderFields]; NSString *headerErrorMessage = httpResponse.allHeaderFields[ARTHttpHeaderFieldErrorMessageKey]; if (headerErrorMessage && ![headerErrorMessage isEqualToString:@""]) { [self.logger warn:@"%@", headerErrorMessage]; From 6d4fe595088fbf26a5132082b9bb97c350dc0f75 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 22 Oct 2020 19:36:22 +0100 Subject: [PATCH 4/6] Fix #1074 --- Source/ARTRestChannel.m | 3 --- Spec/RestClientChannel.swift | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/Source/ARTRestChannel.m b/Source/ARTRestChannel.m index 6f0f60680..970713e32 100644 --- a/Source/ARTRestChannel.m +++ b/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]; diff --git a/Spec/RestClientChannel.swift b/Spec/RestClientChannel.swift index 086f2129e..2b71a1607 100644 --- a/Spec/RestClientChannel.swift +++ b/Spec/RestClientChannel.swift @@ -320,7 +320,7 @@ class RestClientChannel: QuickSpec { } // https://github.com/ably/ably-cocoa/issues/1074 and related with RSL1m - fit("should not fail sending a message with no clientId in the client options and credentials that can assume any clientId") { + 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 From a2852026418373f630d25cb357f036ee76886cc7 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Fri, 23 Oct 2020 09:14:50 +0100 Subject: [PATCH 5/6] Update RSA7a1 --- Spec/Auth.swift | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/Spec/Auth.swift b/Spec/Auth.swift index c407168d1..e2734a626 100644 --- a/Spec/Auth.swift +++ b/Spec/Auth.swift @@ -1103,39 +1103,29 @@ class Auth : QuickSpec { // RSA7 context("clientId and authenticated clients") { - // RSA7a1 - 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 HTTPBody") - return - } - expect(requestedClientId).to(equal(expectedClientId)) + let message = httpBody.unbox + expect(message["clientId"]).to(beNil()) + expect(message["name"] as? String).to(equal("foo")) } - - // TODO: add more operations } // RSA7a2 From 4b4d4179115c617b12d10a5f70b6a20b9256e166 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 29 Oct 2020 14:57:40 +0000 Subject: [PATCH 6/6] fixup! Logger: if the body is MsgPack encoded then it should not show nil but Base64 encoded string instead. --- Source/ARTHttp.m | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/Source/ARTHttp.m b/Source/ARTHttp.m index a24d078c2..8cfa1c7dd 100644 --- a/Source/ARTHttp.m +++ b/Source/ARTHttp.m @@ -47,22 +47,14 @@ - (void)dealloc { } - (NSObject *)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTTPURLResponse *_Nullable, NSData *_Nullable, NSError *_Nullable))callback { - NSString *requestBodyStr = [[NSString alloc] initWithData:request.HTTPBody encoding:NSUTF8StringEncoding]; - if (!requestBodyStr) { - requestBodyStr = [request.HTTPBody base64EncodedStringWithOptions:NSDataBase64EncodingEndLineWithCarriageReturn]; - } - [self.logger debug:@"--> %@ %@\n Body: %@\n Headers: %@", request.HTTPMethod, request.URL.absoluteString, requestBodyStr, 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 { - NSString *responseDataStr = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; - if (!responseDataStr) { - responseDataStr = [data base64EncodedStringWithOptions:NSDataBase64EncodingEndLineWithCarriageReturn]; - } - [self.logger debug:@"<-- %@ %@: statusCode %ld\n Data: %@\n Headers: %@\n", request.HTTPMethod, request.URL.absoluteString, (long)httpResponse.statusCode, responseDataStr, 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]; @@ -72,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