From 8a1b2ace652900bac1cba89d1f73ae2a72ee6295 Mon Sep 17 00:00:00 2001 From: Cesare Rocchi Date: Fri, 28 Sep 2018 12:08:56 +0200 Subject: [PATCH 1/3] Add nilToEmpty String helper --- Ably.xcodeproj/project.pbxproj | 8 ++++++++ Source/ARTNSString+ARTUtil.h | 15 +++++++++++++++ Source/ARTNSString+ARTUtil.m | 20 ++++++++++++++++++++ 3 files changed, 43 insertions(+) create mode 100644 Source/ARTNSString+ARTUtil.h create mode 100644 Source/ARTNSString+ARTUtil.m diff --git a/Ably.xcodeproj/project.pbxproj b/Ably.xcodeproj/project.pbxproj index 3d4b3b446..acf200570 100644 --- a/Ably.xcodeproj/project.pbxproj +++ b/Ably.xcodeproj/project.pbxproj @@ -64,6 +64,8 @@ 96E408471A3895E800087F77 /* ARTWebSocketTransport.h in Headers */ = {isa = PBXBuildFile; fileRef = 96E408451A3895E800087F77 /* ARTWebSocketTransport.h */; settings = {ATTRIBUTES = (Public, ); }; }; 96E408481A3895E800087F77 /* ARTWebSocketTransport.m in Sources */ = {isa = PBXBuildFile; fileRef = 96E408461A3895E800087F77 /* ARTWebSocketTransport.m */; }; D38D22F220FDFD9900194B40 /* ULID.framework in Copy Carthage Frameworks to Test bundle */ = {isa = PBXBuildFile; fileRef = D77A0C951FFEEF0300711131 /* ULID.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; + D3AD0EBD215E2FB000312105 /* ARTNSString+ARTUtil.h in Headers */ = {isa = PBXBuildFile; fileRef = D3AD0EBB215E2FB000312105 /* ARTNSString+ARTUtil.h */; }; + D3AD0EBE215E2FB000312105 /* ARTNSString+ARTUtil.m in Sources */ = {isa = PBXBuildFile; fileRef = D3AD0EBC215E2FB000312105 /* ARTNSString+ARTUtil.m */; }; D70EAAED1BC3376200CD8B9E /* ARTRestChannel.h in Headers */ = {isa = PBXBuildFile; fileRef = D70EAAEB1BC3376200CD8B9E /* ARTRestChannel.h */; settings = {ATTRIBUTES = (Public, ); }; }; D70EAAEE1BC3376200CD8B9E /* ARTRestChannel.m in Sources */ = {isa = PBXBuildFile; fileRef = D70EAAEC1BC3376200CD8B9E /* ARTRestChannel.m */; }; D70EECAC1FEAF331008A50CD /* ARTPendingMessage.h in Headers */ = {isa = PBXBuildFile; fileRef = D70EECAA1FEAF331008A50CD /* ARTPendingMessage.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -322,6 +324,8 @@ 96E408421A38939E00087F77 /* ARTProtocolMessage.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ARTProtocolMessage.m; sourceTree = ""; }; 96E408451A3895E800087F77 /* ARTWebSocketTransport.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ARTWebSocketTransport.h; sourceTree = ""; }; 96E408461A3895E800087F77 /* ARTWebSocketTransport.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ARTWebSocketTransport.m; sourceTree = ""; }; + D3AD0EBB215E2FB000312105 /* ARTNSString+ARTUtil.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "ARTNSString+ARTUtil.h"; sourceTree = ""; }; + D3AD0EBC215E2FB000312105 /* ARTNSString+ARTUtil.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "ARTNSString+ARTUtil.m"; sourceTree = ""; }; D70EAAEB1BC3376200CD8B9E /* ARTRestChannel.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ARTRestChannel.h; sourceTree = ""; }; D70EAAEC1BC3376200CD8B9E /* ARTRestChannel.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ARTRestChannel.m; sourceTree = ""; }; D70EECAA1FEAF331008A50CD /* ARTPendingMessage.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ARTPendingMessage.h; sourceTree = ""; }; @@ -788,6 +792,8 @@ D75A3F1A1DDE5B62002A4AAD /* ARTGCD.m */, EBD947B31EBB8F160005DD16 /* ARTSentry.h */, EBD947B51EBB8F2B0005DD16 /* ARTSentry.m */, + D3AD0EBB215E2FB000312105 /* ARTNSString+ARTUtil.h */, + D3AD0EBC215E2FB000312105 /* ARTNSString+ARTUtil.m */, ); name = Utilities; sourceTree = ""; @@ -936,6 +942,7 @@ 96E408471A3895E800087F77 /* ARTWebSocketTransport.h in Headers */, EB503C8A1C7F1FE40053AF00 /* ARTLog+Private.h in Headers */, 96E4083F1A3892C700087F77 /* ARTRealtimeTransport.h in Headers */, + D3AD0EBD215E2FB000312105 /* ARTNSString+ARTUtil.h in Headers */, EBF2285B1F4D9AD6009091DD /* ARTWebSocketTransport+Private.h in Headers */, EBFFAC191E97919C003E7326 /* ARTLocalDevice+Private.h in Headers */, D746AE4F1BBD84E7003ECEF8 /* ARTChannelOptions.h in Headers */, @@ -1201,6 +1208,7 @@ D785C42A1E549E33008FEC05 /* ARTPushChannelSubscription.m in Sources */, D7B17EE41C07208B00A6958E /* ARTConnectionDetails.m in Sources */, 96BF61591A35B52C004CF2B3 /* ARTHttp.m in Sources */, + D3AD0EBE215E2FB000312105 /* ARTNSString+ARTUtil.m in Sources */, D73692001DB788C40062C150 /* ARTAuthDetails.m in Sources */, D71966EB1E5E006E000974DD /* ARTPushActivationState.m in Sources */, 1C578E201B3435CA00EF46EC /* ARTFallback.m in Sources */, diff --git a/Source/ARTNSString+ARTUtil.h b/Source/ARTNSString+ARTUtil.h new file mode 100644 index 000000000..b1cd632da --- /dev/null +++ b/Source/ARTNSString+ARTUtil.h @@ -0,0 +1,15 @@ +// +// NSString+ARTNSString.h +// Ably +// +// Created by Cesare Rocchi on 28/09/2018. +// Copyright © 2018 Ably. All rights reserved. +// + +#import + +@interface NSString (ARTUtil) + ++ (NSString *)nilToEmpty:(NSString*)aString; + +@end diff --git a/Source/ARTNSString+ARTUtil.m b/Source/ARTNSString+ARTUtil.m new file mode 100644 index 000000000..598694348 --- /dev/null +++ b/Source/ARTNSString+ARTUtil.m @@ -0,0 +1,20 @@ +// +// NSString+ARTNSString.m +// Ably +// +// Created by Cesare Rocchi on 28/09/2018. +// Copyright © 2018 Ably. All rights reserved. +// + +#import "ARTNSString+ARTUtil.h" + +@implementation NSString (ARTUtil) + ++ (NSString *)nilToEmpty:(NSString*)aString { + if ([aString length] == 0) { + return @""; + } + return aString; +} + +@end From 1344c841f3c3c99de7ff66c2ff9828bc82716cb8 Mon Sep 17 00:00:00 2001 From: Cesare Rocchi Date: Fri, 28 Sep 2018 12:09:59 +0200 Subject: [PATCH 2/3] Prevent merging messages with different clientIDs --- Source/ARTProtocolMessage.m | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Source/ARTProtocolMessage.m b/Source/ARTProtocolMessage.m index 8cbd158ac..e79580e8f 100644 --- a/Source/ARTProtocolMessage.m +++ b/Source/ARTProtocolMessage.m @@ -11,6 +11,7 @@ #import "ARTProtocolMessage+Private.h" #import "ARTStatus.h" #import "ARTConnectionDetails.h" +#import "ARTNSString+ARTUtil.h" @implementation ARTProtocolMessage @@ -89,6 +90,9 @@ - (BOOL)mergeFrom:(ARTProtocolMessage *)other { if ([self mergeWouldExceedMaxSize:self.messages]) { return NO; } + if ([self clientIdsAreDifferent:other.messages]) { + return NO; + } switch (self.action) { case ARTProtocolMessageMessage: @@ -102,6 +106,23 @@ - (BOOL)mergeFrom:(ARTProtocolMessage *)other { } } +- (BOOL)clientIdsAreDifferent:(NSArray*)messages { + NSMutableSet *queuedClientIds = [NSMutableSet new]; + NSMutableSet *incomingClientIds = [NSMutableSet new]; + for (ARTMessage *message in self.messages) { + [queuedClientIds addObject:[NSString nilToEmpty:message.clientId]]; + } + for (ARTMessage *message in messages) { + [incomingClientIds addObject:[NSString nilToEmpty:message.clientId]]; + } + [queuedClientIds unionSet:incomingClientIds]; + if (queuedClientIds.count == 1) { + return NO; + } else { + return YES; + } +} + - (BOOL)mergeWouldExceedMaxSize:(NSArray*)messages { NSInteger queuedMessagesSize = 0; for (ARTMessage *message in self.messages) { From 9f6a369879ecb358c00b24a227bb7ff77c52dafb Mon Sep 17 00:00:00 2001 From: Cesare Rocchi Date: Fri, 28 Sep 2018 12:10:19 +0200 Subject: [PATCH 3/3] Add RTL6d2 tests --- Spec/RealtimeClientChannel.swift | 78 ++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/Spec/RealtimeClientChannel.swift b/Spec/RealtimeClientChannel.swift index 77c6bc7c7..7f72be893 100644 --- a/Spec/RealtimeClientChannel.swift +++ b/Spec/RealtimeClientChannel.swift @@ -2088,6 +2088,84 @@ class RealtimeClientChannel: QuickSpec { let messagesSent = protocolMessages.compactMap{$0.messages?.count}.reduce(0, +) expect(messagesSent).to(equal(messagesToBeSent)) } + + //RTL6d2 + context("Messages with differing clientId values must not be bundled together") { + + it("messages with different (non empty) clientIds are posted via different protocol messages") { + let options = AblyTests.commonAppSetup() + options.autoConnect = false + let client = AblyTests.newRealtime(options) + defer { client.dispose(); client.close() } + let channel = client.channels.get("test-message-bundling-prevention") + let clientIDs = ["client1", "client2", "client3"] + + waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(clientIDs.count, done: done) + for (i, el) in clientIDs.enumerated() { + channel.publish("name\(i)", data: "data\(i)", clientId: el) { error in + expect(error).to(beNil()) + partialDone() + } + } + client.connect() + } + + let transport = client.transport as! TestProxyTransport + let protocolMessages = transport.protocolMessagesSent.filter{ $0.action == .message } + expect(protocolMessages.count).to(equal(clientIDs.count)) + } + + it("messages with mixed empty/non empty clientIds are posted via different protocol messages") { + let options = AblyTests.commonAppSetup() + options.autoConnect = false + let client = AblyTests.newRealtime(options) + defer { client.dispose(); client.close() } + let channel = client.channels.get("test-message-bundling-prevention") + + waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) + channel.publish("name1", data: "data1", clientId: "clientID1") { error in + expect(error).to(beNil()) + partialDone() + } + channel.publish("name2", data: "data2") { error in + expect(error).to(beNil()) + partialDone() + } + client.connect() + } + + let transport = client.transport as! TestProxyTransport + let protocolMessages = transport.protocolMessagesSent.filter{ $0.action == .message } + expect(protocolMessages.count).to(equal(2)) + } + + it("messages bundled by the user are posted in a single protocol message even if they have mixed clientIds") { + let options = AblyTests.commonAppSetup() + options.autoConnect = false + let client = AblyTests.newRealtime(options) + defer { client.dispose(); client.close() } + let channel = client.channels.get("test-message-bundling-prevention") + var messages = [ARTMessage]() + for i in 1...3 { + messages.append(ARTMessage(name: "name\(i)", data: "data\(i)", clientId: "clientId\(i)")) + } + + waitUntil(timeout: testTimeout) { done in + channel.publish(messages) { error in + expect(error).to(beNil()) + done() + } + client.connect() + } + + let transport = client.transport as! TestProxyTransport + let protocolMessages = transport.protocolMessagesSent.filter{ $0.action == .message } + expect(protocolMessages.count).to(equal(1)) + } + + } // RTL6e context("Unidentified clients using Basic Auth") {