Skip to content

Commit

Permalink
Support ';&' as an alternative to '?' in bundle URLs
Browse files Browse the repository at this point in the history
Summary:
Recent versions of JavaScriptCore strip query strings from `Error.prototype.stack`, which breaks our use of query strings to carry bundle build parameters. See facebook/react-native#36794 for context.

This allows Metro to accept an alternative format where we interpret the special reserved character sequence `;&`, the first time it appears in a URL *path*, as equivalent to `?`.

So that this does not break custom `rewriteRequestUrl` implementations, we (temporarily) pass a normalised URL (`;&` replaced with `?`), and then reverse that operation on the URL returned by `rewriteRequestUrl`. *[Simplifying the details here somewhat - see the implementation for specifics].*

Changelog:
```
**[Feature]**: Support alternative JavaScriptCore-safe URL format (`;&` as `?`).
```

Differential Revision: D45477505

fbshipit-source-id: ff78c751d45fcf2f7508cce9fcb6f3a81f58ee32
  • Loading branch information
robhogan authored and facebook-github-bot committed May 2, 2023
1 parent 97d5544 commit fa99364
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 1 deletion.
14 changes: 13 additions & 1 deletion packages/metro/src/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const ResourceNotFoundError = require('./IncrementalBundler/ResourceNotFoundErro
const bundleToString = require('./lib/bundleToString');
const formatBundlingError = require('./lib/formatBundlingError');
const getGraphId = require('./lib/getGraphId');
const jscUrlUtils = require('./lib/jscUrlUtils');
const parseOptionsFromUrl = require('./lib/parseOptionsFromUrl');
const splitBundleOptions = require('./lib/splitBundleOptions');
const transformHelpers = require('./lib/transformHelpers');
Expand Down Expand Up @@ -495,7 +496,18 @@ class Server {
next: (?Error) => mixed,
) {
const originalUrl = req.url;
req.url = this._config.server.rewriteRequestUrl(req.url);
const normalizedUrl = jscUrlUtils.normalizeJscUrl(originalUrl);

// TODO: rewriteRequestUrl should receive originalUrl, but this is a
// breaking change with the introduction of ';&' "query string" support.
// Expose a way for consumers to parse+construct these "JSC URLs" and
// require custom rewriteRequestUrl implementations to handle+emit them.
req.url = this._config.server.rewriteRequestUrl(normalizedUrl);

// TOOD: Remove this once the above breaking change is made.
if (originalUrl !== normalizedUrl) {
req.url = jscUrlUtils.toJscUrl(req.url);
}

const urlObj = url.parse(req.url, true);
const {host} = req.headers;
Expand Down
75 changes: 75 additions & 0 deletions packages/metro/src/lib/__tests__/jscUrlUtils-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict-local
* @format
* @oncall react_native
*/

const {normalizeJscUrl, toJscUrl} = require('../jscUrlUtils');

describe('normalizeJscUrl', () => {
test.each([
[
'/path1/path2;&foo=bar?bar=baz#frag?',
'/path1/path2?foo=bar&bar=baz#frag?',
],
[
'relative/path;&foo=bar?bar=baz#frag?',
'relative/path?foo=bar&bar=baz#frag?',
],
[
'https://user;&:password;&@mydomain.com:8080/path1/path2;&foo=bar?bar=baz#frag?',
'https://user%3B&:password%3B&@mydomain.com:8080/path1/path2?foo=bar&bar=baz#frag?',
],
[
'http://127.0.0.1/path1/path2;&foo=bar&bar=baz',
'http://127.0.0.1/path1/path2?foo=bar&bar=baz',
],
])('rewrites urls treating ;& in paths as ? (%s => %s)', (input, output) => {
expect(normalizeJscUrl(input)).toEqual(output);
});

test.each([
['http://user;&:password;&@mydomain.com/foo?bar=zoo?baz=quux;&'],
['/foo?bar=zoo?baz=quux'],
['proto:arbitrary_bad_url'],
['*'],
['relative/path'],
])('returns other strings exactly as given (%s)', input => {
expect(normalizeJscUrl(input)).toEqual(input);
});
});

describe('toJscUrl', () => {
test.each([
[
'https://user;&:password;&@mydomain.com:8080/path1/path2?foo=bar&bar=question?#frag?',
'https://user%3B&:password%3B&@mydomain.com:8080/path1/path2;&foo=bar&bar=question%3F#frag?',
],
[
'http://127.0.0.1/path1/path2?foo=bar',
'http://127.0.0.1/path1/path2;&foo=bar',
],
['*', '*'],
['/absolute/path', '/absolute/path'],
['relative/path', 'relative/path'],
['http://127.0.0.1/path1/path', 'http://127.0.0.1/path1/path'],
[
'/path1/path2?foo=bar&bar=question?#frag?',
'/path1/path2;&foo=bar&bar=question%3F#frag?',
],
[
'relative/path?foo=bar&bar=question?#frag?',
'relative/path;&foo=bar&bar=question%3F#frag?',
],
])(
'replaces the first ? with a JSC-friendly delimeter, url-encodes subsequent ? (%s => %s)',
(input, output) => {
expect(toJscUrl(input)).toEqual(output);
},
);
});
103 changes: 103 additions & 0 deletions packages/metro/src/lib/jscUrlUtils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @format
* @flow strict
*/

/**
* These functions are for handling of query-string free URLs, necessitated
* by query string stripping of URLs in JavaScriptCore stack traces
* introduced in iOS 16.4.
*
* See https://github.com/facebook/react-native/issues/36794 for context.
*/

const PLACEHOLDER_HOST = 'placeholder://example.com';
const JSC_QUERY_STRING_DELIMETER = ';&';

function normalizeJscUrl(urlToNormalize: string): string {
try {
const urlObj = new URL(urlToNormalize, PLACEHOLDER_HOST);
const delimeterIdx = urlObj.pathname.indexOf(JSC_QUERY_STRING_DELIMETER);
if (delimeterIdx === -1) {
return urlToNormalize;
}

// HTTP request lines may be either absolute *paths* (HTTP GET /foo) or
// absolute URIs (HTTP GET http://domain.com/foo) - so we should handle
// both.
// ( https://datatracker.ietf.org/doc/html/rfc9112#name-request-target )
const isAbsoluteURI = !urlObj.href.startsWith(PLACEHOLDER_HOST);

// Relative paths are not valid in an HTTP GET request line, but account
// for them for completeness. We'll use this to conditionally remove the
// `/` added by `URL`.
const isAbsolutePath = urlToNormalize.startsWith('/');

// This is our regular pathname
const pathBeforeDelimeter = urlObj.pathname.substring(0, delimeterIdx);
// This will become our query string
const pathAfterDelimeter = urlObj.pathname.substring(
delimeterIdx + JSC_QUERY_STRING_DELIMETER.length,
);

urlObj.pathname = pathBeforeDelimeter;
if (urlObj.search) {
// JSC-style URLs wouldn't normally be expected to have regular query
// strings, but append them if present
urlObj.search = `?${pathAfterDelimeter}&${urlObj.search.substring(1)}`;
} else {
urlObj.search = `?${pathAfterDelimeter}`;
}
let urlToReturn = urlObj.href;
if (!isAbsoluteURI) {
urlToReturn = urlToReturn.replace(PLACEHOLDER_HOST, '');
if (!isAbsolutePath) {
urlToReturn = urlToReturn.substring(1);
}
}
return urlToReturn;
} catch (e) {
// Preserve malformed URLs
return urlToNormalize;
}
}

function toJscUrl(urlToConvert: string): string {
try {
const urlObj = new URL(urlToConvert, PLACEHOLDER_HOST);
if (urlObj.search == null || !urlObj.search.startsWith('?')) {
return urlToConvert;
}
const isAbsoluteURI = !urlObj.href.startsWith(PLACEHOLDER_HOST);
// Relative paths are not valid in an HTTP GET request line, but may appear otherwise
const isAbsolutePath = urlToConvert.startsWith('/');

const queryString = urlObj.search.substring(1);
// NB: queryString may legally contain unencoded '?' in key or value names.
// Writing them into the path will implicitly encode them.
urlObj.pathname =
urlObj.pathname + JSC_QUERY_STRING_DELIMETER + queryString;
urlObj.search = '';
let urlToReturn = urlObj.href;
if (!isAbsoluteURI) {
urlToReturn = urlToReturn.replace(PLACEHOLDER_HOST, '');
if (!isAbsolutePath) {
urlToReturn = urlToReturn.substring(1);
}
}
return urlToReturn;
} catch (e) {
// Preserve malformed URLs
return urlToConvert;
}
}

module.exports = {
normalizeJscUrl,
toJscUrl,
};

0 comments on commit fa99364

Please sign in to comment.