From b7713043b15b26606b2d11fb183379881d7ad498 Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Thu, 4 Aug 2022 15:41:50 +0100 Subject: [PATCH] Fix an infinite loop when tapping an unjoined room alias. (#6522) * Fix a crash in universal link handling. * Fix an infinite loop when tapping an unjoined room alias. --- Riot/Modules/Application/LegacyAppDelegate.h | 4 +-- Riot/Modules/Application/LegacyAppDelegate.m | 26 +++++++------------ .../Common/Recents/RecentsViewController.m | 8 +++--- .../Home/GroupHomeViewController.m | 3 ++- Riot/Modules/Room/RoomViewController.m | 7 ++--- Riot/Utils/UniversalLink.h | 5 ++++ Riot/Utils/UniversalLink.m | 23 +++++++++++++--- changelog.d/6492.bugfix | 1 + 8 files changed, 46 insertions(+), 31 deletions(-) create mode 100644 changelog.d/6492.bugfix diff --git a/Riot/Modules/Application/LegacyAppDelegate.h b/Riot/Modules/Application/LegacyAppDelegate.h index 429fccb142..b4ed788525 100644 --- a/Riot/Modules/Application/LegacyAppDelegate.h +++ b/Riot/Modules/Application/LegacyAppDelegate.h @@ -234,10 +234,10 @@ UINavigationControllerDelegate Process the fragment part of a vector.im link. @param fragment the fragment part of the universal link. - @param universalLinkURL the unprocessed the universal link URL (optional). + @param universalLink the original universal link. @return YES in case of processing success. */ -- (BOOL)handleUniversalLinkFragment:(NSString*)fragment fromURL:(NSURL*)universalLinkURL; +- (BOOL)handleUniversalLinkFragment:(NSString*)fragment fromLink:(UniversalLink*)universalLink; /** Process the URL of a vector.im link. diff --git a/Riot/Modules/Application/LegacyAppDelegate.m b/Riot/Modules/Application/LegacyAppDelegate.m index 925897120d..7f42cb1889 100644 --- a/Riot/Modules/Application/LegacyAppDelegate.m +++ b/Riot/Modules/Application/LegacyAppDelegate.m @@ -1242,7 +1242,7 @@ - (BOOL)handleUniversalLink:(NSUserActivity*)userActivity // Continue the registration with the passed nextLink MXLogDebug(@"[AppDelegate] handleUniversalLink. Complete registration with nextLink"); NSURL *nextLink = [NSURL URLWithString:queryParams[@"nextLink"]]; - [self handleUniversalLinkFragment:nextLink.fragment fromURL:nextLink]; + [self handleUniversalLinkURL:nextLink]; } else { @@ -1295,8 +1295,6 @@ - (BOOL)handleUniversalLinkWithParameters:(UniversalLinkParameters*)universalLin BOOL continueUserActivity = NO; MXKAccountManager *accountManager = [MXKAccountManager sharedManager]; - MXLogDebug(@"[AppDelegate] Universal link: handleUniversalLinkFragment: %@", fragment); - // Make sure we have plain utf8 character for separators fragment = [fragment stringByRemovingPercentEncoding]; MXLogDebug(@"[AppDelegate] Universal link: handleUniversalLinkFragment: %@", fragment); @@ -1312,17 +1310,8 @@ - (BOOL)handleUniversalLinkWithParameters:(UniversalLinkParameters*)universalLin // Sanity check if (!pathParams.count) { - // Handle simple room links with aliases/identifiers as UniversalLink will not parse these. - NSString* absoluteUrl = [universalLink.url.absoluteString stringByRemovingPercentEncoding]; - if ([MXTools isMatrixRoomAlias:absoluteUrl] - || [MXTools isMatrixRoomIdentifier:absoluteUrl]) - { - pathParams = @[absoluteUrl]; - } - else { - MXLogDebug(@"[AppDelegate] Universal link: Error: No path parameters"); - return NO; - } + MXLogFailure(@"[AppDelegate] Universal link: Error: No path parameters"); + return NO; } NSString *roomIdOrAlias; @@ -1498,9 +1487,11 @@ - (BOOL)handleUniversalLinkWithParameters:(UniversalLinkParameters*)universalLin if (newFragment && ![newFragment isEqualToString:fragment]) { self->universalLinkFragmentPendingRoomAlias = @{resolution.roomId: roomIdOrAlias}; - + + // Create a new link with the updated fragment, otherwise we loop back round resolving the room ID infinitely. + UniversalLink *newLink = [[UniversalLink alloc] initWithUrl:universalLink.url updatedFragment:newFragment]; UniversalLinkParameters *newParameters = [[UniversalLinkParameters alloc] initWithFragment:newFragment - universalLink:universalLink + universalLink:newLink presentationParameters:presentationParameters]; [self handleUniversalLinkWithParameters:newParameters]; } @@ -1704,8 +1695,9 @@ - (BOOL)handleUniversalLinkURL:(NSURL*)universalLinkURL { // iOS Patch: fix vector.im urls before using it NSURL *fixedURL = [Tools fixURLWithSeveralHashKeys:universalLinkURL]; + UniversalLink *link = [[UniversalLink alloc] initWithUrl:universalLinkURL]; - return [self handleUniversalLinkFragment:fixedURL.fragment fromURL:universalLinkURL]; + return [self handleUniversalLinkFragment:fixedURL.fragment fromLink:link]; } - (void)resetPendingUniversalLink diff --git a/Riot/Modules/Common/Recents/RecentsViewController.m b/Riot/Modules/Common/Recents/RecentsViewController.m index 28a6beca12..aeacb789e1 100644 --- a/Riot/Modules/Common/Recents/RecentsViewController.m +++ b/Riot/Modules/Common/Recents/RecentsViewController.m @@ -1602,10 +1602,10 @@ - (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath if (roomIdOrAlias.length) { - // Open the room or preview it - NSString *fragment = [NSString stringWithFormat:@"/room/%@", [MXTools encodeURIComponent:roomIdOrAlias]]; - NSURL *url = [NSURL URLWithString:[MXTools permalinkToRoom:fragment]]; - [[AppDelegate theDelegate] handleUniversalLinkFragment:fragment fromURL:url]; + // Create a permalink to open or preview the room. + NSString *permalink = [MXTools permalinkToRoom:roomIdOrAlias]; + NSURL *permalinkURL = [NSURL URLWithString:permalink]; + [[AppDelegate theDelegate] handleUniversalLinkURL:permalinkURL]; } [tableView deselectRowAtIndexPath:indexPath animated:NO]; } diff --git a/Riot/Modules/Communities/Home/GroupHomeViewController.m b/Riot/Modules/Communities/Home/GroupHomeViewController.m index 051ee78805..1821eff53d 100644 --- a/Riot/Modules/Communities/Home/GroupHomeViewController.m +++ b/Riot/Modules/Communities/Home/GroupHomeViewController.m @@ -892,7 +892,8 @@ - (BOOL)textView:(UITextView *)textView shouldInteractWithURL:(NSURL *)URL inRan // Open the group or preview it NSString *fragment = [NSString stringWithFormat:@"/group/%@", [MXTools encodeURIComponent:absoluteURLString]]; - [[AppDelegate theDelegate] handleUniversalLinkFragment:fragment fromURL:URL]; + UniversalLink *link = [[UniversalLink alloc] initWithUrl:URL]; + [[AppDelegate theDelegate] handleUniversalLinkFragment:fragment fromLink:link]; } return shouldInteractWithURL; diff --git a/Riot/Modules/Room/RoomViewController.m b/Riot/Modules/Room/RoomViewController.m index cf4b80b8c5..abfe72807b 100644 --- a/Riot/Modules/Room/RoomViewController.m +++ b/Riot/Modules/Room/RoomViewController.m @@ -4279,10 +4279,11 @@ - (BOOL)dataSource:(MXKDataSource *)dataSource shouldDoAction:(NSString *)action NSString *roomIdOrAlias = absoluteURLString; - // Open the room or preview it - NSString *fragment = [NSString stringWithFormat:@"/room/%@", [MXTools encodeURIComponent:roomIdOrAlias]]; + // Create a permalink to open or preview the room. + NSString *permalink = [MXTools permalinkToRoom:roomIdOrAlias]; + NSURL *permalinkURL = [NSURL URLWithString:permalink]; - [self handleUniversalLinkFragment:fragment fromURL:url]; + [self handleUniversalLinkURL:permalinkURL]; } // Preview the clicked group else if ([MXTools isMatrixGroupIdentifier:absoluteURLString]) diff --git a/Riot/Utils/UniversalLink.h b/Riot/Utils/UniversalLink.h index bb7c629526..a5ea787635 100644 --- a/Riot/Utils/UniversalLink.h +++ b/Riot/Utils/UniversalLink.h @@ -40,6 +40,11 @@ NS_ASSUME_NONNULL_BEGIN /// @param url original url - (id)initWithUrl:(NSURL *)url; +/// An Initializer that preserves the original URL, but parses the parameters from an updated fragment. +/// @param url original url +/// @param fragment the updated fragment to parse. +- (id)initWithUrl:(NSURL *)url updatedFragment:(NSString *)fragment; + @end NS_ASSUME_NONNULL_END diff --git a/Riot/Utils/UniversalLink.m b/Riot/Utils/UniversalLink.m index edd73c7b85..98d3802dfb 100644 --- a/Riot/Utils/UniversalLink.m +++ b/Riot/Utils/UniversalLink.m @@ -27,7 +27,22 @@ - (id)initWithUrl:(NSURL *)url _url = url; // Extract required parameters from the link - [self parsePathAndQueryParams]; + [self parsePathAndQueryParamsForURL:url]; + } + return self; +} + +- (id)initWithUrl:(NSURL *)url updatedFragment:(NSString *)fragment +{ + self = [super init]; + if (self) + { + _url = url; + + // Update the url with the fragment + NSURLComponents *components = [[NSURLComponents alloc] initWithURL:url resolvingAgainstBaseURL:NO]; + components.fragment = fragment; + [self parsePathAndQueryParamsForURL:components.URL]; } return self; } @@ -38,12 +53,12 @@ Extract params from the URL fragment part (after '#') of a vector.im Universal l The fragment can contain a '?'. So there are two kinds of parameters: path params and query params. It is in the form of /[pathParam1]/[pathParam2]?[queryParam1Key]=[queryParam1Value]&[queryParam2Key]=[queryParam2Value] */ -- (void)parsePathAndQueryParams +- (void)parsePathAndQueryParamsForURL:(NSURL *)url { NSArray *pathParams; NSMutableDictionary *queryParams = [NSMutableDictionary dictionary]; - NSArray *fragments = [_url.fragment componentsSeparatedByString:@"?"]; + NSArray *fragments = [url.fragment componentsSeparatedByString:@"?"]; // Extract path params pathParams = [[fragments[0] stringByRemovingPercentEncoding] componentsSeparatedByString:@"/"]; @@ -57,7 +72,7 @@ - (void)parsePathAndQueryParams }]; // Extract query params - NSURLComponents *components = [NSURLComponents componentsWithURL:_url resolvingAgainstBaseURL:NO]; + NSURLComponents *components = [NSURLComponents componentsWithURL:url resolvingAgainstBaseURL:NO]; for (NSURLQueryItem *item in components.queryItems) { if (item.value) diff --git a/changelog.d/6492.bugfix b/changelog.d/6492.bugfix new file mode 100644 index 0000000000..563447802d --- /dev/null +++ b/changelog.d/6492.bugfix @@ -0,0 +1 @@ +Universal Links: Fix an infinite loop when handling a universal link for an unjoined room (or in some cases a crash).