From 043218fb9166baeba59717e8e5fc1c7662155eab Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 4 May 2018 14:40:01 -0400 Subject: [PATCH] Ensure thread safety in phone parsing logic. --- SignalServiceKit/src/Contacts/PhoneNumber.m | 10 ++-- .../src/Contacts/PhoneNumberUtil.h | 10 ++-- .../src/Contacts/PhoneNumberUtil.m | 52 ++++++++++++------- 3 files changed, 44 insertions(+), 28 deletions(-) diff --git a/SignalServiceKit/src/Contacts/PhoneNumber.m b/SignalServiceKit/src/Contacts/PhoneNumber.m index 02982379f..a22b84595 100644 --- a/SignalServiceKit/src/Contacts/PhoneNumber.m +++ b/SignalServiceKit/src/Contacts/PhoneNumber.m @@ -40,7 +40,7 @@ static NSString *const RPDefaultsKeyPhoneNumberCanonical = @"RPDefaultsKeyPhoneN OWSAssert(text != nil); OWSAssert(regionCode != nil); - PhoneNumberUtil *phoneUtil = [PhoneNumberUtil sharedUtil]; + PhoneNumberUtil *phoneUtil = [PhoneNumberUtil sharedThreadLocal]; NSError *parseError = nil; NBPhoneNumber *number = [phoneUtil parse:text defaultRegion:regionCode error:&parseError]; @@ -71,7 +71,7 @@ static NSString *const RPDefaultsKeyPhoneNumberCanonical = @"RPDefaultsKeyPhoneN NSString *_Nullable countryCode = nil; #if TARGET_OS_IPHONE - countryCode = [[PhoneNumberUtil sharedUtil].nbPhoneNumberUtil countryCodeByCarrier]; + countryCode = [[PhoneNumberUtil sharedThreadLocal].nbPhoneNumberUtil countryCodeByCarrier]; if ([countryCode isEqualToString:@"ZZ"]) { countryCode = [locale objectForKey:NSLocaleCountryCode]; @@ -164,7 +164,7 @@ static NSString *const RPDefaultsKeyPhoneNumberCanonical = @"RPDefaultsKeyPhoneN } + (NSString *)regionCodeFromCountryCodeString:(NSString *)countryCodeString { - NBPhoneNumberUtil *phoneUtil = [PhoneNumberUtil sharedUtil].nbPhoneNumberUtil; + NBPhoneNumberUtil *phoneUtil = [PhoneNumberUtil sharedThreadLocal].nbPhoneNumberUtil; NSString *regionCode = [phoneUtil getRegionCodeForCountryCode:@([[countryCodeString substringFromIndex:1] integerValue])]; return regionCode; @@ -381,11 +381,11 @@ static NSString *const RPDefaultsKeyPhoneNumberCanonical = @"RPDefaultsKeyPhoneN } - (BOOL)isValid { - return [[PhoneNumberUtil sharedUtil].nbPhoneNumberUtil isValidNumber:self.phoneNumber]; + return [[PhoneNumberUtil sharedThreadLocal].nbPhoneNumberUtil isValidNumber:self.phoneNumber]; } - (NSString *)localizedDescriptionForUser { - NBPhoneNumberUtil *phoneUtil = [PhoneNumberUtil sharedUtil].nbPhoneNumberUtil; + NBPhoneNumberUtil *phoneUtil = [PhoneNumberUtil sharedThreadLocal].nbPhoneNumberUtil; NSError *formatError = nil; NSString *pretty = diff --git a/SignalServiceKit/src/Contacts/PhoneNumberUtil.h b/SignalServiceKit/src/Contacts/PhoneNumberUtil.h index b4410d481..b6ecd5050 100644 --- a/SignalServiceKit/src/Contacts/PhoneNumberUtil.h +++ b/SignalServiceKit/src/Contacts/PhoneNumberUtil.h @@ -1,14 +1,18 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // -#import #import "PhoneNumber.h" +#import @interface PhoneNumberUtil : NSObject @property (nonatomic, retain) NBPhoneNumberUtil *nbPhoneNumberUtil; +- (instancetype)init NS_UNAVAILABLE; + ++ (instancetype)sharedThreadLocal; + + (BOOL)name:(NSString *)nameString matchesQuery:(NSString *)queryString; + (NSString *)callingCodeFromCountryCode:(NSString *)countryCode; @@ -28,8 +32,6 @@ + (NSString *)examplePhoneNumberForCountryCode:(NSString *)countryCode; -+ (instancetype)sharedUtil; - - (NBPhoneNumber *)parse:(NSString *)numberToParse defaultRegion:(NSString *)defaultRegion error:(NSError **)error; - (NSString *)format:(NBPhoneNumber *)phoneNumber numberFormat:(NBEPhoneNumberFormat)numberFormat diff --git a/SignalServiceKit/src/Contacts/PhoneNumberUtil.m b/SignalServiceKit/src/Contacts/PhoneNumberUtil.m index ca1f715a2..3f4e324ef 100644 --- a/SignalServiceKit/src/Contacts/PhoneNumberUtil.m +++ b/SignalServiceKit/src/Contacts/PhoneNumberUtil.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "PhoneNumberUtil.h" @@ -18,13 +18,28 @@ @implementation PhoneNumberUtil -+ (instancetype)sharedUtil { ++ (NSObject *)sharedLock +{ static dispatch_once_t onceToken; - static id sharedInstance = nil; + static NSObject *lock = nil; dispatch_once(&onceToken, ^{ - sharedInstance = [self.class new]; + lock = [NSObject new]; }); - return sharedInstance; + return lock; +} + ++ (PhoneNumberUtil *)sharedThreadLocal +{ + @synchronized(self.sharedLock) + { + NSString *key = PhoneNumberUtil.logTag; + PhoneNumberUtil *_Nullable threadLocal = NSThread.currentThread.threadDictionary[key]; + if (!threadLocal) { + threadLocal = [PhoneNumberUtil new]; + NSThread.currentThread.threadDictionary[key] = threadLocal; + } + return threadLocal; + } } - (instancetype)init { @@ -138,9 +153,10 @@ return @"+1"; } - NSString *callingCode = [NSString stringWithFormat:@"%@%@", - COUNTRY_CODE_PREFIX, - [[[self sharedUtil] nbPhoneNumberUtil] getCountryCodeForRegion:countryCode]]; + NSString *callingCode = + [NSString stringWithFormat:@"%@%@", + COUNTRY_CODE_PREFIX, + [[[self sharedThreadLocal] nbPhoneNumberUtil] getCountryCodeForRegion:countryCode]]; return callingCode; } @@ -578,26 +594,24 @@ + (NSString *)examplePhoneNumberForCountryCode:(NSString *)countryCode { + PhoneNumberUtil *sharedUtil = [self sharedThreadLocal]; + // Signal users are very likely using mobile devices, so prefer that kind of example. NSError *error; NBPhoneNumber *nbPhoneNumber = - [[[self sharedUtil] nbPhoneNumberUtil] getExampleNumberForType:countryCode - type:NBEPhoneNumberTypeMOBILE - error:&error]; + [sharedUtil.nbPhoneNumberUtil getExampleNumberForType:countryCode type:NBEPhoneNumberTypeMOBILE error:&error]; OWSAssert(!error); if (!nbPhoneNumber) { // For countries that with similar mobile and land lines, use "line or mobile" // examples. - nbPhoneNumber = - [[[self sharedUtil] nbPhoneNumberUtil] getExampleNumberForType:countryCode - type:NBEPhoneNumberTypeFIXED_LINE_OR_MOBILE - error:&error]; + nbPhoneNumber = [sharedUtil.nbPhoneNumberUtil getExampleNumberForType:countryCode + type:NBEPhoneNumberTypeFIXED_LINE_OR_MOBILE + error:&error]; OWSAssert(!error); } - NSString *result = (nbPhoneNumber ? [[[self sharedUtil] nbPhoneNumberUtil] format:nbPhoneNumber - numberFormat:NBEPhoneNumberFormatE164 - error:&error] - : nil); + NSString *result = (nbPhoneNumber + ? [sharedUtil.nbPhoneNumberUtil format:nbPhoneNumber numberFormat:NBEPhoneNumberFormatE164 error:&error] + : nil); OWSAssert(!error); return result; }