Reapply Fix escaping in the URL conversion (facebook#36949)

Summary:
Pull Request resolved: facebook#36949

This change is a second attempt at fixing URL encoding and escaping that was already tried [here](facebook@2b4e1f5).

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 facebook#28508 for good.

## Changelog:
[iOS][Fixed] - Properly escape URLs

Reviewed By: mdvacca

Differential Revision: D45078923

fbshipit-source-id: a09e434ac1365949f1a738c9c17111a92d55677b

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,
     (@{

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