Skip to content

Commit

Permalink
Improve normalisation of percent-encoded request URLs
Browse files Browse the repository at this point in the history
Summary:
When processing requests, Metro compares path prefixes and "extensions" to specific magic strings, eg `/[metro-project]/`. Currently, this matching is performed on the literal requested path, which makes it sensitive to URL (percent) encoding.

In particular, iOS (`NSURL urlWithString`) encodes some characters more aggressively than other clients, including encoding `[` and `]`. This causes a request for `/[metro-project]/foo.js` to fail with a 404 when executed through `NSURLRequest`, because `urlObj.pathname` begins `/%5Bmetro-project%5D/`.

Metro should ideally be insensitive to percent-encoding on all requests. This fix isn't complete, but addresses the particular blocking issue with source URLs.

```
 - **[Fix]**: Improve dev server insensitivity to percent encoding on source requests.
```

Reviewed By: motiz88

Differential Revision: D58289369

fbshipit-source-id: 616da5d5e44539996c16f103254f1b5aaa83704c
  • Loading branch information
robhogan authored and facebook-github-bot committed Jun 9, 2024
1 parent 05fd223 commit 6e71699
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 11 deletions.
17 changes: 6 additions & 11 deletions packages/metro/src/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ class Server {
) {
const originalUrl = req.url;
req.url = this._rewriteAndNormalizeUrl(req.url);
const urlObj = url.parse(req.url, true);
const urlObj = url.parse(decodeURI(req.url), true);
const {host} = req.headers;
debug(
`Handling request: ${host ? 'http://' + host : ''}${req.url}` +
Expand Down Expand Up @@ -597,11 +597,11 @@ class Server {
for (const [pathnamePrefix, normalizedRootDir] of this
._sourceRequestRoutingMap) {
if (pathname.startsWith(pathnamePrefix)) {
const relativePathname = pathname.substr(pathnamePrefix.length);
await this._processSourceRequest(
req,
res,
pathnamePrefix,
relativePathname,
normalizedRootDir,
res,
);
handled = true;
break;
Expand All @@ -614,15 +614,10 @@ class Server {
}

async _processSourceRequest(
req: IncomingMessage,
res: ServerResponse,
pathnamePrefix: string,
relativePathname: string,
rootDir: string,
res: ServerResponse,
): Promise<void> {
const urlObj = url.parse(req.url, true);
const relativePathname = nullthrows(urlObj.pathname).substr(
pathnamePrefix.length,
);
if (
!this._allowedSuffixesForSourceRequests.some(suffix =>
relativePathname.endsWith(suffix),
Expand Down
15 changes: 15 additions & 0 deletions packages/metro/src/integration_tests/__tests__/server-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,5 +195,20 @@ describe('Metro development server serves bundles via HTTP', () => {
),
);
});

test('requested with aggressive URL encoding /%5Bmetro-project%5D', async () => {
const response = await fetch(
'http://localhost:' +
config.server.port +
'/%5Bmetro-project%5D/Foo%2Ejs',
);
expect(response.status).toBe(200);
expect(await response.text()).toEqual(
await fs.promises.readFile(
path.join(__dirname, '../basic_bundle/Foo.js'),
'utf8',
),
);
});
});
});

0 comments on commit 6e71699

Please sign in to comment.