Skip to content

Commit

Permalink
dev-middleware: Redefine "serverBaseUrl" as server-relative, '/json/l…
Browse files Browse the repository at this point in the history
…ist' by requestor (#47628)

Summary:
Pull Request resolved: #47628

`serverBaseUrl` is currently documented as:

> The base URL to the dev server, as addressible from the local developer machine

This is problematic in general because `dev-middleware` on a server doesn't necessarily know about where clients might be reaching it from, how tunnels or port-forwards are set up, etc., and this can change over the lifetime of the server and vary between clients.

Indeed, our own use of `serverBaseUrl` from both `community-cli-plugin` and internally simply sets it to the host and port the dev server is listening on - ie it's the address of the dev server accessible *from the server*.

This PR changes the docs, redefining `serverBaseUrl`, to match the way we currently specify it.

One usage where we *do* want the previously documented behaviour is in responses to `/json/list` (`getPageDescriptions`) where the URLs in the response should be reachable by a browser requesting `/json/list`.

Here, we use the request (host header, etc.) to attempt to get working base URL.

History:
It should be mentioned that this is the latest in a series of changes like this:
 - #39394
 - #39456

Learning from those:
 - This change does *not* break Android emulators, which routes `10.0.2.2` to localhost, or other routed devices, because `/open-debugger` still uses server-relative URLs, and now formally delegates to `BrowserLauncher` to decide what to do with those URLs (internally, VSCode / `xdg-open` handles port forwarding)
 - Middleware configuration is no longer required to specify how it is reachable from clients.

This sets up some subsequent changes for more robust handling of tunnelled connections.

Changelog:
[General][Breaking] dev-middleware: Frameworks should specify `serverBaseUrl` relative to the middleware host.

Reviewed By: huntie

Differential Revision: D65974487

fbshipit-source-id: 1face8fc7715df387f75b329e80932d8543ee419
  • Loading branch information
robhogan authored and facebook-github-bot committed Nov 18, 2024
1 parent ca9c563 commit acf384a
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* 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
*/

import getBaseUrlFromRequest from '../utils/getBaseUrlFromRequest';

test('returns a base url based on req.headers.host', () => {
expect(
getBaseUrlFromRequest(makeRequest('localhost:8081', false))?.href,
).toEqual('http://localhost:8081/');
});

test('identifies https using socket.encrypted', () => {
expect(
getBaseUrlFromRequest(makeRequest('secure.net:8443', true))?.href,
).toEqual('https://secure.net:8443/');
});

test('works with ipv6 hosts', () => {
expect(getBaseUrlFromRequest(makeRequest('[::1]:8081', false))?.href).toEqual(
'http://[::1]:8081/',
);
});

test('returns null on an invalid host header', () => {
expect(getBaseUrlFromRequest(makeRequest('local[]host', false))).toBeNull();
});

test('returns null on an empty host header', () => {
expect(getBaseUrlFromRequest(makeRequest(null, false))).toBeNull();
});

function makeRequest(
host: ?string,
encrypted: boolean,
): http$IncomingMessage<> | http$IncomingMessage<tls$TLSSocket> {
// $FlowIgnore[incompatible-return] Partial mock of request
return {
socket: encrypted ? {encrypted: true} : {},
headers: host != null ? {host} : {},
};
}
7 changes: 2 additions & 5 deletions packages/dev-middleware/src/createDevMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,8 @@ type Options = $ReadOnly<{
projectRoot: string,

/**
* The base URL to the dev server, as addressible from the local developer
* machine. This is used in responses which return URLs to other endpoints,
* e.g. the debugger frontend and inspector proxy targets.
*
* Example: `'http://localhost:8081'`.
* The base URL to the dev server, as reachable from the machine on which
* dev-middleware is hosted. Typically `http://localhost:${metroPort}`.
*/
serverBaseUrl: string,

Expand Down
1 change: 1 addition & 0 deletions packages/dev-middleware/src/inspector-proxy/Device.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ export default class Device {
projectRoot: string,
eventReporter: ?EventReporter,
createMessageMiddleware: ?CreateCustomMessageHandlerFn,
serverBaseUrl?: URL,
) {
this.#dangerouslyConstruct(
id,
Expand Down
28 changes: 20 additions & 8 deletions packages/dev-middleware/src/inspector-proxy/InspectorProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type {IncomingMessage, ServerResponse} from 'http';
// $FlowFixMe[cannot-resolve-module] libdef missing in RN OSS
import type {Timeout} from 'timers';

import getBaseUrlFromRequest from '../utils/getBaseUrlFromRequest';
import Device from './Device';
import nullthrows from 'nullthrows';
// Import these from node:timers to get the correct Flow types.
Expand All @@ -47,7 +48,7 @@ export interface InspectorProxyQueries {
* Returns list of page descriptions ordered by device connection order, then
* page addition order.
*/
getPageDescriptions(): Array<PageDescription>;
getPageDescriptions(requestorRelativeBaseUrl: URL): Array<PageDescription>;
}

/**
Expand All @@ -57,8 +58,8 @@ export default class InspectorProxy implements InspectorProxyQueries {
// Root of the project used for relative to absolute source path conversion.
#projectRoot: string;

/** The base URL to the dev server from the developer machine. */
#serverBaseUrl: string;
// The base URL to the dev server from the dev-middleware host.
#serverBaseUrl: URL;

// Maps device ID to Device instance.
#devices: Map<string, Device>;
Expand All @@ -81,22 +82,27 @@ export default class InspectorProxy implements InspectorProxyQueries {
customMessageHandler: ?CreateCustomMessageHandlerFn,
) {
this.#projectRoot = projectRoot;
this.#serverBaseUrl = serverBaseUrl;
this.#serverBaseUrl = new URL(serverBaseUrl);
this.#devices = new Map();
this.#eventReporter = eventReporter;
this.#experiments = experiments;
this.#customMessageHandler = customMessageHandler;
}

getPageDescriptions(): Array<PageDescription> {
getPageDescriptions(requestorRelativeBaseUrl: URL): Array<PageDescription> {
// Build list of pages from all devices.
let result: Array<PageDescription> = [];
Array.from(this.#devices.entries()).forEach(([deviceId, device]) => {
result = result.concat(
device
.getPagesList()
.map((page: Page) =>
this.#buildPageDescription(deviceId, device, page),
this.#buildPageDescription(
deviceId,
device,
page,
requestorRelativeBaseUrl,
),
),
);
});
Expand All @@ -117,7 +123,12 @@ export default class InspectorProxy implements InspectorProxyQueries {
pathname === PAGES_LIST_JSON_URL ||
pathname === PAGES_LIST_JSON_URL_2
) {
this.#sendJsonResponse(response, this.getPageDescriptions());
this.#sendJsonResponse(
response,
this.getPageDescriptions(
getBaseUrlFromRequest(request) ?? this.#serverBaseUrl,
),
);
} else if (pathname === PAGES_LIST_JSON_VERSION_URL) {
this.#sendJsonResponse(response, {
Browser: 'Mobile JavaScript',
Expand All @@ -143,8 +154,9 @@ export default class InspectorProxy implements InspectorProxyQueries {
deviceId: string,
device: Device,
page: Page,
requestorRelativeBaseUrl: URL,
): PageDescription {
const {host, protocol} = new URL(this.#serverBaseUrl);
const {host, protocol} = requestorRelativeBaseUrl;
const webSocketScheme = protocol === 'https:' ? 'wss' : 'ws';

const webSocketUrlWithoutProtocol = `${host}${WS_DEBUGGER_URL}?device=${deviceId}&page=${page.id}`;
Expand Down
14 changes: 8 additions & 6 deletions packages/dev-middleware/src/middleware/openDebuggerMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,14 @@ export default function openDebuggerMiddleware({
...
} = query;

const targets = inspectorProxy.getPageDescriptions().filter(
// Only use targets with better reloading support
app =>
app.title === LEGACY_SYNTHETIC_PAGE_TITLE ||
app.reactNative.capabilities?.nativePageReloads === true,
);
const targets = inspectorProxy
.getPageDescriptions(new URL(serverBaseUrl))
.filter(
// Only use targets with better reloading support
app =>
app.title === LEGACY_SYNTHETIC_PAGE_TITLE ||
app.reactNative.capabilities?.nativePageReloads === true,
);

let target;

Expand Down
4 changes: 4 additions & 0 deletions packages/dev-middleware/src/types/BrowserLauncher.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ export interface BrowserLauncher {
* Attempt to open a debugger frontend URL in a browser app window,
* optionally returning an object to control the launched browser instance.
* The browser used should be capable of running Chrome DevTools.
*
* The provided url is based on serverBaseUrl, and therefore reachable from
* the host of dev-middleware. Implementations are responsible for rewriting
* this as necessary where the server is remote.
*/
launchDebuggerAppWindow: (url: string) => Promise<void>;
}
28 changes: 28 additions & 0 deletions packages/dev-middleware/src/utils/getBaseUrlFromRequest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* 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
*/

// Determine the base URL (scheme and host) used by a client to reach this
// server.
//
// TODO: Support X-Forwarded-Host, etc. for trusted proxies
export default function getBaseUrlFromRequest(
req: http$IncomingMessage<tls$TLSSocket> | http$IncomingMessage<net$Socket>,
): ?URL {
const hostHeader = req.headers.host;
if (hostHeader == null) {
return null;
}
// `encrypted` is always true for TLS sockets and undefined for net
// https://github.com/nodejs/node/issues/41863#issuecomment-1030709186
const scheme = req.socket.encrypted === true ? 'https' : 'http';
const url = `${scheme}://${req.headers.host}`;
return URL.canParse(url) ? new URL(url) : null;
}
6 changes: 5 additions & 1 deletion packages/dev-middleware/src/utils/getDevToolsFrontendUrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ function getWsParam({
const serverHost = new URL(devServerUrl).host;
let value;
if (wsUrl.host === serverHost) {
// Use a path-absolute (host-relative) URL
// Use a path-absolute (host-relative) URL if the WS server and frontend
// server are colocated. This is more robust for cases where the frontend
// may actually load through a tunnel or proxy, and the WS connection
// should therefore do the same.
//
// Depends on https://github.com/facebookexperimental/rn-chrome-devtools-frontend/pull/4
value = wsUrl.pathname + wsUrl.search + wsUrl.hash;
} else {
Expand Down

0 comments on commit acf384a

Please sign in to comment.