Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions Source/ARTHttp.m
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,22 @@ - (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];
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];
QuintinWillison marked this conversation as resolved.
Show resolved Hide resolved

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];
Expand Down
3 changes: 0 additions & 3 deletions Source/ARTRestChannel.m
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
QuintinWillison marked this conversation as resolved.
Show resolved Hide resolved
message.clientId = self.rest.auth.clientId_nosync;
}

NSError *encodeError = nil;
encodedMessage = [self.rest.defaultEncoder encodeMessage:message error:&encodeError];
Expand Down
33 changes: 14 additions & 19 deletions Spec/Auth.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
28 changes: 28 additions & 0 deletions Spec/RestClientChannel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down