From 5e983d51d8bc2abded5659a77808542c6dc1377a Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Tue, 25 Apr 2023 04:43:21 -0700 Subject: [PATCH] Reapply Fix escaping in the URL conversion (#36949) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36949 This change is a second attempt at fixing URL encoding and escaping that was already tried [here](https://github.com/facebook/react-native/commit/2b4e1f5ece7d160935b19d4862af8706a44cee59). We had to roll it back due to some internal tests failing as it looks like Jest is manipulating the URL somehow. We manage to replicate the issue, which occur when we pre-decode a url even if it is not partially encoded (we were too aggrsssive). This fix ensure that we pre-decode the urls only if they present some `%` characters. The problem here was that the e2e tests sends some urls with some `%` symbol which does not belongs to an escape sequence. For example: `anna://launch?height=25%`. The previous code (v1) was trying to unescape this case. V2 fixes this. This change should also fix #28508 for good. ## Changelog: [iOS][Fixed] - Properly escape URLs Reviewed By: mdvacca Differential Revision: D45078923 fbshipit-source-id: 010a5c173784f8341a1a08bcbd06a6ad14299c75 --- packages/react-native/React/Base/RCTConvert.m | 61 ++++++++++++------- .../RNTesterUnitTests/RCTConvert_NSURLTests.m | 40 ++++++++++++ 2 files changed, 79 insertions(+), 22 deletions(-) diff --git a/packages/react-native/React/Base/RCTConvert.m b/packages/react-native/React/Base/RCTConvert.m index 600ff3fff47335..0d76283e48be6e 100644 --- a/packages/react-native/React/Base/RCTConvert.m +++ b/packages/react-native/React/Base/RCTConvert.m @@ -83,30 +83,13 @@ + (NSURL *)NSURL:(id)json return nil; } - @try { // NSURL has a history of crashing with bad input, so let's be safe + @try { // NSURL has a history of crashing with bad input, so let's be - NSURL *URL = [NSURL URLWithString:path]; - if (URL.scheme) { // Was a well-formed absolute URL - return URL; + NSURLComponents *urlComponents = [NSURLComponents componentsWithString:path]; //[NSURL URLWithString:path]; + if (urlComponents.scheme) { + return [self _preprocessURLComponents:urlComponents from:path].URL; } - // Check if it has a scheme - if ([path rangeOfString:@"://"].location != NSNotFound) { - NSMutableCharacterSet *urlAllowedCharacterSet = [NSMutableCharacterSet new]; - [urlAllowedCharacterSet formUnionWithCharacterSet:[NSCharacterSet URLUserAllowedCharacterSet]]; - [urlAllowedCharacterSet formUnionWithCharacterSet:[NSCharacterSet URLPasswordAllowedCharacterSet]]; - [urlAllowedCharacterSet formUnionWithCharacterSet:[NSCharacterSet URLHostAllowedCharacterSet]]; - [urlAllowedCharacterSet formUnionWithCharacterSet:[NSCharacterSet URLPathAllowedCharacterSet]]; - [urlAllowedCharacterSet formUnionWithCharacterSet:[NSCharacterSet URLQueryAllowedCharacterSet]]; - [urlAllowedCharacterSet formUnionWithCharacterSet:[NSCharacterSet URLFragmentAllowedCharacterSet]]; - path = [path stringByAddingPercentEncodingWithAllowedCharacters:urlAllowedCharacterSet]; - URL = [NSURL URLWithString:path]; - if (URL) { - return URL; - } - } - - // Assume that it's a local path path = path.stringByRemovingPercentEncoding; if ([path hasPrefix:@"~"]) { // Path is inside user directory @@ -115,7 +98,8 @@ + (NSURL *)NSURL:(id)json // Assume it's a resource path path = [[NSBundle mainBundle].resourcePath stringByAppendingPathComponent:path]; } - if (!(URL = [NSURL fileURLWithPath:path])) { + NSURL *URL = [NSURL fileURLWithPath:path]; + if (!URL) { RCTLogConvertError(json, @"a valid URL"); } return URL; @@ -125,6 +109,39 @@ + (NSURL *)NSURL:(id)json } } +// This function preprocess the URLComponents received to make sure that we decode it properly +// handling all the use cases. +// See the `RCTConvert_NSURLTests` file for a list of use cases that we want to support: +// To achieve that, we are currently splitting the url, extracting the fragment, so we can +// decode and encode everything but the fragment (which has to be left unmodified) ++ (NSURLComponents *)_preprocessURLComponents:(NSURLComponents *)urlComponents from:(NSString *)path +{ + // https://developer.apple.com/documentation/foundation/nsurlcomponents + // "[NSURLComponents's] behavior differs subtly from the NSURL class, which conforms to older RFCs" + // Specifically, NSURL rejects some URLs that NSURLComponents will handle + // gracefully. + NSRange fragmentRange = urlComponents.rangeOfFragment; + + if (fragmentRange.length == 0) { + // No fragment, pre-remove all escaped characters so we can encode them once if they are present. + NSError *error = nil; + NSRegularExpression *regex = [NSRegularExpression regularExpressionWithPattern:@"%[0-90-9]" options:0 error:&error]; + NSTextCheckingResult *match = [regex firstMatchInString:path options:0 range:NSMakeRange(0, path.length)]; + if (match) { + return [NSURLComponents componentsWithString:path.stringByRemovingPercentEncoding]; + } + return [NSURLComponents componentsWithString:path]; + } + // Pre-remove all escaped characters (excluding the fragment) to handle partially encoded strings + NSString *baseUrlString = [path substringToIndex:fragmentRange.location].stringByRemovingPercentEncoding; + // Fragment must be kept as they are passed. We don't have to escape them + NSString *unmodifiedFragment = [path substringFromIndex:fragmentRange.location]; + + // Recreate the url by using a decoded base and an unmodified fragment. + NSString *preprocessedURL = [NSString stringWithFormat:@"%@%@", baseUrlString, unmodifiedFragment]; + return [NSURLComponents componentsWithString:preprocessedURL]; +} + RCT_ENUM_CONVERTER( NSURLRequestCachePolicy, (@{ diff --git a/packages/rn-tester/RNTesterUnitTests/RCTConvert_NSURLTests.m b/packages/rn-tester/RNTesterUnitTests/RCTConvert_NSURLTests.m index 7d3dadde38fbc8..e1c0a58cb8abd6 100644 --- a/packages/rn-tester/RNTesterUnitTests/RCTConvert_NSURLTests.m +++ b/packages/rn-tester/RNTesterUnitTests/RCTConvert_NSURLTests.m @@ -77,4 +77,44 @@ - (void)testDataURL XCTAssertEqualObjects([testURL absoluteString], [expectedURL absoluteString]); } +// Escaping edge cases +TEST_URL( + urlWithMultipleHashes, + @"https://example.com/#/abc/#test:example.com", + @"https://example.com/#/abc/%23test:example.com") +TEST_URL(urlWithEqualsInQuery, @"https://example.com/abc.def?ghi=1234", @"https://example.com/abc.def?ghi=1234") +TEST_URL( + urlWithEscapedCharacterInFragment, + @"https://example.com/abc/def.ghi#jkl-mno%27p-qrs", + @"https://example.com/abc/def.ghi#jkl-mno%27p-qrs") +TEST_URL( + urlWithLongQuery, + @"https://example.com/abc?q=def+ghi+jkl&mno=p-q-r-s&tuv=wxy&z_=abc&abc=5", + @"https://example.com/abc?q=def+ghi+jkl&mno=p-q-r-s&tuv=wxy&z_=abc&abc=5") +TEST_URL( + urlWithEscapedCharacterInPathFragment, + @"https://example.com/#/abc/%23def%3Aghi.org", + @"https://example.com/#/abc/%23def%3Aghi.org") +TEST_URL( + urlWithEscapedCharacterInQuery, + @"https://site.com/script?foo=bar#this_ref", + @"https://site.com/script?foo=bar#this_ref") +TEST_URL( + urlWithUnescapedJson, + @"https://example.com/?{\"key\":\"value\"}", + @"https://example.com/?%7B%22key%22:%22value%22%7D") +TEST_URL( + urlWithPartiallyEscapedData, + @"https://example.com/?{%22key%22:%22value%22}", + @"https://example.com/?%7B%22key%22:%22value%22%7D") +TEST_URL(urlWithPercent, @"https://example.com/?width=22%", @"https://example.com/?width=22%25") +// NOTE: This is illegal per RFC 3986, but earlier URL specs allowed it +TEST_URL(urlWithSquareBracketInPath, @"http://www.foo.com/file[.html", @"http://www.foo.com/file%5B.html") + +TEST_URL(baseDeepLink, @"myapp://launch", @"myapp://launch") +TEST_URL( + deepLinkWithParams, + @"myapp://screen_route?withId=123&prodId=456&isSelected=true", + @"myapp://screen_route?withId=123&prodId=456&isSelected=true") + @end