Fix escaping in the URL conversion (facebook#36898)

Summary:
Pull Request resolved: facebook#36898

This changes are a followup from D42281798 and the task T141309497.

In the previous diff, we were able to handle most cases thanks to `NSURLComponents`. `NSURLComponents` provides us with more flexibility so that we could handle the missing cases.

## Changelog:
[iOS][Fixed] - Handle doulbe `#` and partially escaped urls

Reviewed By: sammy-SC

Differential Revision: D44958172

fbshipit-source-id: 03628d86966c149d0785ad90fdbccbcb5e70106e

Author: cipolleschi

packages/react-native/React/Base/RCTConvert.m

@@ -83,30 +83,12 @@ + (NSURL *)NSURL:(id)json
     return nil;
   }
 
-  @try { // NSURL has a history of crashing with bad input, so let's be safe
-
-    NSURL *URL = [NSURL URLWithString:path];
-    if (URL.scheme) { // Was a well-formed absolute URL
-      return URL;
+  @try { // NSURL has a history of crashing with bad input, so let's be
+    NSURLComponents *urlComponents = [NSURLComponents componentsWithString: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 +97,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 +108,32 @@ + (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.
+    return [NSURLComponents componentsWithString:path.stringByRemovingPercentEncoding];
+  }
+  // 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,37 @@ - (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")
+// 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")
+
 @end