From 145af9a18ba6b52840438985d9ecb4d29711fa7a Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Mon, 17 Apr 2023 14:01:08 -0700 Subject: [PATCH] Back out "Fix escaping in the URL conversion" (#36939) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36939 Revert changes implemented in [this commit](https://github.com/facebook/react-native/commit/2b4e1f5ece7d160935b19d4862af8706a44cee59). It makes some e2e internal test fail, although it is unclear why. ## Changelog: [General][Fixed] - Rollback changes on URL escaping Reviewed By: philIip, mdvacca Differential Revision: D45061886 fbshipit-source-id: 61e388472209097c5629fc4df402369ef806d081 --- packages/react-native/React/Base/RCTConvert.m | 55 ++++++++----------- .../RNTesterUnitTests/RCTConvert_NSURLTests.m | 33 ----------- 2 files changed, 23 insertions(+), 65 deletions(-) diff --git a/packages/react-native/React/Base/RCTConvert.m b/packages/react-native/React/Base/RCTConvert.m index f61d41eb566..600ff3fff47 100644 --- a/packages/react-native/React/Base/RCTConvert.m +++ b/packages/react-native/React/Base/RCTConvert.m @@ -83,12 +83,30 @@ RCT_CUSTOM_CONVERTER(NSData *, NSData, [json dataUsingEncoding:NSUTF8StringEncod return nil; } - @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; + @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; } + // 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 @@ -97,8 +115,7 @@ RCT_CUSTOM_CONVERTER(NSData *, NSData, [json dataUsingEncoding:NSUTF8StringEncod // Assume it's a resource path path = [[NSBundle mainBundle].resourcePath stringByAppendingPathComponent:path]; } - NSURL *URL = [NSURL fileURLWithPath:path]; - if (!URL) { + if (!(URL = [NSURL fileURLWithPath:path])) { RCTLogConvertError(json, @"a valid URL"); } return URL; @@ -108,32 +125,6 @@ RCT_CUSTOM_CONVERTER(NSData *, NSData, [json dataUsingEncoding:NSUTF8StringEncod } } -// 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 d7b21b529be..7d3dadde38f 100644 --- a/packages/rn-tester/RNTesterUnitTests/RCTConvert_NSURLTests.m +++ b/packages/rn-tester/RNTesterUnitTests/RCTConvert_NSURLTests.m @@ -77,37 +77,4 @@ TEST_URL( 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