From 3615757801283f0cb93976e2231c9f1f6044f018 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Tue, 18 Nov 2025 17:53:07 +0000 Subject: [PATCH] Avoid parsing an http body error if response is not from Ably. --- Source/ARTJsonLikeEncoder.m | 11 +-- Source/ARTRealtime.m | 27 ++++---- Source/ARTRest.m | 67 ++++++++++++------- Source/ARTStatus.m | 30 +++++---- .../Ably/ARTErrorInfo+Private.h | 3 + Source/include/Ably/ARTStatus.h | 5 +- Test/AblyTests/Tests/AuthTests.swift | 21 ++++++ Test/AblyTests/Tests/UtilitiesTests.swift | 9 ++- 8 files changed, 114 insertions(+), 59 deletions(-) diff --git a/Source/ARTJsonLikeEncoder.m b/Source/ARTJsonLikeEncoder.m index b64fc2280..fb041d107 100644 --- a/Source/ARTJsonLikeEncoder.m +++ b/Source/ARTJsonLikeEncoder.m @@ -1129,11 +1129,12 @@ - (ARTErrorInfo *)decodeErrorInfo:(NSData *)artError statusCode:(NSInteger)statu status:[(statusNumber ?: @(statusCode)) intValue] message:message ?: [NSString stringWithFormat:@"HTTP request failed with status code %ld", statusCode]]; } else { - // We expect `decodedError` as a dictionary from Ably REST API, but in case user sets custom authUrl in the auth options, it can be anything - // We'll address this in an upcoming proper fix for https://github.com/ably/ably-cocoa/issues/2135 - return [ARTErrorInfo createWithCode:statusCode * 100 - status:statusCode - message:[NSString stringWithFormat:@"HTTP request failed with status code %ld", statusCode]]; + // We expect `decodedError` as a dictionary from Ably REST API + // In case it's something else we set the decoding error + *error = [ARTErrorInfo createWithCode:ARTClientCodeErrorInvalidType + status:0 + message:[NSString stringWithFormat:@"Failed to decode error dictionary from the provided error data."]]; + return nil; } } diff --git a/Source/ARTRealtime.m b/Source/ARTRealtime.m index 4296640ff..373406088 100644 --- a/Source/ARTRealtime.m +++ b/Source/ARTRealtime.m @@ -1,13 +1,8 @@ -// -// ARTRealtime.m -// -// - #import "ARTRealtime+Private.h" #import "ARTRealtime+WrapperSDKProxy.h" - #import "ARTRealtimeChannel+Private.h" #import "ARTStatus.h" +#import "ARTErrorInfo+Private.h" #import "ARTDefault.h" #import "ARTRest+Private.h" #import "ARTAuth+Private.h" @@ -1073,10 +1068,14 @@ - (void)onConnectionTimeOut { ARTErrorInfo *error; if (self.auth.authorizing_nosync && (self.options.authUrl || self.options.authCallback)) { - error = [ARTErrorInfo createWithCode:ARTErrorAuthConfiguredProviderFailure status:ARTStateConnectionFailed message:@"timed out"]; + error = [ARTErrorInfo createWithCode:ARTErrorAuthConfiguredProviderFailure + status:ARTHttpStatusCodeFromErrorCode(ARTErrorUnauthorized) // RSA4c1 + message:@"timed out"]; } else { - error = [ARTErrorInfo createWithCode:ARTErrorConnectionTimedOut status:ARTStateConnectionFailed message:@"timed out"]; + error = [ARTErrorInfo createWithCode:ARTErrorConnectionTimedOut + status:ARTStateConnectionFailed + message:@"timed out"]; } switch (self.connection.state_nosync) { case ARTRealtimeConnected: { @@ -1195,19 +1194,23 @@ - (void)handleTokenAuthError:(NSError *)error { else if (self.options.authUrl || self.options.authCallback) { if (error.code == ARTErrorForbidden /* RSA4d */) { ARTErrorInfo *errorInfo = [ARTErrorInfo createWithCode:ARTErrorAuthConfiguredProviderFailure - status:error.artStatusCode - message:error.description]; + status:ARTHttpStatusCodeFromErrorCode(ARTErrorForbidden) + message:error.description + underlyingError:error]; ARTConnectionStateChangeParams *const params = [[ARTConnectionStateChangeParams alloc] initWithErrorInfo:errorInfo]; [self performTransitionToState:ARTRealtimeFailed withParams:params]; } else { - ARTErrorInfo *errorInfo = [ARTErrorInfo createWithCode:ARTErrorAuthConfiguredProviderFailure status:ARTStateConnectionFailed message:error.description]; + ARTErrorInfo *errorInfo = [ARTErrorInfo createWithCode:ARTErrorAuthConfiguredProviderFailure + status:ARTHttpStatusCodeFromErrorCode(ARTErrorUnauthorized) + message:error.description + underlyingError:error]; switch (self.connection.state_nosync) { case ARTRealtimeConnected: // RSA4c3 [self.connection setErrorReason:errorInfo]; break; default: { - // RSA4c + // RSA4c1, RSA4c2 ARTConnectionStateChangeParams *const params = [[ARTConnectionStateChangeParams alloc] initWithErrorInfo:errorInfo]; [self performTransitionToDisconnectedOrSuspendedWithParams:params]; break; diff --git a/Source/ARTRest.m b/Source/ARTRest.m index b17792a3e..2b7d3b9e4 100644 --- a/Source/ARTRest.m +++ b/Source/ARTRest.m @@ -1,5 +1,5 @@ #import "ARTRest+Private.h" - +#import "ARTErrorInfo+Private.h" #import "ARTChannel+Private.h" #import "ARTRestChannels+Private.h" #import "ARTDataQuery+Private.h" @@ -335,6 +335,32 @@ - (NSString *)agentIdentifierWithWrapperSDKAgents:(nullable NSDictionary_encoders.allValues valueForKeyPath:@"mimeType"]) { + if ([contentType containsString:mimeType]) { + return true; + } + } + return false; +} + +// Checks if the auth request was made with the user's authUrl +- (BOOL)isCustomAuthRequest:(NSURLRequest *)request { + return [request.URL.host isEqualToString:self.options.authUrl.host]; +} + /** originalRequestId is used only for fallback requests. It should never be used to execute request by yourself, it's passed from within below method. */ @@ -383,40 +409,34 @@ - (NSString *)agentIdentifierWithWrapperSDKAgents:(nullable NSDictionary *task; task = [self.httpExecutor executeRequest:request completion:^(NSHTTPURLResponse *response, NSData *data, NSError *error) { // Error messages in plaintext and HTML format (only if the URL request is different than `options.authUrl` and we don't have an error already) - if (error == nil && data != nil && data.length != 0 && ![request.URL.host isEqualToString:[self.options.authUrl host]]) { - NSString *contentType = [response.allHeaderFields objectForKey:@"Content-Type"]; - - BOOL validContentType = NO; - for (NSString *mimeType in [self->_encoders.allValues valueForKeyPath:@"mimeType"]) { - if ([contentType containsString:mimeType]) { - validContentType = YES; - break; - } - } - - if (!validContentType) { - NSString *plain = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; + if (error == nil && data != nil && data.length != 0 && ![self isCustomAuthRequest:request]) { + if (![self hasValidContentType:response]) { // Construct artificial error + NSString *plain = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; error = [ARTErrorInfo createWithCode:response.statusCode * 100 status:response.statusCode - message:[plain art_shortString] requestId:requestId]; + message:[plain art_shortString] + requestId:requestId]; data = nil; // Discard data; format is unreliable. ARTLogError(self.logger, @"Request %@ failed with %@", request, error); } } if (response.statusCode >= 400) { - if (data) { + if (data && data.length != 0) { NSError *decodeError = nil; - ARTErrorInfo *dataError = [self->_encoders[response.MIMEType] decodeErrorInfo:data - statusCode:response.statusCode - error:&decodeError]; - if ([self shouldRenewToken:&dataError] && [request isKindOfClass:[NSMutableURLRequest class]]) { + ARTErrorInfo *dataError = nil; + id encoder = self->_encoders[response.MIMEType]; + + if (encoder && [self isAblyResponse:response]) { + dataError = [encoder decodeErrorInfo:data statusCode:response.statusCode error:&decodeError]; + } + + if (dataError && [self shouldRenewToken:&dataError] && [request isKindOfClass:[NSMutableURLRequest class]]) { ARTLogDebug(self.logger, @"RS:%p retry request %@", self, request); // Make a single attempt to reissue the token and resend the request if (self->_tokenErrorRetries < 1) { @@ -432,10 +452,11 @@ - (NSString *)agentIdentifierWithWrapperSDKAgents:(nullable NSDictionary *)additionalUserInfo { - return [ARTErrorInfo createWithCode:code status:getStatusFromCode(code) message:message requestId:requestId additionalUserInfo:additionalUserInfo]; + return [ARTErrorInfo createWithCode:code status:ARTHttpStatusCodeFromErrorCode(code) message:message requestId:requestId additionalUserInfo:additionalUserInfo underlyingError:nil]; } + (ARTErrorInfo *)createWithCode:(NSInteger)code message:(NSString *)message { - return [ARTErrorInfo createWithCode:code status:getStatusFromCode(code) message:message requestId:nil]; + return [ARTErrorInfo createWithCode:code status:ARTHttpStatusCodeFromErrorCode(code) message:message requestId:nil]; +} + ++ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message underlyingError:(NSError *)underlyingError { + return [ARTErrorInfo createWithCode:code status:status message:message requestId:nil additionalUserInfo:nil underlyingError:underlyingError]; } + (ARTErrorInfo *)createWithCode:(NSInteger)code message:(NSString *)message additionalUserInfo:(nullable NSDictionary *)additionalUserInfo { - return [ARTErrorInfo createWithCode:code status:getStatusFromCode(code) message:message requestId:nil additionalUserInfo:additionalUserInfo]; + return [ARTErrorInfo createWithCode:code status:ARTHttpStatusCodeFromErrorCode(code) message:message requestId:nil additionalUserInfo:additionalUserInfo underlyingError:nil]; } + (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message additionalUserInfo:(nullable NSDictionary *)additionalUserInfo { - return [ARTErrorInfo createWithCode:code status:status message:message requestId:nil additionalUserInfo:additionalUserInfo]; + return [ARTErrorInfo createWithCode:code status:status message:message requestId:nil additionalUserInfo:additionalUserInfo underlyingError:nil]; } + (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId { - return [ARTErrorInfo createWithCode:code status:status message:message requestId:requestId additionalUserInfo:nil]; + return [ARTErrorInfo createWithCode:code status:status message:message requestId:requestId additionalUserInfo:nil underlyingError:nil]; } -+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId additionalUserInfo:(nullable NSDictionary *)additionalUserInfo { ++ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId additionalUserInfo:(nullable NSDictionary *)additionalUserInfo underlyingError:(NSError *)underlyingError { NSMutableDictionary *userInfo = [NSMutableDictionary new]; userInfo[ARTErrorInfoStatusCodeKey] = [NSNumber numberWithInteger:status]; userInfo[NSLocalizedDescriptionKey] = message; userInfo[ARTErrorInfoRequestIdKey] = requestId; + userInfo[NSUnderlyingErrorKey] = underlyingError; // Add any additional userInfo values if (additionalUserInfo) { @@ -132,9 +137,9 @@ - (NSInteger)statusCode { - (NSString *)description { if (self.reason != nil) { - return [NSString stringWithFormat:@"Error %ld - %@ (reason: %@)", (long)self.code, self.message ?: @"", self.reason]; + return [NSString stringWithFormat:@"Error %ld (status: %ld) - %@ (reason: %@)", (long)self.code, (long)self.statusCode, self.message ?: @"", self.reason]; } else { - return [NSString stringWithFormat:@"Error %ld - %@", (long)self.code, self.message ?: @""]; + return [NSString stringWithFormat:@"Error %ld (status: %ld) - %@", (long)self.code, (long)self.statusCode, self.message ?: @""]; } } @@ -153,7 +158,6 @@ - (NSString *)requestId { @end - @implementation ARTStatus - (instancetype)init { @@ -184,7 +188,7 @@ - (NSString *)description { #pragma mark private --(void) setErrorInfo:(ARTErrorInfo *)errorInfo { +- (void)setErrorInfo:(ARTErrorInfo *)errorInfo { _errorInfo = errorInfo; } diff --git a/Source/PrivateHeaders/Ably/ARTErrorInfo+Private.h b/Source/PrivateHeaders/Ably/ARTErrorInfo+Private.h index 436a68c75..826c4c2aa 100644 --- a/Source/PrivateHeaders/Ably/ARTErrorInfo+Private.h +++ b/Source/PrivateHeaders/Ably/ARTErrorInfo+Private.h @@ -12,4 +12,7 @@ NS_ASSUME_NONNULL_BEGIN @end #endif +// Gets an HTTP status code from error code by taking first three digits. Doesn't perform any checks if the result is a valid status code. +NSInteger ARTHttpStatusCodeFromErrorCode(ARTErrorCode errorCode); + NS_ASSUME_NONNULL_END diff --git a/Source/include/Ably/ARTStatus.h b/Source/include/Ably/ARTStatus.h index 5933ceff3..d96afca5d 100644 --- a/Source/include/Ably/ARTStatus.h +++ b/Source/include/Ably/ARTStatus.h @@ -231,6 +231,9 @@ FOUNDATION_EXPORT NSString *const ARTAblyMessageNoMeansToRenewToken; /// :nodoc: + (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message; +/// :nodoc: ++ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(nullable NSString *)message underlyingError:(nullable NSError *)error; + /// :nodoc: + (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message additionalUserInfo:(nullable NSDictionary *)additionalUserInfo; @@ -250,7 +253,7 @@ FOUNDATION_EXPORT NSString *const ARTAblyMessageNoMeansToRenewToken; + (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId; /// :nodoc: -+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId additionalUserInfo:(nullable NSDictionary *)additionalUserInfo; ++ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId additionalUserInfo:(nullable NSDictionary *)additionalUserInfo underlyingError:(nullable NSError *)underlyingError; /// :nodoc: + (ARTErrorInfo *)createFromNSException:(NSException *)error requestId:(nullable NSString *)requestId; diff --git a/Test/AblyTests/Tests/AuthTests.swift b/Test/AblyTests/Tests/AuthTests.swift index ad53c2653..a98941a3f 100644 --- a/Test/AblyTests/Tests/AuthTests.swift +++ b/Test/AblyTests/Tests/AuthTests.swift @@ -875,6 +875,27 @@ class AuthTests: XCTestCase { XCTAssertEqual(realtime.connection.state, ARTRealtimeConnectionState.connected) } + + // RSA4c2 (with 404, see https://github.com/ably/ably-cocoa/issues/2135) + func test__040__Token__authentication_method__if_a_request_by_a_realtime_client_to_an_authUrl_results_in_an_HTTP_404_the_client_library_should_transition_to_the_DISCONNECTED_state() throws { + let test = Test() + let options = try AblyTests.clientOptions(for: test) + options.autoConnect = false + options.authUrl = URL(string: "https://api.restful-api.dev/objects/\(UUID())")! // random address to get 404 + let realtime = ARTRealtime(options: options) + defer { realtime.dispose(); realtime.close() } + + waitUntil(timeout: testTimeout) { done in + realtime.connection.once(.disconnected) { stateChange in + XCTAssertEqual(stateChange.reason?.code, ARTErrorCode.authConfiguredProviderFailure.intValue) + XCTAssertEqual(stateChange.reason?.statusCode, 401) + XCTAssertEqual(stateChange.reason?.cause?.statusCode, 404) + XCTAssertEqual(realtime.connection.errorReason, stateChange.reason) + done() + } + realtime.connect() + } + } // RSA15 diff --git a/Test/AblyTests/Tests/UtilitiesTests.swift b/Test/AblyTests/Tests/UtilitiesTests.swift index 30837dc62..504b492ce 100644 --- a/Test/AblyTests/Tests/UtilitiesTests.swift +++ b/Test/AblyTests/Tests/UtilitiesTests.swift @@ -239,7 +239,7 @@ class UtilitiesTests: XCTestCase { } } - func test__Utilities__JSON_Encoder__should_decode_rest_error_response_with_only_error_field() throws { + func test__Utilities__JSON_Encoder__should_not_decode_rest_http_error_if_it_doesnt_contains_error_dictionary_under_error_key() throws { let test = Test() beforeEach__Utilities__JSON_Encoder() @@ -263,15 +263,14 @@ class UtilitiesTests: XCTestCase { guard let error = error as? ARTErrorInfo else { fail("Should be ARTErrorInfo"); done(); return } - XCTAssertTrue(error.code == 40400) - XCTAssertTrue(error.statusCode == 404) - XCTAssertTrue(error.message == "HTTP request failed with status code 404") + XCTAssertTrue(error.code == ARTClientCodeError.invalidType.rawValue) + XCTAssertTrue(error.message == "Failed to decode error dictionary from the provided error data.") done() }) } } - func test__Utilities__JSON_Encoder__should_decode_rest_error_response_with_complete_error_info() throws { + func test__Utilities__JSON_Encoder__should_decode_rest_http_error_if_it_contains_error_dictionary_under_error_key() throws { let test = Test() beforeEach__Utilities__JSON_Encoder()