Skip to content

Commit

Permalink
Remove usage of NSLog in SocketRocket code
Browse files Browse the repository at this point in the history 
Replace it with calls to an ARTLog instance.

Closes #1554.
  • Loading branch information
@lawrence-forooghian
lawrence-forooghian committed Dec 7, 2022
1 parent b2e024c commit 527abca
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 80 deletions.
11 changes: 10 additions & 1 deletion Package.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,16 @@ let package = Package(
.headerSearchPath("Internal/Utilities"),
.headerSearchPath("Internal/RunLoop"),
.headerSearchPath("Internal/Delegate"),
.headerSearchPath("Internal/IOConsumer")
.headerSearchPath("Internal/IOConsumer"),
/*
This is a quick solution that allows us to access the Ably
logger types from inside SocketRocket. I think a neater
solution might be to remove this separation and move the
SocketRocket code into the Ably target so that they can share
types freely, but I think this is OK for now.
*/
.headerSearchPath("../../Source/include/Ably"), // For the #import "ARTLog.h" in ARTSRLog.m
.headerSearchPath("../../Source/include") // For the #import <Ably/ARTTypes.h> in the ARTLog.h imported by ARTSRLog.m
]
),
.target(
Expand Down
19 changes: 10 additions & 9 deletions SocketRocket/SocketRocket/ARTSRWebSocket.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ typedef NS_ENUM(NSInteger, ARTSRStatusCode) {

@class ARTSRWebSocket;
@class ARTSRSecurityPolicy;
@class ARTLog;

/**
Error domain used for errors reported by ARTSRWebSocket.
Expand Down Expand Up @@ -131,23 +132,23 @@ extern NSString *const ARTSRHTTPResponseErrorKey;
@param request Request to initialize with.
*/
- (instancetype)initWithURLRequest:(NSURLRequest *)request;
- (instancetype)initWithURLRequest:(NSURLRequest *)request logger:(nullable ARTLog *)logger;

/**
Initializes a web socket with a given `NSURLRequest`, specifying a transport security policy (e.g. SSL configuration).
@param request Request to initialize with.
@param securityPolicy Policy object describing transport security behavior.
*/
- (instancetype)initWithURLRequest:(NSURLRequest *)request securityPolicy:(ARTSRSecurityPolicy *)securityPolicy;
- (instancetype)initWithURLRequest:(NSURLRequest *)request securityPolicy:(ARTSRSecurityPolicy *)securityPolicy logger:(nullable ARTLog *)logger;

/**
Initializes a web socket with a given `NSURLRequest` and list of sub-protocols.
@param request Request to initialize with.
@param protocols An array of strings that turn into `Sec-WebSocket-Protocol`. Default: `nil`.
*/
- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(nullable NSArray<NSString *> *)protocols;
- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(nullable NSArray<NSString *> *)protocols logger:(nullable ARTLog *)logger;

/**
Initializes a web socket with a given `NSURLRequest`, list of sub-protocols and whether untrusted SSL certificates are allowed.
Expand All @@ -156,7 +157,7 @@ extern NSString *const ARTSRHTTPResponseErrorKey;
@param protocols An array of strings that turn into `Sec-WebSocket-Protocol`. Default: `nil`.
@param allowsUntrustedSSLCertificates Boolean value indicating whether untrusted SSL certificates are allowed. Default: `false`.
*/
- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(nullable NSArray<NSString *> *)protocols allowsUntrustedSSLCertificates:(BOOL)allowsUntrustedSSLCertificates
- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(nullable NSArray<NSString *> *)protocols allowsUntrustedSSLCertificates:(BOOL)allowsUntrustedSSLCertificates logger:(nullable ARTLog *)logger
DEPRECATED_MSG_ATTRIBUTE("Disabling certificate chain validation is unsafe. "
"Please use a proper Certificate Authority to issue your TLS certificates.");

Expand All @@ -167,30 +168,30 @@ extern NSString *const ARTSRHTTPResponseErrorKey;
@param protocols An array of strings that turn into `Sec-WebSocket-Protocol`. Default: `nil`.
@param securityPolicy Policy object describing transport security behavior.
*/
- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(nullable NSArray<NSString *> *)protocols securityPolicy:(ARTSRSecurityPolicy *)securityPolicy NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(nullable NSArray<NSString *> *)protocols securityPolicy:(ARTSRSecurityPolicy *)securityPolicy logger:(nullable ARTLog *)logger NS_DESIGNATED_INITIALIZER;

/**
Initializes a web socket with a given `NSURL`.
@param url URL to initialize with.
*/
- (instancetype)initWithURL:(NSURL *)url;
- (instancetype)initWithURL:(NSURL *)url logger:(nullable ARTLog *)logger;

/**
Initializes a web socket with a given `NSURL` and list of sub-protocols.
@param url URL to initialize with.
@param protocols An array of strings that turn into `Sec-WebSocket-Protocol`. Default: `nil`.
*/
- (instancetype)initWithURL:(NSURL *)url protocols:(nullable NSArray<NSString *> *)protocols;
- (instancetype)initWithURL:(NSURL *)url protocols:(nullable NSArray<NSString *> *)protocols logger:(nullable ARTLog *)logger;

/**
Initializes a web socket with a given `NSURL`, specifying a transport security policy (e.g. SSL configuration).
@param url URL to initialize with.
@param securityPolicy Policy object describing transport security behavior.
*/
- (instancetype)initWithURL:(NSURL *)url securityPolicy:(ARTSRSecurityPolicy *)securityPolicy;
- (instancetype)initWithURL:(NSURL *)url securityPolicy:(ARTSRSecurityPolicy *)securityPolicy logger:(nullable ARTLog *)logger;

/**
Initializes a web socket with a given `NSURL`, list of sub-protocols and whether untrusted SSL certificates are allowed.
Expand All @@ -199,7 +200,7 @@ extern NSString *const ARTSRHTTPResponseErrorKey;
@param protocols An array of strings that turn into `Sec-WebSocket-Protocol`. Default: `nil`.
@param allowsUntrustedSSLCertificates Boolean value indicating whether untrusted SSL certificates are allowed. Default: `false`.
*/
- (instancetype)initWithURL:(NSURL *)url protocols:(nullable NSArray<NSString *> *)protocols allowsUntrustedSSLCertificates:(BOOL)allowsUntrustedSSLCertificates
- (instancetype)initWithURL:(NSURL *)url protocols:(nullable NSArray<NSString *> *)protocols allowsUntrustedSSLCertificates:(BOOL)allowsUntrustedSSLCertificates logger:(nullable ARTLog *)logger
DEPRECATED_MSG_ATTRIBUTE("Disabling certificate chain validation is unsafe. "
"Please use a proper Certificate Authority to issue your TLS certificates.");

Expand Down
79 changes: 41 additions & 38 deletions SocketRocket/SocketRocket/ARTSRWebSocket.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ @interface ARTSRWebSocket () <NSStreamDelegate>

@property (nonatomic, strong, readonly) ARTSRDelegateController *delegateController;

@property (nonatomic, strong, readonly, nullable) ARTLog * logger;

@end

@implementation ARTSRWebSocket {
Expand Down Expand Up @@ -147,7 +149,7 @@ @implementation ARTSRWebSocket {
#pragma mark - Init
///--------------------------------------

- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(NSArray<NSString *> *)protocols securityPolicy:(ARTSRSecurityPolicy *)securityPolicy
- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(NSArray<NSString *> *)protocols securityPolicy:(ARTSRSecurityPolicy *)securityPolicy logger:(nullable ARTLog *)logger
{
self = [super init];
if (!self) return self;
Expand All @@ -158,6 +160,7 @@ - (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(NSArray<NS
_requestedProtocols = [protocols copy];
_securityPolicy = securityPolicy;
_requestRequiresSSL = ARTSRURLRequiresSSL(_url);
_logger = logger;

_readyState = ARTSR_CONNECTING;

Expand All @@ -184,7 +187,7 @@ - (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(NSArray<NS
return self;
}

- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(NSArray<NSString *> *)protocols allowsUntrustedSSLCertificates:(BOOL)allowsUntrustedSSLCertificates
- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(NSArray<NSString *> *)protocols allowsUntrustedSSLCertificates:(BOOL)allowsUntrustedSSLCertificates logger:(nullable ARTLog *)logger
{
ARTSRSecurityPolicy *securityPolicy;
BOOL certificateChainValidationEnabled = !allowsUntrustedSSLCertificates;
Expand All @@ -196,54 +199,54 @@ - (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(NSArray<NS

#pragma clang diagnostic pop

return [self initWithURLRequest:request protocols:protocols securityPolicy:securityPolicy];
return [self initWithURLRequest:request protocols:protocols securityPolicy:securityPolicy logger:logger];
}

- (instancetype)initWithURLRequest:(NSURLRequest *)request securityPolicy:(ARTSRSecurityPolicy *)securityPolicy
- (instancetype)initWithURLRequest:(NSURLRequest *)request securityPolicy:(ARTSRSecurityPolicy *)securityPolicy logger:(nullable ARTLog *)logger
{
return [self initWithURLRequest:request protocols:nil securityPolicy:securityPolicy];
return [self initWithURLRequest:request protocols:nil securityPolicy:securityPolicy logger:logger];
}

- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(NSArray<NSString *> *)protocols
- (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(NSArray<NSString *> *)protocols logger:(nullable ARTLog *)logger
{
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated"

return [self initWithURLRequest:request protocols:protocols allowsUntrustedSSLCertificates:NO];
return [self initWithURLRequest:request protocols:protocols allowsUntrustedSSLCertificates:NO logger:logger];

#pragma clang diagnostic pop
}

- (instancetype)initWithURLRequest:(NSURLRequest *)request
- (instancetype)initWithURLRequest:(NSURLRequest *)request logger:(nullable ARTLog *)logger
{
return [self initWithURLRequest:request protocols:nil];
return [self initWithURLRequest:request protocols:nil logger:logger];
}

- (instancetype)initWithURL:(NSURL *)url;
- (instancetype)initWithURL:(NSURL *)url logger:(nullable ARTLog *)logger;
{
return [self initWithURL:url protocols:nil];
return [self initWithURL:url protocols:nil logger:logger];
}

- (instancetype)initWithURL:(NSURL *)url protocols:(NSArray<NSString *> *)protocols;
- (instancetype)initWithURL:(NSURL *)url protocols:(NSArray<NSString *> *)protocols logger:(nullable ARTLog *)logger;
{
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated"

return [self initWithURL:url protocols:protocols allowsUntrustedSSLCertificates:NO];
return [self initWithURL:url protocols:protocols allowsUntrustedSSLCertificates:NO logger:logger];

#pragma clang diagnostic pop
}

- (instancetype)initWithURL:(NSURL *)url securityPolicy:(ARTSRSecurityPolicy *)securityPolicy
- (instancetype)initWithURL:(NSURL *)url securityPolicy:(ARTSRSecurityPolicy *)securityPolicy logger:(nullable ARTLog *)logger
{
NSURLRequest *request = [NSURLRequest requestWithURL:url];
return [self initWithURLRequest:request protocols:nil securityPolicy:securityPolicy];
return [self initWithURLRequest:request protocols:nil securityPolicy:securityPolicy logger:logger];
}

- (instancetype)initWithURL:(NSURL *)url protocols:(NSArray<NSString *> *)protocols allowsUntrustedSSLCertificates:(BOOL)allowsUntrustedSSLCertificates
- (instancetype)initWithURL:(NSURL *)url protocols:(NSArray<NSString *> *)protocols allowsUntrustedSSLCertificates:(BOOL)allowsUntrustedSSLCertificates logger:(nullable ARTLog *)logger
{
NSURLRequest *request = [NSURLRequest requestWithURL:url];
return [self initWithURLRequest:request protocols:protocols allowsUntrustedSSLCertificates:allowsUntrustedSSLCertificates];
return [self initWithURLRequest:request protocols:protocols allowsUntrustedSSLCertificates:allowsUntrustedSSLCertificates logger:logger];
}

- (void)assertOnWorkQueue;
Expand Down Expand Up @@ -328,7 +331,7 @@ - (void)open
});
}

_proxyConnect = [[ARTSRProxyConnect alloc] initWithURL:_url];
_proxyConnect = [[ARTSRProxyConnect alloc] initWithURL:_url logger:self.logger];

__weak typeof(self) wself = self;
[_proxyConnect openNetworkStreamWithCompletion:^(NSError *error, NSInputStream *readStream, NSOutputStream *writeStream) {
Expand Down Expand Up @@ -385,7 +388,7 @@ - (void)_HTTPHeadersDidFinish;
{
NSInteger responseCode = CFHTTPMessageGetResponseStatusCode(_receivedHTTPHeaders);
if (responseCode >= 400) {
ARTSRDebugLog(@"Request failed with response code %d", responseCode);
ARTSRDebugLog(self.logger, @"Request failed with response code %d", responseCode);
NSError *error = ARTSRHTTPErrorWithCodeDescription(responseCode, 2132,
[NSString stringWithFormat:@"Received bad response code from server: %d.",
(int)responseCode]);
Expand Down Expand Up @@ -435,7 +438,7 @@ - (void)_readHTTPHeader;
CFHTTPMessageAppendBytes(self->_receivedHTTPHeaders, (const UInt8 *)data.bytes, data.length);

if (CFHTTPMessageIsHeaderComplete(self->_receivedHTTPHeaders)) {
ARTSRDebugLog(@"Finished reading headers %@", CFBridgingRelease(CFHTTPMessageCopyAllHeaderFields(self->_receivedHTTPHeaders)));
ARTSRDebugLog(self.logger, @"Finished reading headers %@", CFBridgingRelease(CFHTTPMessageCopyAllHeaderFields(self->_receivedHTTPHeaders)));
[self _HTTPHeadersDidFinish];
} else {
[self _readHTTPHeader];
Expand All @@ -445,7 +448,7 @@ - (void)_readHTTPHeader;

- (void)didConnect;
{
ARTSRDebugLog(@"Connected");
ARTSRDebugLog(self.logger, @"Connected");

_secKey = ARTSRBase64EncodedStringFromData(ARTSRRandomData(16));
assert([_secKey length] == 24);
Expand All @@ -467,7 +470,7 @@ - (void)didConnect;
- (void)_updateSecureStreamOptions
{
if (_requestRequiresSSL) {
ARTSRDebugLog(@"Setting up security for streams.");
ARTSRDebugLog(self.logger, @"Setting up security for streams.");
[_securityPolicy updateSecurityOptionsInStream:_inputStream];
[_securityPolicy updateSecurityOptionsInStream:_outputStream];
}
Expand Down Expand Up @@ -512,7 +515,7 @@ - (void)closeWithCode:(NSInteger)code reason:(NSString *)reason;

self.readyState = ARTSR_CLOSING;

ARTSRDebugLog(@"Closing with code %d reason %@", code, reason);
ARTSRDebugLog(self.logger, @"Closing with code %d reason %@", code, reason);

if (wasConnecting) {
[self closeConnection];
Expand Down Expand Up @@ -570,7 +573,7 @@ - (void)_failWithError:(NSError *)error;

self.readyState = ARTSR_CLOSED;

ARTSRDebugLog(@"Failing with error %@", error.localizedDescription);
ARTSRDebugLog(self.logger, @"Failing with error %@", error.localizedDescription);

[self closeConnection];
[self _scheduleCleanup];
Expand Down Expand Up @@ -614,7 +617,7 @@ - (BOOL)sendString:(NSString *)string error:(NSError **)error
if (error) {
*error = ARTSRErrorWithCodeDescription(2134, message);
}
ARTSRDebugLog(message);
ARTSRDebugLog(self.logger, message);
return NO;
}

Expand All @@ -638,7 +641,7 @@ - (BOOL)sendDataNoCopy:(nullable NSData *)data error:(NSError **)error
if (error) {
*error = ARTSRErrorWithCodeDescription(2134, message);
}
ARTSRDebugLog(message);
ARTSRDebugLog(self.logger, message);
return NO;
}

Expand All @@ -659,7 +662,7 @@ - (BOOL)sendPing:(nullable NSData *)data error:(NSError **)error
if (error) {
*error = ARTSRErrorWithCodeDescription(2134, message);
}
ARTSRDebugLog(message);
ARTSRDebugLog(self.logger, message);
return NO;
}

Expand All @@ -685,7 +688,7 @@ - (void)_handlePingWithData:(nullable NSData *)data

- (void)handlePong:(NSData *)pongData;
{
ARTSRDebugLog(@"Received pong");
ARTSRDebugLog(self.logger, @"Received pong");
[self.delegateController performDelegateBlock:^(id<ARTSRWebSocketDelegate> _Nullable delegate, ARTSRDelegateAvailableMethods availableMethods) {
if (availableMethods.didReceivePong) {
[delegate webSocket:self didReceivePong:pongData];
Expand Down Expand Up @@ -733,7 +736,7 @@ - (void)handleCloseWithData:(NSData *)data;
size_t dataSize = data.length;
__block uint16_t closeCode = 0;

ARTSRDebugLog(@"Received close frame");
ARTSRDebugLog(self.logger, @"Received close frame");

if (dataSize == 1) {
// TODO handle error
Expand Down Expand Up @@ -770,7 +773,7 @@ - (void)handleCloseWithData:(NSData *)data;
- (void)closeConnection;
{
[self assertOnWorkQueue];
ARTSRDebugLog(@"Trying to disconnect");
ARTSRDebugLog(self.logger, @"Trying to disconnect");
_closeWhenFinishedWriting = YES;
[self _pumpWriting];
}
Expand Down Expand Up @@ -802,7 +805,7 @@ - (void)_handleFrameWithData:(NSData *)frameData opCode:(ARTSROpCode)opcode
});
return;
}
ARTSRDebugLog(@"Received text message.");
ARTSRDebugLog(self.logger, @"Received text message.");
[self.delegateController performDelegateBlock:^(id<ARTSRWebSocketDelegate> _Nullable delegate, ARTSRDelegateAvailableMethods availableMethods) {
// Don't convert into string - iff `delegate` tells us not to. Otherwise - create UTF8 string and handle that.
if (availableMethods.shouldConvertTextFrameToString && ![delegate webSocketShouldConvertTextFrameToString:self]) {
Expand All @@ -824,7 +827,7 @@ - (void)_handleFrameWithData:(NSData *)frameData opCode:(ARTSROpCode)opcode
break;
}
case ARTSROpCodeBinaryFrame: {
ARTSRDebugLog(@"Received data message.");
ARTSRDebugLog(self.logger, @"Received data message.");
[self.delegateController performDelegateBlock:^(id<ARTSRWebSocketDelegate> _Nullable delegate, ARTSRDelegateAvailableMethods availableMethods) {
if (availableMethods.didReceiveMessage) {
[delegate webSocket:self didReceiveMessage:frameData];
Expand Down Expand Up @@ -1434,7 +1437,7 @@ - (void)safeHandleEvent:(NSStreamEvent)eventCode stream:(NSStream *)aStream
{
switch (eventCode) {
case NSStreamEventOpenCompleted: {
ARTSRDebugLog(@"NSStreamEventOpenCompleted %@", aStream);
ARTSRDebugLog(self.logger, @"NSStreamEventOpenCompleted %@", aStream);
if (self.readyState >= ARTSR_CLOSING) {
return;
}
Expand All @@ -1451,7 +1454,7 @@ - (void)safeHandleEvent:(NSStreamEvent)eventCode stream:(NSStream *)aStream
}

case NSStreamEventErrorOccurred: {
ARTSRDebugLog(@"NSStreamEventErrorOccurred %@ %@", aStream, [[aStream streamError] copy]);
ARTSRDebugLog(self.logger, @"NSStreamEventErrorOccurred %@ %@", aStream, [[aStream streamError] copy]);
/// TODO specify error better!
[self _failWithError:aStream.streamError];
_readBufferOffset = 0;
Expand All @@ -1462,7 +1465,7 @@ - (void)safeHandleEvent:(NSStreamEvent)eventCode stream:(NSStream *)aStream

case NSStreamEventEndEncountered: {
[self _pumpScanner];
ARTSRDebugLog(@"NSStreamEventEndEncountered %@", aStream);
ARTSRDebugLog(self.logger, @"NSStreamEventEndEncountered %@", aStream);
if (aStream.streamError) {
[self _failWithError:aStream.streamError];
} else {
Expand Down Expand Up @@ -1491,7 +1494,7 @@ - (void)safeHandleEvent:(NSStreamEvent)eventCode stream:(NSStream *)aStream
}

case NSStreamEventHasBytesAvailable: {
ARTSRDebugLog(@"NSStreamEventHasBytesAvailable %@", aStream);
ARTSRDebugLog(self.logger, @"NSStreamEventHasBytesAvailable %@", aStream);
uint8_t buffer[ARTSRDefaultBufferSize()];

while (_inputStream.hasBytesAvailable) {
Expand All @@ -1514,13 +1517,13 @@ - (void)safeHandleEvent:(NSStreamEvent)eventCode stream:(NSStream *)aStream
}

case NSStreamEventHasSpaceAvailable: {
ARTSRDebugLog(@"NSStreamEventHasSpaceAvailable %@", aStream);
ARTSRDebugLog(self.logger, @"NSStreamEventHasSpaceAvailable %@", aStream);
[self _pumpWriting];
break;
}

case NSStreamEventNone:
ARTSRDebugLog(@"(default) %@", aStream);
ARTSRDebugLog(self.logger, @"(default) %@", aStream);
break;
}
}
Expand Down
Loading

0 comments on commit 527abca

Please sign in to comment.