Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevents JS Debugger issues with CORS #17720

Closed
wants to merge 11 commits into from
11 changes: 6 additions & 5 deletions local-cli/server/middleware/getDevToolsMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ const launchChrome = require('../util/launchChrome');

const {exec} = require('child_process');

function launchChromeDevTools(port, args = '') {
var debuggerURL = 'http://localhost:' + port + '/debugger-ui' + args;
function launchChromeDevTools(host, args = '') {
var debuggerURL = 'http://' + host + '/debugger-ui' + args;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why eslint does not throw error when using "var" ?

Copy link
Contributor

@janicduplessis janicduplessis Jan 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are still a lot of files that are not converted to es6 so the rule is not enabled.

console.log('Launching Dev Tools...');
launchChrome(debuggerURL);
}
Expand All @@ -25,7 +25,7 @@ function escapePath(pathname) {
return '"' + pathname + '"';
}

function launchDevTools({port, projectRoots}, isChromeConnected) {
function launchDevTools({host, projectRoots}, isChromeConnected) {
// Explicit config always wins
var customDebugger = process.env.REACT_DEBUGGER;
if (customDebugger) {
Expand All @@ -39,12 +39,13 @@ function launchDevTools({port, projectRoots}, isChromeConnected) {
});
} else if (!isChromeConnected()) {
// Dev tools are not yet open; we need to open a session
launchChromeDevTools(port);
launchChromeDevTools(host);
}
}

module.exports = function(options, isChromeConnected) {
return function(req, res, next) {
var host = req.headers.host;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later it might be nice to honor the protocol too (ex: setting up an HTTPS reverse proxy in front of the packager server) and to use something like Express’s approach to inferring the protocol: https://github.com/expressjs/express/blob/master/lib/request.js#L306

if (req.url === '/launch-safari-devtools') {
// TODO: remove `console.log` and dev tools binary
console.log(
Expand All @@ -62,7 +63,7 @@ module.exports = function(options, isChromeConnected) {
launchDevTools(options, isChromeConnected);
res.end('OK');
} else if (req.url === '/launch-js-devtools') {
launchDevTools(options, isChromeConnected);
launchDevTools({...options, host}, isChromeConnected);
res.end('OK');
} else {
next();
Expand Down