Skip to content

Commit 7b301b2

Browse files
robbiehansonrobbiehanson
committed
Fixing thread safety issues (multicast delegate is asynchronous)
1 parent d3788c0 commit 7b301b2

File tree

4 files changed

+172
-70
lines changed

4 files changed

+172
-70
lines changed

‎Core/XMPPJID.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,22 @@ typedef enum XMPPJIDCompareOptions XMPPJIDCompareOptions;
2727
@property (strong, readonly) NSString *domain;
2828
@property (strong, readonly) NSString *resource;
2929

30+
/**
31+
* Terminology (from RFC 6120):
32+
*
33+
* The term "bare JID" refers to an XMPP address of the form <localpart@domainpart> (for an account at a server)
34+
* or of the form <domainpart> (for a server).
35+
*
36+
* The term "full JID" refers to an XMPP address of the form
37+
* <localpart@domainpart/resourcepart> (for a particular authorized client or device associated with an account)
38+
* or of the form <domainpart/resourcepart> (for a particular resource or script associated with a server).
39+
*
40+
* Thus a bareJID is one that does not have a resource.
41+
* And a fullJID is one that does have a resource.
42+
*
43+
* For convenience, there are also methods that that check for a user component as well.
44+
**/
45+
3046
- (XMPPJID *)bareJID;
3147
- (XMPPJID *)domainJID;
3248

@@ -39,8 +55,16 @@ typedef enum XMPPJIDCompareOptions XMPPJIDCompareOptions;
3955
- (BOOL)isFull;
4056
- (BOOL)isFullWithUser;
4157

58+
/**
59+
* A server JID does not have a user component.
60+
**/
4261
- (BOOL)isServer;
4362

63+
/**
64+
* Returns a new jid with the given resource.
65+
**/
66+
- (XMPPJID *)jidWithNewResource:(NSString *)resource;
67+
4468
/**
4569
* When you know both objects are JIDs, this method is a faster way to check equality than isEqual:.
4670
**/

‎Core/XMPPJID.m

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,8 @@ + (XMPPJID *)jidWithString:(NSString *)jidStr
129129

130130
+ (XMPPJID *)jidWithString:(NSString *)jidStr resource:(NSString *)resource
131131
{
132-
if (![self validateResource:resource]) return nil;
132+
NSString *prepResource = [LibIDN prepResource:resource];
133+
if (![self validateResource:prepResource]) return nil;
133134

134135
NSString *user;
135136
NSString *domain;
@@ -139,7 +140,7 @@ + (XMPPJID *)jidWithString:(NSString *)jidStr resource:(NSString *)resource
139140
XMPPJID *jid = [[XMPPJID alloc] init];
140141
jid->user = [user copy];
141142
jid->domain = [domain copy];
142-
jid->resource = [resource copy];
143+
jid->resource = [prepResource copy];
143144

144145
return jid;
145146
}
@@ -166,7 +167,9 @@ + (XMPPJID *)jidWithUser:(NSString *)user domain:(NSString *)domain resource:(NS
166167
return nil;
167168
}
168169

169-
+ (XMPPJID *)jidWithPrevalidatedUser:(NSString *)user domain:(NSString *)domain resource:(NSString *)resource
170+
+ (XMPPJID *)jidWithPrevalidatedUser:(NSString *)user
171+
prevalidatedDomain:(NSString *)domain
172+
prevalidatedResource:(NSString *)resource
170173
{
171174
XMPPJID *jid = [[XMPPJID alloc] init];
172175
jid->user = [user copy];
@@ -176,6 +179,21 @@ + (XMPPJID *)jidWithPrevalidatedUser:(NSString *)user domain:(NSString *)domain
176179
return jid;
177180
}
178181

182+
+ (XMPPJID *)jidWithPrevalidatedUser:(NSString *)user
183+
prevalidatedDomain:(NSString *)domain
184+
resource:(NSString *)resource
185+
{
186+
NSString *prepResource = [LibIDN prepResource:resource];
187+
if (![self validateResource:prepResource]) return nil;
188+
189+
XMPPJID *jid = [[XMPPJID alloc] init];
190+
jid->user = [user copy];
191+
jid->domain = [domain copy];
192+
jid->resource = [prepResource copy];
193+
194+
return jid;
195+
}
196+
179197
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
180198
#pragma mark Encoding, Decoding:
181199
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -272,7 +290,7 @@ - (XMPPJID *)bareJID
272290
}
273291
else
274292
{
275-
return [XMPPJID jidWithPrevalidatedUser:user domain:domain resource:nil];
293+
return [XMPPJID jidWithPrevalidatedUser:user prevalidatedDomain:domain prevalidatedResource:nil];
276294
}
277295
}
278296

@@ -284,7 +302,7 @@ - (XMPPJID *)domainJID
284302
}
285303
else
286304
{
287-
return [XMPPJID jidWithPrevalidatedUser:nil domain:domain resource:nil];
305+
return [XMPPJID jidWithPrevalidatedUser:nil prevalidatedDomain:domain prevalidatedResource:nil];
288306
}
289307
}
290308

@@ -350,6 +368,11 @@ - (BOOL)isServer
350368
return (user == nil);
351369
}
352370

371+
- (XMPPJID *)jidWithNewResource:(NSString *)newResource
372+
{
373+
return [XMPPJID jidWithPrevalidatedUser:user prevalidatedDomain:domain resource:newResource];
374+
}
375+
353376
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
354377
#pragma mark NSObject Methods:
355378
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -363,7 +386,7 @@ - (BOOL)isEqual:(id)anObject
363386
{
364387
if ([anObject isMemberOfClass:[self class]])
365388
{
366-
return [self isEqualToJID:(XMPPJID *)anObject];
389+
return [self isEqualToJID:(XMPPJID *)anObject options:XMPPJIDCompareFull];
367390
}
368391
return NO;
369392
}

‎Core/XMPPStream.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -793,9 +793,11 @@ typedef enum XMPPStreamErrorCode XMPPStreamErrorCode;
793793
- (void)xmppStream:(XMPPStream *)sender didNotAuthenticate:(NSXMLElement *)error;
794794

795795
/**
796-
* This method is called if the XMPP server doesn't allow are resource of choice because it conflicts with an existing resource.
797-
* Return an alternative resource or return nil to let the server pick a resource.
798-
**/
796+
* This method is called if the XMPP server doesn't allow our resource of choice
797+
* because it conflicts with an existing resource.
798+
*
799+
* Return an alternative resource or return nil to let the server automatically pick a resource for us.
800+
**/
799801
- (NSString *)xmppStream:(XMPPStream *)sender alternativeResourceForConflictingResource:(NSString *)conflictingResource;
800802

801803
/**

‎Core/XMPPStream.m

Lines changed: 114 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ - (void)continueSendElement:(NSXMLElement *)element withTag:(long)tag;
7272
- (void)startNegotiation;
7373
- (void)sendOpeningNegotiation;
7474
- (void)continueStartTLS:(NSMutableDictionary *)settings;
75+
- (void)continueHandleBinding:(NSString *)alternativeResource;
7576
- (void)setupKeepAliveTimer;
7677
- (void)keepAlive;
7778

@@ -2551,69 +2552,121 @@ - (void)handleBinding:(NSXMLElement *)response
25512552
}
25522553
else
25532554
{
2554-
// It appears the server didn't allow our resource choice
2555-
// First check if we want to try an alternative resource
2556-
2557-
NSString *alternativeResource = nil;
2558-
2559-
NSXMLElement *r_error = [response elementForName:@"error"];
2560-
NSXMLElement *r_conflict = [r_error elementForName:@"conflict" xmlns:@"urn:ietf:params:xml:ns:xmpp-stanzas"];
2561-
2562-
if(r_conflict)
2563-
{
2564-
alternativeResource = [multicastDelegate xmppStream:self alternativeResourceForConflictingResource:[[self myJID] resource]];
2565-
}
2555+
// It appears the server didn't allow our resource choice
2556+
// First check if we want to try an alternative resource
2557+
2558+
NSXMLElement *r_error = [response elementForName:@"error"];
2559+
NSXMLElement *r_conflict = [r_error elementForName:@"conflict" xmlns:@"urn:ietf:params:xml:ns:xmpp-stanzas"];
25662560

2567-
if([alternativeResource length])
2568-
{
2569-
XMPPJID *alternativeJID = [XMPPJID jidWithUser:[[self myJID] user] domain:[[self myJID] domain] resource:alternativeResource];
2570-
[self setMyJID:alternativeJID];
2571-
2572-
NSXMLElement *resource = [NSXMLElement elementWithName:@"resource"];
2573-
[resource setStringValue:alternativeResource];
2574-
2575-
NSXMLElement *bind = [NSXMLElement elementWithName:@"bind" xmlns:@"urn:ietf:params:xml:ns:xmpp-bind"];
2576-
[bind addChild:resource];
2577-
2578-
NSXMLElement *iq = [NSXMLElement elementWithName:@"iq"];
2579-
[iq addAttributeWithName:@"type" stringValue:@"set"];
2580-
[iq addChild:bind];
2581-
2582-
NSString *outgoingStr = [iq compactXMLString];
2583-
NSData *outgoingData = [outgoingStr dataUsingEncoding:NSUTF8StringEncoding];
2584-
2585-
XMPPLogSend(@"SEND: %@", outgoingStr);
2586-
numberOfBytesSent += [outgoingData length];
2587-
2588-
[asyncSocket writeData:outgoingData
2589-
withTimeout:TIMEOUT_XMPP_WRITE
2590-
tag:TAG_XMPP_WRITE_STREAM];
2591-
2592-
// The state remains in STATE_XMPP_BINDING
2593-
2594-
}else{
2595-
2596-
// We'll simply let the server choose then
2597-
2598-
NSXMLElement *bind = [NSXMLElement elementWithName:@"bind" xmlns:@"urn:ietf:params:xml:ns:xmpp-bind"];
2599-
2600-
NSXMLElement *iq = [NSXMLElement elementWithName:@"iq"];
2601-
[iq addAttributeWithName:@"type" stringValue:@"set"];
2602-
[iq addChild:bind];
2603-
2604-
NSString *outgoingStr = [iq compactXMLString];
2605-
NSData *outgoingData = [outgoingStr dataUsingEncoding:NSUTF8StringEncoding];
2606-
2607-
XMPPLogSend(@"SEND: %@", outgoingStr);
2608-
numberOfBytesSent += [outgoingData length];
2609-
2610-
[asyncSocket writeData:outgoingData
2611-
withTimeout:TIMEOUT_XMPP_WRITE
2612-
tag:TAG_XMPP_WRITE_STREAM];
2613-
2614-
// The state remains in STATE_XMPP_BINDING
2615-
2561+
if (r_conflict)
2562+
{
2563+
SEL selector = @selector(xmppStream:alternativeResourceForConflictingResource:);
2564+
2565+
if ([multicastDelegate countForSelector:selector] == 0)
2566+
{
2567+
// None of the delegates implement the method.
2568+
// Use a shortcut.
2569+
2570+
[self continueHandleBinding:nil];
2571+
}
2572+
else
2573+
{
2574+
// Query all interested delegates.
2575+
// This must be done serially to maintain thread safety.
2576+
2577+
GCDMulticastDelegateEnumerator *delegateEnumerator = [multicastDelegate delegateEnumerator];
2578+
2579+
dispatch_queue_t concurrentQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
2580+
dispatch_async(concurrentQueue, ^{ @autoreleasepool {
2581+
2582+
// Query delegates for alternative resource
2583+
2584+
NSString *currentResource = [[self myJID] resource];
2585+
__block NSString *alternativeResource = nil;
2586+
2587+
id delegate;
2588+
dispatch_queue_t dq;
2589+
2590+
while ([delegateEnumerator getNextDelegate:&delegate delegateQueue:&dq forSelector:selector])
2591+
{
2592+
dispatch_sync(dq, ^{ @autoreleasepool {
2593+
2594+
NSString *delegateAlternativeResource =
2595+
[delegate xmppStream:self alternativeResourceForConflictingResource:currentResource];
2596+
2597+
if (delegateAlternativeResource)
2598+
{
2599+
alternativeResource = delegateAlternativeResource;
2600+
}
2601+
}});
2602+
}
2603+
2604+
dispatch_async(xmppQueue, ^{ @autoreleasepool {
2605+
2606+
[self continueHandleBinding:alternativeResource];
2607+
2608+
}});
2609+
2610+
}});
2611+
}
26162612
}
2613+
else
2614+
{
2615+
// Appears to be a conflicting resource, but server didn't specify conflict
2616+
[self continueHandleBinding:nil];
2617+
}
2618+
}
2619+
}
2620+
2621+
- (void)continueHandleBinding:(NSString *)alternativeResource
2622+
{
2623+
if ([alternativeResource length] > 0)
2624+
{
2625+
// Update myJID
2626+
2627+
[self setMyJID_setByClient:[myJID_setByClient jidWithNewResource:alternativeResource]];
2628+
2629+
NSXMLElement *resource = [NSXMLElement elementWithName:@"resource"];
2630+
[resource setStringValue:alternativeResource];
2631+
2632+
NSXMLElement *bind = [NSXMLElement elementWithName:@"bind" xmlns:@"urn:ietf:params:xml:ns:xmpp-bind"];
2633+
[bind addChild:resource];
2634+
2635+
XMPPIQ *iq = [XMPPIQ iqWithType:@"set"];
2636+
[iq addChild:bind];
2637+
2638+
NSString *outgoingStr = [iq compactXMLString];
2639+
NSData *outgoingData = [outgoingStr dataUsingEncoding:NSUTF8StringEncoding];
2640+
2641+
XMPPLogSend(@"SEND: %@", outgoingStr);
2642+
numberOfBytesSent += [outgoingData length];
2643+
2644+
[asyncSocket writeData:outgoingData
2645+
withTimeout:TIMEOUT_XMPP_WRITE
2646+
tag:TAG_XMPP_WRITE_STREAM];
2647+
2648+
// The state remains in STATE_XMPP_BINDING
2649+
}
2650+
else
2651+
{
2652+
// We'll simply let the server choose then
2653+
2654+
NSXMLElement *bind = [NSXMLElement elementWithName:@"bind" xmlns:@"urn:ietf:params:xml:ns:xmpp-bind"];
2655+
2656+
XMPPIQ *iq = [XMPPIQ iqWithType:@"set"];
2657+
[iq addChild:bind];
2658+
2659+
NSString *outgoingStr = [iq compactXMLString];
2660+
NSData *outgoingData = [outgoingStr dataUsingEncoding:NSUTF8StringEncoding];
2661+
2662+
XMPPLogSend(@"SEND: %@", outgoingStr);
2663+
numberOfBytesSent += [outgoingData length];
2664+
2665+
[asyncSocket writeData:outgoingData
2666+
withTimeout:TIMEOUT_XMPP_WRITE
2667+
tag:TAG_XMPP_WRITE_STREAM];
2668+
2669+
// The state remains in STATE_XMPP_BINDING
26172670
}
26182671
}
26192672

0 commit comments

Comments
 (0)