Skip to content

fix(react-scripts): do not set allowedHosts if undefined #11877

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ylemkimon
Copy link

@ylemkimon ylemkimon commented Jan 3, 2022

Fixes #11762.

urls.lanUrlForConfig may be undefined, if the HOST is set, the IP address is not private, or the IP address retrieval has failed.

TEST PLAN: manual testing with the steps to reproduce #11762.

@daweimau
Copy link

Can this please be merged so projects can use cra ^5 and proxying?

@BalzGuenat
Copy link

Could this please get reviewed and merged? The issue currently prevents us from updating CRA.

@lukaselmer
Copy link

@iansu @mrmckeb anything preventing this PR from being merged?

Copy link

@gustavomoser gustavomoser left a comment

Choose a reason for hiding this comment

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

This PR can already be merged, right? 😄

Comment on lines +25 to +30
let allowedHosts;
if (!proxy || process.env.DANGEROUSLY_DISABLE_HOST_CHECK === 'true') {
allowedHosts = 'all';
} else if (allowedHost != null) {
allowedHosts = [allowedHost];
}
Copy link

@gustavomoser gustavomoser Sep 14, 2022

Choose a reason for hiding this comment

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

I think a simpler way could be something like

const disableFirewall = !proxy || process.env.DANGEROUSLY_DISABLE_HOST_CHECK === 'true';
return {
    ... 
    allowedHosts: disableFirewall ? 'all' : allowedHost ? [allowedHost] : undefined,
    ...
};

@soerenBoisen
Copy link

What is blocking this fix? Just encountered this bug that was reported in 2021.

@lukaselmer
Copy link

What is blocking this fix? Just encountered this bug that was reported in 2021.

Yeah very good question

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In CRA 5.0.0, react-scripts start fails when both HOST in .env and proxy in package.json are defined.
7 participants