From 3a5253eb129fa712f6962d0b8dc2b680b4e616c3 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 31 Jul 2015 23:48:56 -0700 Subject: Move _channel from GRPCCall into GRPCWrappedCall --- .../GRPCClient/private/GRPCWrappedCall.h | 7 ++++--- .../GRPCClient/private/GRPCWrappedCall.m | 23 +++++++++++++++------- 2 files changed, 20 insertions(+), 10 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h index 18f8bb5531..da11cbb761 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h @@ -81,11 +81,12 @@ @end +#pragma mark GRPCWrappedCall + @interface GRPCWrappedCall : NSObject -- (instancetype)initWithChannel:(GRPCChannel *)channel - path:(NSString *)path - host:(NSString *)host NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithHost:(NSString *)host + path:(NSString *)path NS_DESIGNATED_INITIALIZER; - (void)startBatchWithOperations:(NSArray *)ops errorHandler:(void(^)())errorHandler; diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m index 1db63df77f..4681994bb2 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m @@ -32,10 +32,13 @@ */ #import "GRPCWrappedCall.h" + #import #include #include #include + +#import "GRPCChannel.h" #import "GRPCCompletionQueue.h" #import "NSDictionary+GRPC.h" #import "NSData+GRPC.h" @@ -219,21 +222,23 @@ @end +#pragma mark GRPCWrappedCall + @implementation GRPCWrappedCall{ + GRPCChannel *_channel; grpc_call *_call; GRPCCompletionQueue *_queue; } - (instancetype)init { - return [self initWithChannel:nil path:nil host:nil]; + return [self initWithHost:nil path:nil]; } -- (instancetype)initWithChannel:(GRPCChannel *)channel - path:(NSString *)path - host:(NSString *)host { - if (!channel || !path || !host) { +- (instancetype)initWithHost:(NSString *)host + path:(NSString *)path { + if (!path || !host) { [NSException raise:NSInvalidArgumentException - format:@"channel, method, and host cannot be nil."]; + format:@"path and host cannot be nil."]; } if (self = [super init]) { @@ -246,7 +251,11 @@ if (!_queue) { return nil; } - _call = grpc_channel_create_call(channel.unmanagedChannel, + _channel = [GRPCChannel channelToHost:host]; + if (!_channel) { + return nil; + } + _call = grpc_channel_create_call(_channel.unmanagedChannel, _queue.unmanagedQueue, path.UTF8String, host.UTF8String, -- cgit v1.2.3 From bd993df3f6eeaea7b032b272703c5fc41beeebef Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Sat, 1 Aug 2015 02:51:22 -0700 Subject: Encapsulate grpc_call creation inside GRPCChannel --- src/objective-c/GRPCClient/private/GRPCChannel.h | 14 +++++------ src/objective-c/GRPCClient/private/GRPCChannel.m | 28 ++++++++++++++++++++-- .../GRPCClient/private/GRPCSecureChannel.m | 3 ++- .../GRPCClient/private/GRPCUnsecuredChannel.m | 3 ++- .../GRPCClient/private/GRPCWrappedCall.m | 21 ++++------------ 5 files changed, 41 insertions(+), 28 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.h b/src/objective-c/GRPCClient/private/GRPCChannel.h index bc6a47d469..49f5bfcc18 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCChannel.h @@ -33,19 +33,19 @@ #import -struct grpc_channel; +#include -// Each separate instance of this class represents at least one TCP -// connection to the provided host. To create a grpc_call, pass the -// value of the unmanagedChannel property to grpc_channel_create_call. -// Release this object when the call is finished. +// Each separate instance of this class represents at least one TCP connection to the provided host. +// To create a grpc_call to that host, use |unmanagedCallWithPath|. Release this object when the +// call is finished. @interface GRPCChannel : NSObject -@property(nonatomic, readonly) struct grpc_channel *unmanagedChannel; // Convenience constructor to allow for reuse of connections. + (instancetype)channelToHost:(NSString *)host; - (instancetype)initWithHost:(NSString *)host NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel + hostName:(NSString *)hostName NS_DESIGNATED_INITIALIZER; -- (instancetype)initWithChannel:(struct grpc_channel *)unmanagedChannel NS_DESIGNATED_INITIALIZER; +- (grpc_call *)unmanagedCallWithPath:(NSString *)path; @end diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.m b/src/objective-c/GRPCClient/private/GRPCChannel.m index af4326332f..44f64e704a 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCChannel.m @@ -35,10 +35,15 @@ #include +#import "GRPCCompletionQueue.h" #import "GRPCSecureChannel.h" #import "GRPCUnsecuredChannel.h" -@implementation GRPCChannel +@implementation GRPCChannel { + grpc_channel *_unmanagedChannel; + NSString *_hostName; + GRPCCompletionQueue *_queue; +} + (instancetype)channelToHost:(NSString *)host { // TODO(mlumish): Investigate whether a cache with strong links is a good idea @@ -81,13 +86,32 @@ return nil; // silence warning. } -- (instancetype)initWithChannel:(struct grpc_channel *)unmanagedChannel { +- (instancetype)initWithChannel:(struct grpc_channel *)unmanagedChannel + hostName:(NSString *)hostName { + if (!unmanagedChannel || !hostName) { + [NSException raise:NSInvalidArgumentException + format:@"Neither unmanagedChannel nor hostName can be nil."]; + } if ((self = [super init])) { _unmanagedChannel = unmanagedChannel; + _hostName = hostName; + // In case sharing the queue creates contention, we can change it to one per grpc_call. + _queue = [GRPCCompletionQueue completionQueue]; + if (!_queue) { + return nil; + } } return self; } +- (grpc_call *)unmanagedCallWithPath:(NSString *)path { + return grpc_channel_create_call(_unmanagedChannel, + _queue.unmanagedQueue, + path.UTF8String, + _hostName.UTF8String, + gpr_inf_future(GPR_CLOCK_REALTIME)); +} + - (void)dealloc { // _unmanagedChannel is NULL when deallocating an object of the base class (because the // initializer returns a different object). diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m index 43a8bd539e..2dbbd52087 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m @@ -54,7 +54,8 @@ }); return (self = [super initWithChannel:grpc_secure_channel_create(kCredentials, host.UTF8String, - NULL)]); + NULL) + hostName:host]); } @end diff --git a/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m b/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m index 8518f78c5b..4941cbc3dd 100644 --- a/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m @@ -38,7 +38,8 @@ @implementation GRPCUnsecuredChannel - (instancetype)initWithHost:(NSString *)host { - return (self = [super initWithChannel:grpc_insecure_channel_create(host.UTF8String, NULL)]); + return (self = [super initWithChannel:grpc_insecure_channel_create(host.UTF8String, NULL) + hostName:host]); } @end diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m index 4681994bb2..db062336aa 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m @@ -39,7 +39,6 @@ #include #import "GRPCChannel.h" -#import "GRPCCompletionQueue.h" #import "NSDictionary+GRPC.h" #import "NSData+GRPC.h" #import "NSError+GRPC.h" @@ -227,7 +226,6 @@ @implementation GRPCWrappedCall{ GRPCChannel *_channel; grpc_call *_call; - GRPCCompletionQueue *_queue; } - (instancetype)init { @@ -240,26 +238,15 @@ [NSException raise:NSInvalidArgumentException format:@"path and host cannot be nil."]; } - + if (self = [super init]) { static dispatch_once_t initialization; dispatch_once(&initialization, ^{ grpc_init(); }); - - _queue = [GRPCCompletionQueue completionQueue]; - if (!_queue) { - return nil; - } + _channel = [GRPCChannel channelToHost:host]; - if (!_channel) { - return nil; - } - _call = grpc_channel_create_call(_channel.unmanagedChannel, - _queue.unmanagedQueue, - path.UTF8String, - host.UTF8String, - gpr_inf_future(GPR_CLOCK_REALTIME)); + _call = [_channel unmanagedCallWithPath:path]; if (_call == NULL) { return nil; } @@ -308,4 +295,4 @@ grpc_call_destroy(_call); } -@end \ No newline at end of file +@end -- cgit v1.2.3 From 148403af988522e61f4e71dc1bbd00b9f0d51a42 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Sat, 1 Aug 2015 02:56:04 -0700 Subject: Remove GRPCChannel-initWithHost to simplify implementation --- src/objective-c/GRPCClient/private/GRPCChannel.h | 1 - src/objective-c/GRPCClient/private/GRPCChannel.m | 22 +++++++++------------- .../GRPCClient/private/GRPCSecureChannel.h | 2 +- .../GRPCClient/private/GRPCUnsecuredChannel.h | 2 +- 4 files changed, 11 insertions(+), 16 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.h b/src/objective-c/GRPCClient/private/GRPCChannel.h index 49f5bfcc18..49f8b712b9 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCChannel.h @@ -43,7 +43,6 @@ // Convenience constructor to allow for reuse of connections. + (instancetype)channelToHost:(NSString *)host; -- (instancetype)initWithHost:(NSString *)host NS_DESIGNATED_INITIALIZER; - (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel hostName:(NSString *)hostName NS_DESIGNATED_INITIALIZER; diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.m b/src/objective-c/GRPCClient/private/GRPCChannel.m index 44f64e704a..90973cd83d 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCChannel.m @@ -54,17 +54,13 @@ }); GRPCChannel *channel = channelCache[host]; if (!channel) { - channel = [[self alloc] initWithHost:host]; + channel = [self uncachedChannelToHost:host]; channelCache[host] = channel; } return channel; } -- (instancetype)init { - return [self initWithHost:nil]; -} - -- (instancetype)initWithHost:(NSString *)host { ++ (instancetype)uncachedChannelToHost:(NSString *)host { if (![host rangeOfString:@"://"].length) { // No scheme provided; assume https. host = [@"https://" stringByAppendingString:host]; @@ -86,6 +82,10 @@ return nil; // silence warning. } +- (instancetype)init { + return [self initWithChannel:NULL hostName:nil]; +} + - (instancetype)initWithChannel:(struct grpc_channel *)unmanagedChannel hostName:(NSString *)hostName { if (!unmanagedChannel || !hostName) { @@ -113,12 +113,8 @@ } - (void)dealloc { - // _unmanagedChannel is NULL when deallocating an object of the base class (because the - // initializer returns a different object). - if (_unmanagedChannel) { - // TODO(jcanizales): Be sure to add a test with a server that closes the connection prematurely, - // as in the past that made this call to crash. - grpc_channel_destroy(_unmanagedChannel); - } + // TODO(jcanizales): Be sure to add a test with a server that closes the connection prematurely, + // as in the past that made this call to crash. + grpc_channel_destroy(_unmanagedChannel); } @end diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h index d34ceaea0c..8c5fe33d61 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h @@ -34,5 +34,5 @@ #import "GRPCChannel.h" @interface GRPCSecureChannel : GRPCChannel - +- (instancetype)initWithHost:(NSString *)host NS_DESIGNATED_INITIALIZER; @end diff --git a/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.h b/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.h index 9d89cfb541..8528be44c0 100644 --- a/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.h @@ -34,5 +34,5 @@ #import "GRPCChannel.h" @interface GRPCUnsecuredChannel : GRPCChannel - +- (instancetype)initWithHost:(NSString *)host NS_DESIGNATED_INITIALIZER; @end -- cgit v1.2.3 From e8543b071502fe0609d0c1657cfaf39d1a04c25d Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Sat, 1 Aug 2015 17:37:40 -0700 Subject: Let register SSL config per-host. Surfaced in GRPCCall+Tests.h Add GRPCHost to store channel config, and to create channels on demand with that config. GRPCChannels and configs are cached together. GRPCSecureChannel is now initialized with (nullable) path to a certificates file and (nullable) name override. The same mechanism will be used for creating insecure channels, removing the ability to do it by specifying the HTTP scheme in the address (which was deemed too subtle for its implications). --- src/objective-c/GRPCClient/GRPCCall+Tests.h | 45 ++++++++ src/objective-c/GRPCClient/GRPCCall+Tests.m | 47 ++++++++ src/objective-c/GRPCClient/private/GRPCChannel.h | 12 +- src/objective-c/GRPCClient/private/GRPCChannel.m | 74 +----------- .../GRPCClient/private/GRPCCompletionQueue.m | 2 - src/objective-c/GRPCClient/private/GRPCHost.h | 56 +++++++++ src/objective-c/GRPCClient/private/GRPCHost.m | 126 +++++++++++++++++++++ .../GRPCClient/private/GRPCSecureChannel.h | 12 +- .../GRPCClient/private/GRPCSecureChannel.m | 51 +++++++-- .../GRPCClient/private/GRPCUnsecuredChannel.m | 7 +- .../GRPCClient/private/GRPCWrappedCall.m | 15 ++- 11 files changed, 350 insertions(+), 97 deletions(-) create mode 100644 src/objective-c/GRPCClient/GRPCCall+Tests.h create mode 100644 src/objective-c/GRPCClient/GRPCCall+Tests.m create mode 100644 src/objective-c/GRPCClient/private/GRPCHost.h create mode 100644 src/objective-c/GRPCClient/private/GRPCHost.m (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/GRPCCall+Tests.h b/src/objective-c/GRPCClient/GRPCCall+Tests.h new file mode 100644 index 0000000000..3d617b05d9 --- /dev/null +++ b/src/objective-c/GRPCClient/GRPCCall+Tests.h @@ -0,0 +1,45 @@ +/* + * + * Copyright 2015, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#import "GRPCCall.h" + +@interface GRPCCall (Tests) + +// Establish all SSL connections to the provided host using the passed SSL target name and the root +// certificates found in the file at |certsPath|. +// Must be called before any gRPC call to that host is made. ++ (void)useTestCertsPath:(NSString *)certsPath + testName:(NSString *)testName + forHost:(NSString *)host; + +@end diff --git a/src/objective-c/GRPCClient/GRPCCall+Tests.m b/src/objective-c/GRPCClient/GRPCCall+Tests.m new file mode 100644 index 0000000000..7c5b81d661 --- /dev/null +++ b/src/objective-c/GRPCClient/GRPCCall+Tests.m @@ -0,0 +1,47 @@ +/* + * + * Copyright 2015, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#import "GRPCCall+Tests.h" + +#import "private/GRPCHost.h" + +@implementation GRPCCall (Tests) ++ (void)useTestCertsPath:(NSString *)certsPath + testName:(NSString *)testName + forHost:(NSString *)host { + GRPCHost *hostConfig = [GRPCHost hostWithAddress:host]; + hostConfig.secure = YES; + hostConfig.pathToCertificates = certsPath; + hostConfig.hostNameOverride = testName; +} +@end diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.h b/src/objective-c/GRPCClient/private/GRPCChannel.h index 49f8b712b9..fa9f6f28d1 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCChannel.h @@ -36,15 +36,9 @@ #include // Each separate instance of this class represents at least one TCP connection to the provided host. -// To create a grpc_call to that host, use |unmanagedCallWithPath|. Release this object when the -// call is finished. +// Create them using one of the subclasses |GRPCSecureChannel| and |GRPCUnsecuredChannel|. @interface GRPCChannel : NSObject +@property(nonatomic) grpc_channel *unmanagedChannel; -// Convenience constructor to allow for reuse of connections. -+ (instancetype)channelToHost:(NSString *)host; - -- (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel - hostName:(NSString *)hostName NS_DESIGNATED_INITIALIZER; - -- (grpc_call *)unmanagedCallWithPath:(NSString *)path; +- (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel NS_DESIGNATED_INITIALIZER; @end diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.m b/src/objective-c/GRPCClient/private/GRPCChannel.m index 90973cd83d..68a402db97 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCChannel.m @@ -33,85 +33,23 @@ #import "GRPCChannel.h" -#include - -#import "GRPCCompletionQueue.h" -#import "GRPCSecureChannel.h" -#import "GRPCUnsecuredChannel.h" - -@implementation GRPCChannel { - grpc_channel *_unmanagedChannel; - NSString *_hostName; - GRPCCompletionQueue *_queue; -} - -+ (instancetype)channelToHost:(NSString *)host { - // TODO(mlumish): Investigate whether a cache with strong links is a good idea - static NSMutableDictionary *channelCache; - static dispatch_once_t cacheInitialization; - dispatch_once(&cacheInitialization, ^{ - channelCache = [NSMutableDictionary dictionary]; - }); - GRPCChannel *channel = channelCache[host]; - if (!channel) { - channel = [self uncachedChannelToHost:host]; - channelCache[host] = channel; - } - return channel; -} - -+ (instancetype)uncachedChannelToHost:(NSString *)host { - if (![host rangeOfString:@"://"].length) { - // No scheme provided; assume https. - host = [@"https://" stringByAppendingString:host]; - } - NSURL *hostURL = [NSURL URLWithString:host]; - if (!hostURL) { - [NSException raise:NSInvalidArgumentException format:@"Invalid URL: %@", host]; - } - if ([hostURL.scheme isEqualToString:@"https"]) { - host = [@[hostURL.host, hostURL.port ?: @443] componentsJoinedByString:@":"]; - return [[GRPCSecureChannel alloc] initWithHost:host]; - } - if ([hostURL.scheme isEqualToString:@"http"]) { - host = [@[hostURL.host, hostURL.port ?: @80] componentsJoinedByString:@":"]; - return [[GRPCUnsecuredChannel alloc] initWithHost:host]; - } - [NSException raise:NSInvalidArgumentException - format:@"URL scheme %@ isn't supported.", hostURL.scheme]; - return nil; // silence warning. -} +@implementation GRPCChannel - (instancetype)init { - return [self initWithChannel:NULL hostName:nil]; + return [self initWithChannel:NULL]; } -- (instancetype)initWithChannel:(struct grpc_channel *)unmanagedChannel - hostName:(NSString *)hostName { - if (!unmanagedChannel || !hostName) { - [NSException raise:NSInvalidArgumentException - format:@"Neither unmanagedChannel nor hostName can be nil."]; +// Designated initializer +- (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel { + if (!unmanagedChannel) { + [NSException raise:NSInvalidArgumentException format:@"unmanagedChannel can't be nil."]; } if ((self = [super init])) { _unmanagedChannel = unmanagedChannel; - _hostName = hostName; - // In case sharing the queue creates contention, we can change it to one per grpc_call. - _queue = [GRPCCompletionQueue completionQueue]; - if (!_queue) { - return nil; - } } return self; } -- (grpc_call *)unmanagedCallWithPath:(NSString *)path { - return grpc_channel_create_call(_unmanagedChannel, - _queue.unmanagedQueue, - path.UTF8String, - _hostName.UTF8String, - gpr_inf_future(GPR_CLOCK_REALTIME)); -} - - (void)dealloc { // TODO(jcanizales): Be sure to add a test with a server that closes the connection prematurely, // as in the past that made this call to crash. diff --git a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m index 12535c9616..696069c200 100644 --- a/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m +++ b/src/objective-c/GRPCClient/private/GRPCCompletionQueue.m @@ -38,8 +38,6 @@ @implementation GRPCCompletionQueue + (instancetype)completionQueue { - // TODO(jcanizales): Reuse completion queues to consume only one thread, - // instead of one per call. return [[self alloc] init]; } diff --git a/src/objective-c/GRPCClient/private/GRPCHost.h b/src/objective-c/GRPCClient/private/GRPCHost.h new file mode 100644 index 0000000000..86e43a283d --- /dev/null +++ b/src/objective-c/GRPCClient/private/GRPCHost.h @@ -0,0 +1,56 @@ +/* + * + * Copyright 2015, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#import + +#import "GRPCCompletionQueue.h" + +@interface GRPCHost : NSObject + +@property(nonatomic, readonly) NSString *address; + +// The following properties should only be modified for testing: + +@property(nonatomic, getter=isSecure) BOOL secure; + +@property(nonatomic, copy) NSString *pathToCertificates; +@property(nonatomic, copy) NSString *hostNameOverride; + +// Host objects initialized with the same address are the same. ++ (instancetype)hostWithAddress:(NSString *)address; +- (instancetype)initWithAddress:(NSString *)address NS_DESIGNATED_INITIALIZER; + +// Create a grpc_call object to the provided path on this host. +- (grpc_call *)unmanagedCallWithPath:(NSString *)path completionQueue:(GRPCCompletionQueue *)queue; + +@end diff --git a/src/objective-c/GRPCClient/private/GRPCHost.m b/src/objective-c/GRPCClient/private/GRPCHost.m new file mode 100644 index 0000000000..405545b86b --- /dev/null +++ b/src/objective-c/GRPCClient/private/GRPCHost.m @@ -0,0 +1,126 @@ +/* + * + * Copyright 2015, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#import "GRPCHost.h" + +#import "GRPCChannel.h" +#import "GRPCSecureChannel.h" +#import "GRPCUnsecuredChannel.h" + +@interface GRPCHost () +// TODO(mlumish): Investigate whether a caching channels with strong links is a good idea. +@property(nonatomic, strong) GRPCChannel *channel; +@end + +@implementation GRPCHost + ++ (instancetype)hostWithAddress:(NSString *)address { + return [[self alloc] initWithAddress:address]; +} + +- (instancetype)init { + return [self initWithAddress:nil]; +} + +// Default initializer. +- (instancetype)initWithAddress:(NSString *)address { + + // Verify and normalize the address, and decide whether to use SSL. + if (![address rangeOfString:@"://"].length) { + // No scheme provided; assume https. + address = [@"https://" stringByAppendingString:address]; + } + NSURL *hostURL = [NSURL URLWithString:address]; + if (!hostURL) { + [NSException raise:NSInvalidArgumentException format:@"Invalid URL: %@", address]; + } + NSString *scheme = hostURL.scheme; + if (![scheme isEqualToString:@"https"] && ![scheme isEqualToString:@"http"]) { + [NSException raise:NSInvalidArgumentException format:@"URL scheme %@ isn't supported.", scheme]; + } + NSNumber *port = hostURL.port ?: [scheme isEqualToString:@"https"] ? @443 : @80; + address = [@[hostURL.host, port] componentsJoinedByString:@":"]; + + // Look up the GRPCHost in the cache. + // TODO(jcanizales): Make this cache thread-safe. + static NSMutableDictionary *hostCache; + static dispatch_once_t cacheInitialization; + dispatch_once(&cacheInitialization, ^{ + hostCache = [NSMutableDictionary dictionary]; + }); + if (hostCache[address]) { + // We could verify here that the cached host uses the same protocol that we're expecting. But + // picking HTTP by adding the scheme to the address is going away (to make the use of insecure + // channels less subtle), so it's not worth it now. + return hostCache[address]; + } + + if ((self = [super init])) { + _address = address; + _secure = [scheme isEqualToString:@"https"]; + hostCache[address] = self; + } + return self; +} + +- (grpc_call *)unmanagedCallWithPath:(NSString *)path completionQueue:(GRPCCompletionQueue *)queue { + if (!queue || !path) { + return NULL; + } + return grpc_channel_create_call(self.channel.unmanagedChannel, + queue.unmanagedQueue, + path.UTF8String, + self.hostName.UTF8String, + gpr_inf_future(GPR_CLOCK_REALTIME)); +} + +- (GRPCChannel *)channel { + // Create it lazily. + if (!_channel) { + if (_secure) { + _channel = [[GRPCSecureChannel alloc] initWithHost:_address + pathToCertificates:_pathToCertificates + hostNameOverride:_hostNameOverride]; + } else { + _channel = [[GRPCUnsecuredChannel alloc] initWithHost:_address]; + } + } + return _channel; +} + +- (NSString *)hostName { + // TODO(jcanizales): Default to nil instead of _address when Issue #2635 is clarified. + return _hostNameOverride ?: _address; +} + +@end diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h index 8c5fe33d61..8b259c8dad 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h @@ -31,8 +31,18 @@ * */ +#import + #import "GRPCChannel.h" @interface GRPCSecureChannel : GRPCChannel -- (instancetype)initWithHost:(NSString *)host NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithHost:(NSString *)host; + +- (instancetype)initWithHost:(NSString *)host + pathToCertificates:(NSString *)path + hostNameOverride:(NSString *)hostNameOverride; + +- (instancetype)initWithHost:(NSString *)host + credentials:(grpc_credentials *)credentials + args:(grpc_channel_args *)args NS_DESIGNATED_INITIALIZER; @end diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m index 2dbbd52087..c783462dd3 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m @@ -33,12 +33,24 @@ #import "GRPCSecureChannel.h" -#import +grpc_credentials *CertificatesAtPath(NSString *path) { + NSData *certsData = [NSData dataWithContentsOfFile:path]; + NSCAssert(certsData.length, @"No data read from %@", path); + NSString *certsString = [[NSString alloc] initWithData:certsData encoding:NSUTF8StringEncoding]; + return grpc_ssl_credentials_create(certsString.UTF8String, NULL); +} @implementation GRPCSecureChannel - (instancetype)initWithHost:(NSString *)host { - static grpc_credentials *kCredentials; + return [self initWithHost:host pathToCertificates:nil hostNameOverride:nil]; +} + +- (instancetype)initWithHost:(NSString *)host + pathToCertificates:(NSString *)path + hostNameOverride:(NSString *)hostNameOverride { + // Load default SSL certificates once. + static grpc_credentials *kDefaultCertificates; static dispatch_once_t loading; dispatch_once(&loading, ^{ // Do not use NSBundle.mainBundle, as it's nil for tests of library projects. @@ -47,15 +59,34 @@ NSAssert(certsPath.length, @"gRPCCertificates.bundle/roots.pem not found under %@. This file, with the root " "certificates, is needed to establish TLS (HTTPS) connections.", bundle.bundlePath); - NSData *certsData = [NSData dataWithContentsOfFile:certsPath]; - NSAssert(certsData.length, @"No data read from %@", certsPath); - NSString *certsString = [[NSString alloc] initWithData:certsData encoding:NSUTF8StringEncoding]; - kCredentials = grpc_ssl_credentials_create(certsString.UTF8String, NULL); + kDefaultCertificates = CertificatesAtPath(certsPath); }); - return (self = [super initWithChannel:grpc_secure_channel_create(kCredentials, - host.UTF8String, - NULL) - hostName:host]); + grpc_credentials *certificates = path ? CertificatesAtPath(path) : kDefaultCertificates; + + // Ritual to pass the SSL host name override to the C library. + grpc_channel_args channelArgs; + grpc_arg nameOverrideArg; + channelArgs.num_args = 1; + channelArgs.args = &nameOverrideArg; + nameOverrideArg.type = GRPC_ARG_STRING; + nameOverrideArg.key = GRPC_SSL_TARGET_NAME_OVERRIDE_ARG; + // Cast const away. Hope C gRPC doesn't modify it! + nameOverrideArg.value.string = (char *) hostNameOverride.UTF8String; + grpc_channel_args *args = hostNameOverride ? &channelArgs : NULL; + + return [self initWithHost:host credentials:certificates args:args]; +} + +- (instancetype)initWithHost:(NSString *)host + credentials:(grpc_credentials *)credentials + args:(grpc_channel_args *)args { + return (self = + [super initWithChannel:grpc_secure_channel_create(credentials, host.UTF8String, args)]); +} + +- (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel { + [NSException raise:NSInternalInconsistencyException format:@"use another initializer"]; + return [self initWithHost:nil]; // silence warnings } @end diff --git a/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m b/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m index 4941cbc3dd..5decfba7e3 100644 --- a/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m @@ -38,8 +38,11 @@ @implementation GRPCUnsecuredChannel - (instancetype)initWithHost:(NSString *)host { - return (self = [super initWithChannel:grpc_insecure_channel_create(host.UTF8String, NULL) - hostName:host]); + return (self = [super initWithChannel:grpc_insecure_channel_create(host.UTF8String, NULL)]); } +- (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel { + [NSException raise:NSInternalInconsistencyException format:@"use the other initializer"]; + return [self initWithHost:nil]; // silence warnings +} @end diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m index db062336aa..951c051036 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m @@ -38,7 +38,8 @@ #include #include -#import "GRPCChannel.h" +#import "GRPCCompletionQueue.h" +#import "GRPCHost.h" #import "NSDictionary+GRPC.h" #import "NSData+GRPC.h" #import "NSError+GRPC.h" @@ -223,8 +224,8 @@ #pragma mark GRPCWrappedCall -@implementation GRPCWrappedCall{ - GRPCChannel *_channel; +@implementation GRPCWrappedCall { + GRPCCompletionQueue *_queue; grpc_call *_call; } @@ -245,8 +246,12 @@ grpc_init(); }); - _channel = [GRPCChannel channelToHost:host]; - _call = [_channel unmanagedCallWithPath:path]; + // Each completion queue consumes one thread. There's a trade to be made between creating and + // consuming too many threads and having contention of multiple calls in a single completion + // queue. Currently we favor latency and use one per call. + _queue = [GRPCCompletionQueue completionQueue]; + + _call = [[GRPCHost hostWithAddress:host] unmanagedCallWithPath:path completionQueue:_queue]; if (_call == NULL) { return nil; } -- cgit v1.2.3 From 000fa388b8d7c99601092976b5b1d93a9705e34d Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Sat, 1 Aug 2015 18:31:50 -0700 Subject: Clarify comments in GRPCHost.m --- src/objective-c/GRPCClient/private/GRPCHost.m | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCHost.m b/src/objective-c/GRPCClient/private/GRPCHost.m index 405545b86b..f3e4ce3ab9 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.m +++ b/src/objective-c/GRPCClient/private/GRPCHost.m @@ -38,7 +38,7 @@ #import "GRPCUnsecuredChannel.h" @interface GRPCHost () -// TODO(mlumish): Investigate whether a caching channels with strong links is a good idea. +// TODO(mlumish): Investigate whether caching channels with strong links is a good idea. @property(nonatomic, strong) GRPCChannel *channel; @end @@ -80,8 +80,8 @@ }); if (hostCache[address]) { // We could verify here that the cached host uses the same protocol that we're expecting. But - // picking HTTP by adding the scheme to the address is going away (to make the use of insecure - // channels less subtle), so it's not worth it now. + // creating non-SSL channels by adding "http://" to the address is going away (to make the use + // of insecure channels less subtle), so it's not worth it now. return hostCache[address]; } @@ -105,7 +105,8 @@ } - (GRPCChannel *)channel { - // Create it lazily. + // Create it lazily, because we don't want to open a connection just because someone is + // configuring a host. if (!_channel) { if (_secure) { _channel = [[GRPCSecureChannel alloc] initWithHost:_address -- cgit v1.2.3 From e21b467dc59b8d0ddc6f64bde65df07e121a0132 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Sat, 1 Aug 2015 18:34:02 -0700 Subject: GRPCChannel with NULL grpc_channel is nil. --- src/objective-c/GRPCClient/private/GRPCChannel.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.m b/src/objective-c/GRPCClient/private/GRPCChannel.m index 68a402db97..49d6008fd4 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCChannel.m @@ -42,7 +42,7 @@ // Designated initializer - (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel { if (!unmanagedChannel) { - [NSException raise:NSInvalidArgumentException format:@"unmanagedChannel can't be nil."]; + return nil; } if ((self = [super init])) { _unmanagedChannel = unmanagedChannel; -- cgit v1.2.3 From 56c6574652ce261370582fec7decd4a8e6ce1cb3 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Mon, 3 Aug 2015 16:23:20 -0700 Subject: Fixup: GRPCChannel.unmanagedChannel back to readonly --- src/objective-c/GRPCClient/private/GRPCChannel.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.h b/src/objective-c/GRPCClient/private/GRPCChannel.h index fa9f6f28d1..de0b58ffe4 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCChannel.h @@ -38,7 +38,7 @@ // Each separate instance of this class represents at least one TCP connection to the provided host. // Create them using one of the subclasses |GRPCSecureChannel| and |GRPCUnsecuredChannel|. @interface GRPCChannel : NSObject -@property(nonatomic) grpc_channel *unmanagedChannel; +@property(nonatomic, readonly) grpc_channel *unmanagedChannel; - (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel NS_DESIGNATED_INITIALIZER; @end -- cgit v1.2.3 From 7d261ee5b6bf69378787600971de0361d2c76081 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Mon, 3 Aug 2015 17:03:56 -0700 Subject: Fixup: mark CertificatesAtPath static --- src/objective-c/GRPCClient/private/GRPCSecureChannel.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m index c783462dd3..eb3cfc40eb 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m @@ -33,7 +33,7 @@ #import "GRPCSecureChannel.h" -grpc_credentials *CertificatesAtPath(NSString *path) { +static grpc_credentials *CertificatesAtPath(NSString *path) { NSData *certsData = [NSData dataWithContentsOfFile:path]; NSCAssert(certsData.length, @"No data read from %@", path); NSString *certsString = [[NSString alloc] initWithData:certsData encoding:NSUTF8StringEncoding]; -- cgit v1.2.3 From 354e2127a2fd5003449fb3d6456bd28818d0ac9b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 5 Aug 2015 16:03:55 -0700 Subject: Update Objective-C --- src/objective-c/GRPCClient/private/GRPCWrappedCall.m | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m index 1db63df77f..ecc512edca 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m @@ -246,11 +246,10 @@ if (!_queue) { return nil; } - _call = grpc_channel_create_call(channel.unmanagedChannel, - _queue.unmanagedQueue, - path.UTF8String, - host.UTF8String, - gpr_inf_future(GPR_CLOCK_REALTIME)); + _call = grpc_channel_create_call( + channel.unmanagedChannel, NULL, GRPC_PROPAGATE_DEFAULTS, + _queue.unmanagedQueue, path.UTF8String, host.UTF8String, + gpr_inf_future(GPR_CLOCK_REALTIME)); if (_call == NULL) { return nil; } @@ -299,4 +298,4 @@ grpc_call_destroy(_call); } -@end \ No newline at end of file +@end -- cgit v1.2.3 From 0607bae87e0f3417972bc221a5c4f8dc6bec5fef Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 5 Aug 2015 16:36:10 -0700 Subject: Forward-declare grpc_channel and specify ownership semantics --- src/objective-c/GRPCClient/private/GRPCChannel.h | 8 +++++--- src/objective-c/GRPCClient/private/GRPCChannel.m | 2 ++ 2 files changed, 7 insertions(+), 3 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.h b/src/objective-c/GRPCClient/private/GRPCChannel.h index de0b58ffe4..2a7b701576 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCChannel.h @@ -33,12 +33,14 @@ #import -#include +struct grpc_channel; // Each separate instance of this class represents at least one TCP connection to the provided host. // Create them using one of the subclasses |GRPCSecureChannel| and |GRPCUnsecuredChannel|. @interface GRPCChannel : NSObject -@property(nonatomic, readonly) grpc_channel *unmanagedChannel; +@property(nonatomic, readonly) struct grpc_channel *unmanagedChannel; -- (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel NS_DESIGNATED_INITIALIZER; +// This initializer takes ownership of the passed channel, and will destroy it when this object is +// deallocated. It's illegal to pass the same grpc_channel to two different GRPCChannel objects. +- (instancetype)initWithChannel:(struct grpc_channel *)unmanagedChannel NS_DESIGNATED_INITIALIZER; @end diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.m b/src/objective-c/GRPCClient/private/GRPCChannel.m index 49d6008fd4..4366e63320 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCChannel.m @@ -33,6 +33,8 @@ #import "GRPCChannel.h" +#include + @implementation GRPCChannel - (instancetype)init { -- cgit v1.2.3 From 83b7971c10d18fcc1c04056a028860283b2be051 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 5 Aug 2015 17:12:04 -0700 Subject: Forward-declare GRPCCompletionQueue and grpc_call --- src/objective-c/GRPCClient/private/GRPCHost.h | 6 ++++-- src/objective-c/GRPCClient/private/GRPCHost.m | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCHost.h b/src/objective-c/GRPCClient/private/GRPCHost.h index 86e43a283d..f0bbd53023 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.h +++ b/src/objective-c/GRPCClient/private/GRPCHost.h @@ -33,7 +33,8 @@ #import -#import "GRPCCompletionQueue.h" +@class GRPCCompletionQueue; +struct grpc_call; @interface GRPCHost : NSObject @@ -51,6 +52,7 @@ - (instancetype)initWithAddress:(NSString *)address NS_DESIGNATED_INITIALIZER; // Create a grpc_call object to the provided path on this host. -- (grpc_call *)unmanagedCallWithPath:(NSString *)path completionQueue:(GRPCCompletionQueue *)queue; +- (struct grpc_call *)unmanagedCallWithPath:(NSString *)path + completionQueue:(GRPCCompletionQueue *)queue; @end diff --git a/src/objective-c/GRPCClient/private/GRPCHost.m b/src/objective-c/GRPCClient/private/GRPCHost.m index f3e4ce3ab9..05c72e447d 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.m +++ b/src/objective-c/GRPCClient/private/GRPCHost.m @@ -33,7 +33,10 @@ #import "GRPCHost.h" +#include + #import "GRPCChannel.h" +#import "GRPCCompletionQueue.h" #import "GRPCSecureChannel.h" #import "GRPCUnsecuredChannel.h" -- cgit v1.2.3 From cceeb515927d72e3cfb24ecce4af89a9eed3b42b Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 5 Aug 2015 17:16:42 -0700 Subject: Document intention of hostURL.port conditional check. --- src/objective-c/GRPCClient/private/GRPCHost.m | 1 + 1 file changed, 1 insertion(+) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCHost.m b/src/objective-c/GRPCClient/private/GRPCHost.m index 05c72e447d..7355139fef 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.m +++ b/src/objective-c/GRPCClient/private/GRPCHost.m @@ -71,6 +71,7 @@ if (![scheme isEqualToString:@"https"] && ![scheme isEqualToString:@"http"]) { [NSException raise:NSInvalidArgumentException format:@"URL scheme %@ isn't supported.", scheme]; } + // If the user didn't specify a port (hostURL.port is nil), provide a default one. NSNumber *port = hostURL.port ?: [scheme isEqualToString:@"https"] ? @443 : @80; address = [@[hostURL.host, port] componentsJoinedByString:@":"]; -- cgit v1.2.3 From 82fb883bec8eef4f396723d259ceea06fa2771ea Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 5 Aug 2015 17:22:53 -0700 Subject: Make GRPCHost cache thread-safe. --- src/objective-c/GRPCClient/private/GRPCHost.m | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCHost.m b/src/objective-c/GRPCClient/private/GRPCHost.m index 7355139fef..452283c76e 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.m +++ b/src/objective-c/GRPCClient/private/GRPCHost.m @@ -76,25 +76,27 @@ address = [@[hostURL.host, port] componentsJoinedByString:@":"]; // Look up the GRPCHost in the cache. - // TODO(jcanizales): Make this cache thread-safe. static NSMutableDictionary *hostCache; static dispatch_once_t cacheInitialization; dispatch_once(&cacheInitialization, ^{ hostCache = [NSMutableDictionary dictionary]; }); - if (hostCache[address]) { - // We could verify here that the cached host uses the same protocol that we're expecting. But - // creating non-SSL channels by adding "http://" to the address is going away (to make the use - // of insecure channels less subtle), so it's not worth it now. - return hostCache[address]; - } + @synchronized(hostCache) { + GRPCHost *cachedHost = hostCache[address]; + if (cachedHost) { + // We could verify here that the cached host uses the same protocol that we're expecting. But + // creating non-SSL channels by adding "http://" to the address is going away (to make the use + // of insecure channels less subtle), so it's not worth it now. + return cachedHost; + } - if ((self = [super init])) { - _address = address; - _secure = [scheme isEqualToString:@"https"]; - hostCache[address] = self; + if ((self = [super init])) { + _address = address; + _secure = [scheme isEqualToString:@"https"]; + hostCache[address] = self; + } + return self; } - return self; } - (grpc_call *)unmanagedCallWithPath:(NSString *)path completionQueue:(GRPCCompletionQueue *)queue { -- cgit v1.2.3 From d7f2ab31258a0972eb08e9fac0cac5b64b2a7919 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 5 Aug 2015 17:38:00 -0700 Subject: Forward-declare structs in GRPCSecureChannel.h And add warning about using custom certificates or name override if not testing. --- src/objective-c/GRPCClient/private/GRPCSecureChannel.h | 5 ++++- src/objective-c/GRPCClient/private/GRPCSecureChannel.m | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h index 8b259c8dad..ca8780ee8b 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h @@ -31,13 +31,16 @@ * */ -#import +struct grpc_credentials; +struct grpc_channel_args; #import "GRPCChannel.h" @interface GRPCSecureChannel : GRPCChannel - (instancetype)initWithHost:(NSString *)host; +// Only in tests shouldn't pathToCertificates or hostNameOverride be nil. Passing nil for +// pathToCertificates results in using the default root certificates distributed with the library. - (instancetype)initWithHost:(NSString *)host pathToCertificates:(NSString *)path hostNameOverride:(NSString *)hostNameOverride; diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m index eb3cfc40eb..92421df9c5 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m @@ -33,6 +33,8 @@ #import "GRPCSecureChannel.h" +#import + static grpc_credentials *CertificatesAtPath(NSString *path) { NSData *certsData = [NSData dataWithContentsOfFile:path]; NSCAssert(certsData.length, @"No data read from %@", path); -- cgit v1.2.3 From d13bbed8becc2f3564734399012c09660ceed303 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 5 Aug 2015 18:11:06 -0700 Subject: Fix breakage (struct before undefined structs) --- src/objective-c/GRPCClient/private/GRPCSecureChannel.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h index ca8780ee8b..40207966b5 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h @@ -46,6 +46,6 @@ struct grpc_channel_args; hostNameOverride:(NSString *)hostNameOverride; - (instancetype)initWithHost:(NSString *)host - credentials:(grpc_credentials *)credentials - args:(grpc_channel_args *)args NS_DESIGNATED_INITIALIZER; + credentials:(struct grpc_credentials *)credentials + args:(struct grpc_channel_args *)args NS_DESIGNATED_INITIALIZER; @end -- cgit v1.2.3 From 145f865ce04659bf8759ac66f80ae9ef1c029d74 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 5 Aug 2015 19:30:45 -0700 Subject: Undo forward-declaring grpc_channel_args, which isn’t a struct! MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s a typedef of an anonymous struct. --- src/objective-c/GRPCClient/private/GRPCSecureChannel.h | 7 ++++--- src/objective-c/GRPCClient/private/GRPCSecureChannel.m | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h index 40207966b5..5c93399333 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h @@ -31,11 +31,12 @@ * */ -struct grpc_credentials; -struct grpc_channel_args; +#include #import "GRPCChannel.h" +struct grpc_credentials; + @interface GRPCSecureChannel : GRPCChannel - (instancetype)initWithHost:(NSString *)host; @@ -47,5 +48,5 @@ struct grpc_channel_args; - (instancetype)initWithHost:(NSString *)host credentials:(struct grpc_credentials *)credentials - args:(struct grpc_channel_args *)args NS_DESIGNATED_INITIALIZER; + args:(grpc_channel_args *)args NS_DESIGNATED_INITIALIZER; @end diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m index 92421df9c5..310fdb67c2 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m @@ -33,7 +33,7 @@ #import "GRPCSecureChannel.h" -#import +#include static grpc_credentials *CertificatesAtPath(NSString *path) { NSData *certsData = [NSData dataWithContentsOfFile:path]; -- cgit v1.2.3 From 8c5318a6d122e9b933abc43780c131838cebac80 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 5 Aug 2015 18:50:08 -0700 Subject: Return nil instead of assert when the test certs can’t be read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/objective-c/GRPCClient/GRPCCall.m | 3 ++ src/objective-c/GRPCClient/private/GRPCHost.m | 2 +- .../GRPCClient/private/GRPCSecureChannel.m | 38 +++++++++++++++------- 3 files changed, 31 insertions(+), 12 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index b6d4d52c7e..5f7d74bca8 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -103,6 +103,9 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; } if ((self = [super init])) { _wrappedCall = [[GRPCWrappedCall alloc] initWithHost:host path:path]; + if (!_wrappedCall) { + return nil; + } // Serial queue to invoke the non-reentrant methods of the grpc_call object. _callQueue = dispatch_queue_create("org.grpc.call", NULL); diff --git a/src/objective-c/GRPCClient/private/GRPCHost.m b/src/objective-c/GRPCClient/private/GRPCHost.m index 452283c76e..781eab20c5 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.m +++ b/src/objective-c/GRPCClient/private/GRPCHost.m @@ -100,7 +100,7 @@ } - (grpc_call *)unmanagedCallWithPath:(NSString *)path completionQueue:(GRPCCompletionQueue *)queue { - if (!queue || !path) { + if (!queue || !path || !self.channel) { return NULL; } return grpc_channel_create_call(self.channel.unmanagedChannel, diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m index 310fdb67c2..d3c4d41a13 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m @@ -35,11 +35,18 @@ #include -static grpc_credentials *CertificatesAtPath(NSString *path) { - NSData *certsData = [NSData dataWithContentsOfFile:path]; - NSCAssert(certsData.length, @"No data read from %@", path); - NSString *certsString = [[NSString alloc] initWithData:certsData encoding:NSUTF8StringEncoding]; - return grpc_ssl_credentials_create(certsString.UTF8String, NULL); +// Returns NULL if the file at path couldn't be read. In that case, if errorPtr isn't NULL, +// *errorPtr will be an object describing what went wrong. +static grpc_credentials *CertificatesAtPath(NSString *path, NSError **errorPtr) { + NSString *certsContent = [NSString stringWithContentsOfFile:path + encoding:NSASCIIStringEncoding + error:errorPtr]; + if (!certsContent) { + // Passing NULL to grpc_ssl_credentials_create produces behavior we don't want, so return. + return NULL; + } + const char * asCString = [certsContent cStringUsingEncoding:NSASCIIStringEncoding]; + return grpc_ssl_credentials_create(asCString, NULL); } @implementation GRPCSecureChannel @@ -55,15 +62,24 @@ static grpc_credentials *CertificatesAtPath(NSString *path) { static grpc_credentials *kDefaultCertificates; static dispatch_once_t loading; dispatch_once(&loading, ^{ + NSString *defaultPath = @"gRPCCertificates.bundle/roots"; // .pem // Do not use NSBundle.mainBundle, as it's nil for tests of library projects. NSBundle *bundle = [NSBundle bundleForClass:self.class]; - NSString *certsPath = [bundle pathForResource:@"gRPCCertificates.bundle/roots" ofType:@"pem"]; - NSAssert(certsPath.length, - @"gRPCCertificates.bundle/roots.pem not found under %@. This file, with the root " - "certificates, is needed to establish TLS (HTTPS) connections.", bundle.bundlePath); - kDefaultCertificates = CertificatesAtPath(certsPath); + NSString *path = [bundle pathForResource:defaultPath ofType:@"pem"]; + NSError *error; + kDefaultCertificates = CertificatesAtPath(path, &error); + NSAssert(kDefaultCertificates, @"Could not read %@/%@.pem. This file, with the root " + "certificates, is needed to establish secure (TLS) connections. Because the file is " + "distributed with the gRPC library, this error is usually a sign that the library " + "wasn't configured correctly for your project. Error: %@", + bundle.bundlePath, defaultPath, error); }); - grpc_credentials *certificates = path ? CertificatesAtPath(path) : kDefaultCertificates; + + //TODO(jcanizales): Add NSError** parameter to the initializer. + grpc_credentials *certificates = path ? CertificatesAtPath(path, NULL) : kDefaultCertificates; + if (!certificates) { + return nil; + } // Ritual to pass the SSL host name override to the C library. grpc_channel_args channelArgs; -- cgit v1.2.3 From 77723b127a2eafd1f99a7a3362371be310d5df29 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 5 Aug 2015 20:31:39 -0700 Subject: Document that grpc_channel_args don’t need to survive GRPCSecureChannel init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/objective-c/GRPCClient/private/GRPCSecureChannel.h | 1 + 1 file changed, 1 insertion(+) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h index 5c93399333..74257eb058 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.h +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.h @@ -46,6 +46,7 @@ struct grpc_credentials; pathToCertificates:(NSString *)path hostNameOverride:(NSString *)hostNameOverride; +// The passed arguments aren't required to be valid beyond the invocation of this initializer. - (instancetype)initWithHost:(NSString *)host credentials:(struct grpc_credentials *)credentials args:(grpc_channel_args *)args NS_DESIGNATED_INITIALIZER; -- cgit v1.2.3 From 7e90745b3c609d12dfc63ca6cdddac9446b3d5e5 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 5 Aug 2015 21:19:22 -0700 Subject: Document plan to merge the GRPCChannel subclasses --- src/objective-c/GRPCClient/private/GRPCSecureChannel.m | 2 ++ src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m | 2 ++ 2 files changed, 4 insertions(+) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m index d3c4d41a13..9b4b6768f8 100644 --- a/src/objective-c/GRPCClient/private/GRPCSecureChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCSecureChannel.m @@ -102,6 +102,8 @@ static grpc_credentials *CertificatesAtPath(NSString *path, NSError **errorPtr) [super initWithChannel:grpc_secure_channel_create(credentials, host.UTF8String, args)]); } +// TODO(jcanizales): GRPCSecureChannel and GRPCUnsecuredChannel are just convenience initializers +// for GRPCChannel. Move them into GRPCChannel, which will make the following unnecessary. - (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel { [NSException raise:NSInternalInconsistencyException format:@"use another initializer"]; return [self initWithHost:nil]; // silence warnings diff --git a/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m b/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m index 5decfba7e3..070a529629 100644 --- a/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCUnsecuredChannel.m @@ -41,6 +41,8 @@ return (self = [super initWithChannel:grpc_insecure_channel_create(host.UTF8String, NULL)]); } +// TODO(jcanizales): GRPCSecureChannel and GRPCUnsecuredChannel are just convenience initializers +// for GRPCChannel. Move them into GRPCChannel, which will make the following unnecessary. - (instancetype)initWithChannel:(grpc_channel *)unmanagedChannel { [NSException raise:NSInternalInconsistencyException format:@"use the other initializer"]; return [self initWithHost:nil]; // silence warnings -- cgit v1.2.3 From b2bd06775e255ebf7de96eb00a5b1738311c682e Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Sat, 1 Aug 2015 23:19:11 -0700 Subject: Require very explicit registration of non-SSL hosts. --- src/objective-c/GRPCClient/GRPCCall+Tests.h | 4 +++ src/objective-c/GRPCClient/GRPCCall+Tests.m | 8 +++++- src/objective-c/GRPCClient/private/GRPCHost.m | 37 ++++++++++----------------- src/objective-c/tests/GRPCClientTests.m | 7 +++-- src/objective-c/tests/InteropTests.h | 2 +- src/objective-c/tests/InteropTests.m | 10 +++++++- 6 files changed, 39 insertions(+), 29 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/GRPCCall+Tests.h b/src/objective-c/GRPCClient/GRPCCall+Tests.h index 3d617b05d9..981b154b40 100644 --- a/src/objective-c/GRPCClient/GRPCCall+Tests.h +++ b/src/objective-c/GRPCClient/GRPCCall+Tests.h @@ -33,6 +33,8 @@ #import "GRPCCall.h" +// Methods to let tune down the security of gRPC connections for specific hosts. These shouldn't be +// used in releases, but are sometimes needed for testing. @interface GRPCCall (Tests) // Establish all SSL connections to the provided host using the passed SSL target name and the root @@ -42,4 +44,6 @@ testName:(NSString *)testName forHost:(NSString *)host; +// Establish all connections to the provided host using cleartext instead of SSL. ++ (void)useInsecureConnectionsForHost:(NSString *)host; @end diff --git a/src/objective-c/GRPCClient/GRPCCall+Tests.m b/src/objective-c/GRPCClient/GRPCCall+Tests.m index 7c5b81d661..bade0b2920 100644 --- a/src/objective-c/GRPCClient/GRPCCall+Tests.m +++ b/src/objective-c/GRPCClient/GRPCCall+Tests.m @@ -36,12 +36,18 @@ #import "private/GRPCHost.h" @implementation GRPCCall (Tests) + + (void)useTestCertsPath:(NSString *)certsPath testName:(NSString *)testName forHost:(NSString *)host { GRPCHost *hostConfig = [GRPCHost hostWithAddress:host]; - hostConfig.secure = YES; hostConfig.pathToCertificates = certsPath; hostConfig.hostNameOverride = testName; } + ++ (void)useInsecureConnectionsForHost:(NSString *)host { + GRPCHost *hostConfig = [GRPCHost hostWithAddress:host]; + hostConfig.secure = NO; +} + @end diff --git a/src/objective-c/GRPCClient/private/GRPCHost.m b/src/objective-c/GRPCClient/private/GRPCHost.m index 5d9c48a524..14bde92d98 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.m +++ b/src/objective-c/GRPCClient/private/GRPCHost.m @@ -58,22 +58,12 @@ // Default initializer. - (instancetype)initWithAddress:(NSString *)address { - // Verify and normalize the address, and decide whether to use SSL. - if (![address rangeOfString:@"://"].length) { - // No scheme provided; assume https. - address = [@"https://" stringByAppendingString:address]; + // To provide a default port, we try to interpret the address. + // TODO(jcanizales): Add unit tests for the types of addresses we want to let pass through. + NSURL *hostURL = [NSURL URLWithString:[@"https://" stringByAppendingString:address]]; + if (hostURL && !hostURL.port) { + address = [hostURL.host stringByAppendingString:@":443"]; } - NSURL *hostURL = [NSURL URLWithString:address]; - if (!hostURL) { - [NSException raise:NSInvalidArgumentException format:@"Invalid URL: %@", address]; - } - NSString *scheme = hostURL.scheme; - if (![scheme isEqualToString:@"https"] && ![scheme isEqualToString:@"http"]) { - [NSException raise:NSInvalidArgumentException format:@"URL scheme %@ isn't supported.", scheme]; - } - // If the user didn't specify a port (hostURL.port is nil), provide a default one. - NSNumber *port = hostURL.port ?: [scheme isEqualToString:@"https"] ? @443 : @80; - address = [@[hostURL.host, port] componentsJoinedByString:@":"]; // Look up the GRPCHost in the cache. static NSMutableDictionary *hostCache; @@ -84,19 +74,15 @@ @synchronized(hostCache) { GRPCHost *cachedHost = hostCache[address]; if (cachedHost) { - // We could verify here that the cached host uses the same protocol that we're expecting. But - // creating non-SSL channels by adding "http://" to the address is going away (to make the use - // of insecure channels less subtle), so it's not worth it now. return cachedHost; } - if ((self = [super init])) { - _address = address; - _secure = [scheme isEqualToString:@"https"]; - hostCache[address] = self; - } - return self; + if ((self = [super init])) { + _address = address; + _secure = YES; + hostCache[address] = self; } + return self; } - (grpc_call *)unmanagedCallWithPath:(NSString *)path completionQueue:(GRPCCompletionQueue *)queue { @@ -131,4 +117,7 @@ return _hostNameOverride ?: _address; } +// TODO(jcanizales): Don't let set |secure| to |NO| if |pathToCertificates| or |hostNameOverride| +// have been set. Don't let set either of the latter if |secure| has been set to |NO|. + @end diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 103e5ca3d4..e5d7e43ed9 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -35,6 +35,7 @@ #import #import +#import #import #import #import @@ -43,8 +44,7 @@ // These are a few tests similar to InteropTests, but which use the generic gRPC client (GRPCCall) // rather than a generated proto library on top of it. -// grpc-test.sandbox.google.com -static NSString * const kHostAddress = @"http://localhost:5050"; +static NSString * const kHostAddress = @"localhost:5050"; static NSString * const kPackage = @"grpc.testing"; static NSString * const kService = @"TestService"; @@ -58,6 +58,9 @@ static ProtoMethod *kUnaryCallMethod; @implementation GRPCClientTests - (void)setUp { + // Register test server as non-SSL. + [GRPCCall useInsecureConnectionsForHost:kHostAddress]; + // This method isn't implemented by the remote server. kInexistentMethod = [[ProtoMethod alloc] initWithPackage:kPackage service:kService diff --git a/src/objective-c/tests/InteropTests.h b/src/objective-c/tests/InteropTests.h index c675c8d241..4eb97e9e06 100644 --- a/src/objective-c/tests/InteropTests.h +++ b/src/objective-c/tests/InteropTests.h @@ -37,7 +37,7 @@ // https://github.com/grpc/grpc/blob/master/doc/interop-test-descriptions.md @interface InteropTests : XCTestCase -// Returns @"http://localhost:5050". +// Returns @"localhost:5050". // Override in a subclass to perform the same tests against a different address. // For interop tests, use @"grpc-test.sandbox.google.com". + (NSString *)host; diff --git a/src/objective-c/tests/InteropTests.m b/src/objective-c/tests/InteropTests.m index a6611d27be..b61d567464 100644 --- a/src/objective-c/tests/InteropTests.m +++ b/src/objective-c/tests/InteropTests.m @@ -35,6 +35,7 @@ #include +#import #import #import #import @@ -75,15 +76,22 @@ } @end +#pragma mark Tests + +static NSString * const kLocalCleartextHost = @"localhost:5050"; + @implementation InteropTests { RMTTestService *_service; } + (NSString *)host { - return @"http://localhost:5050"; + return kLocalCleartextHost; } - (void)setUp { + // Register test server as non-SSL. + [GRPCCall useInsecureConnectionsForHost:kLocalCleartextHost]; + _service = [[RMTTestService alloc] initWithHost:self.class.host]; } -- cgit v1.2.3 From 015ab35a2875059c9047de2286c22c96a01628cb Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Thu, 6 Aug 2015 16:51:44 -0700 Subject: Clarify intention of the code that adds a default port --- src/objective-c/GRPCClient/private/GRPCHost.m | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/objective-c/GRPCClient/private') diff --git a/src/objective-c/GRPCClient/private/GRPCHost.m b/src/objective-c/GRPCClient/private/GRPCHost.m index 14bde92d98..6636c48620 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.m +++ b/src/objective-c/GRPCClient/private/GRPCHost.m @@ -58,8 +58,10 @@ // Default initializer. - (instancetype)initWithAddress:(NSString *)address { - // To provide a default port, we try to interpret the address. - // TODO(jcanizales): Add unit tests for the types of addresses we want to let pass through. + // To provide a default port, we try to interpret the address. If it's just a host name without + // scheme and without port, we'll use port 443. If it has a scheme, we pass it untouched to the C + // gRPC library. + // TODO(jcanizales): Add unit tests for the types of addresses we want to let pass untouched. NSURL *hostURL = [NSURL URLWithString:[@"https://" stringByAppendingString:address]]; if (hostURL && !hostURL.port) { address = [hostURL.host stringByAppendingString:@":443"]; -- cgit v1.2.3