[DevTools] Enable support for the React DevTools Client to connect to different host/port/path #603
[DevTools] Enable support for the React DevTools Client to connect to different host/port/path #603everettbu wants to merge 1 commit into
Conversation
Greptile SummaryThis PR adds support for routing the React DevTools client through a remote server (e.g., a reverse proxy) by introducing
Confidence Score: 4/5
Important Files Changed
Last reviewed commit: d987502 |
| `Backend.connectToDevTools({port: ${clientPort}, host: '${clientHost}', useHttps: ${ | ||
| clientUseHttps ? 'true' : 'false' | ||
| }${path != null ? `, path: '${path}'` : ''}});\n` + |
There was a problem hiding this comment.
Unsanitized string interpolation into generated JS
clientHost and path are interpolated directly into single-quoted JavaScript string literals without escaping. If either value contains a single quote or backslash, it will break the generated script (or worse, allow code injection if startServer is ever called with untrusted input).
The pre-existing host interpolation had the same issue, but this PR extends the pattern to two new parameters. Consider using JSON.stringify() for safe embedding, which is already used for componentFiltersString on line 395:
| `Backend.connectToDevTools({port: ${clientPort}, host: '${clientHost}', useHttps: ${ | |
| clientUseHttps ? 'true' : 'false' | |
| }${path != null ? `, path: '${path}'` : ''}});\n` + | |
| `Backend.connectToDevTools({port: ${clientPort}, host: ${JSON.stringify(clientHost)}, useHttps: ${ | |
| clientUseHttps ? 'true' : 'false' | |
| }${path != null ? `, path: ${JSON.stringify(path)}` : ''}});\n` + |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-core/src/standalone.js
Line: 396-398
Comment:
**Unsanitized string interpolation into generated JS**
`clientHost` and `path` are interpolated directly into single-quoted JavaScript string literals without escaping. If either value contains a single quote or backslash, it will break the generated script (or worse, allow code injection if `startServer` is ever called with untrusted input).
The pre-existing `host` interpolation had the same issue, but this PR extends the pattern to two new parameters. Consider using `JSON.stringify()` for safe embedding, which is already used for `componentFiltersString` on line 395:
```suggestion
`Backend.connectToDevTools({port: ${clientPort}, host: ${JSON.stringify(clientHost)}, useHttps: ${
clientUseHttps ? 'true' : 'false'
}${path != null ? `, path: ${JSON.stringify(path)}` : ''}});\n` +
```
How can I resolve this? If you propose a fix, please make it concise.b77151e to
d987502
Compare
| const pathStr = path != null ? path : ''; | ||
| const server = defaultPort ? `${effectiveProtocol}://${effectiveHost}${pathStr}` : `${effectiveProtocol}://${effectiveHost}:${effectivePort}${pathStr}`; | ||
| const serverIp = defaultPort ? `${effectiveProtocol}://${localIp}${pathStr}` : `${effectiveProtocol}://${localIp}:${effectivePort}${pathStr}`; |
There was a problem hiding this comment.
Missing leading / normalization for path
backend.js:112 normalizes the path by prepending / if it's missing, but this display URL construction does not. If a user sets REACT_DEVTOOLS_PATH=__react_devtools__/ (without a leading /), the display URL becomes http://localhost:8097__react_devtools__/ — a broken URL with no path separator.
Consider normalizing pathStr the same way backend.js does:
| const pathStr = path != null ? path : ''; | |
| const server = defaultPort ? `${effectiveProtocol}://${effectiveHost}${pathStr}` : `${effectiveProtocol}://${effectiveHost}:${effectivePort}${pathStr}`; | |
| const serverIp = defaultPort ? `${effectiveProtocol}://${localIp}${pathStr}` : `${effectiveProtocol}://${localIp}:${effectivePort}${pathStr}`; | |
| const pathStr = path != null ? (path !== '' && !path.startsWith('/') ? '/' + path : path) : ''; | |
| const server = defaultPort ? `${effectiveProtocol}://${effectiveHost}${pathStr}` : `${effectiveProtocol}://${effectiveHost}:${effectivePort}${pathStr}`; | |
| const serverIp = defaultPort ? `${effectiveProtocol}://${localIp}${pathStr}` : `${effectiveProtocol}://${localIp}:${effectivePort}${pathStr}`; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools/app.html
Line: 171-173
Comment:
**Missing leading `/` normalization for `path`**
`backend.js:112` normalizes the `path` by prepending `/` if it's missing, but this display URL construction does not. If a user sets `REACT_DEVTOOLS_PATH=__react_devtools__/` (without a leading `/`), the display URL becomes `http://localhost:8097__react_devtools__/` — a broken URL with no path separator.
Consider normalizing `pathStr` the same way `backend.js` does:
```suggestion
const pathStr = path != null ? (path !== '' && !path.startsWith('/') ? '/' + path : path) : '';
const server = defaultPort ? `${effectiveProtocol}://${effectiveHost}${pathStr}` : `${effectiveProtocol}://${effectiveHost}:${effectivePort}${pathStr}`;
const serverIp = defaultPort ? `${effectiveProtocol}://${localIp}${pathStr}` : `${effectiveProtocol}://${localIp}:${effectivePort}${pathStr}`;
```
How can I resolve this? If you propose a fix, please make it concise.| const effectiveHost = clientHost != null ? clientHost : host; | ||
| const effectivePort = clientPort != null ? clientPort : port; | ||
| const effectiveUseHttps = clientUseHttps != null ? clientUseHttps : useHttps; | ||
| const effectiveProtocol = effectiveUseHttps ? 'https' : 'http'; | ||
| const defaultPort = (effectivePort === 443 && effectiveUseHttps) || (effectivePort === 80 && !effectiveUseHttps); | ||
| const pathStr = path != null ? path : ''; | ||
| const server = defaultPort ? `${effectiveProtocol}://${effectiveHost}${pathStr}` : `${effectiveProtocol}://${effectiveHost}:${effectivePort}${pathStr}`; | ||
| const serverIp = defaultPort ? `${effectiveProtocol}://${localIp}${pathStr}` : `${effectiveProtocol}://${localIp}:${effectivePort}${pathStr}`; |
There was a problem hiding this comment.
"By IP" display URL is misleading with client overrides
When client overrides are set (e.g., REACT_DEVTOOLS_CLIENT_HOST=remote.example.com, REACT_DEVTOOLS_CLIENT_PORT=443, REACT_DEVTOOLS_CLIENT_USE_HTTPS=true), serverIp becomes something like https://192.168.1.5:443/__react_devtools__/. This mixes the local IP with the remote proxy's protocol/port, producing a URL that won't work.
Consider either falling back to the server's own port/protocol for the IP-based URL (since that represents the local server), or hiding the "by IP" suggestion entirely when clientHost is set.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools/app.html
Line: 166-173
Comment:
**"By IP" display URL is misleading with client overrides**
When client overrides are set (e.g., `REACT_DEVTOOLS_CLIENT_HOST=remote.example.com`, `REACT_DEVTOOLS_CLIENT_PORT=443`, `REACT_DEVTOOLS_CLIENT_USE_HTTPS=true`), `serverIp` becomes something like `https://192.168.1.5:443/__react_devtools__/`. This mixes the local IP with the remote proxy's protocol/port, producing a URL that won't work.
Consider either falling back to the server's own port/protocol for the IP-based URL (since that represents the local server), or hiding the "by IP" suggestion entirely when `clientHost` is set.
How can I resolve this? If you propose a fix, please make it concise.|
Upstream PR was closed or merged. Code is synced via branch mirror. |
Mirror of facebook/react#35886
Original author: fullstackhacker
Summary
This enables routing the React Dev Tools through a remote server by being able to specify host, port, and path for the client to connect to. Basically allowing the React Dev Tools server to have the client connect elsewhere.
This setups a
clientOptionswhich can be set up through environment variables when starting the React Dev Tools server.This change shouldn't affect the traditional usage for React Dev Tools.
EDIT: the additional change was moved to another PR
How did you test this change?
Run React DevTools with
Confirm that my application connects to the local React Dev Tools server/instance/electron app through my remote server.