Skip to content

Commit

Permalink
Reapply Fix escaping in the URL conversion (facebook#36949)
Browse files Browse the repository at this point in the history
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: 010a5c173784f8341a1a08bcbd06a6ad14299c75
  • Loading branch information
cipolleschi authored and jeongshin committed May 7, 2023
1 parent 8f226b6 commit f26bc06
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 22 deletions.
61 changes: 39 additions & 22 deletions packages/react-native/React/Base/RCTConvert.m
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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,
(@{
Expand Down
40 changes: 40 additions & 0 deletions packages/rn-tester/RNTesterUnitTests/RCTConvert_NSURLTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit f26bc06

Please sign in to comment.