From 910ac9f4beeb0c6c4967f3a91c7533373a5184d3 Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Mon, 17 Apr 2023 05:20:59 -0700 Subject: [PATCH] Fix escaping in the URL conversion (#36898) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/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: 45bfa3135443208f6efba23f66d7952fa9dbc177 --- packages/react-native/React/Base/RCTConvert.m | 55 +++++++++++-------- .../RNTesterUnitTests/RCTConvert_NSURLTests.m | 33 +++++++++++ 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/packages/react-native/React/Base/RCTConvert.m b/packages/react-native/React/Base/RCTConvert.m index 600ff3fff47335..f61d41eb5669f3 100644 --- a/packages/react-native/React/Base/RCTConvert.m +++ b/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, (@{ diff --git a/packages/rn-tester/RNTesterUnitTests/RCTConvert_NSURLTests.m b/packages/rn-tester/RNTesterUnitTests/RCTConvert_NSURLTests.m index 7d3dadde38fbc8..d7b21b529beae4 100644 --- a/packages/rn-tester/RNTesterUnitTests/RCTConvert_NSURLTests.m +++ b/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