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

webpack: Disable host check for webpack-dev-server. #9205

Closed
wants to merge 1 commit into from
Closed

webpack: Disable host check for webpack-dev-server. #9205

wants to merge 1 commit into from

Conversation

priyank-p
Copy link
Member

Webpack dev server by default does host checking for requests. so
in dev environment, if the request came for zulipdev.com it would not
send js files which caused dev environment to not work.

@timabbott
Copy link
Member

Ahh, OK, this is the issue that you thought was expose-loader being broken. Makes sense. I think for security reasons, we don't want to disable this unconditionally; instead, we should disable this only in the remote dev environment case (effectively, when run-dev.py --interface='' is what we're doing). Is there a way to pass this in as an option from the caller?

See webpack/webpack-dev-server#887 for details on the security issue.

@priyank-p
Copy link
Member Author

Yeah, we can pass disable the option only when the the interface flag is --interface=''. The reason i disabled it as i though it was run-dev.py that handles the request passed to webpack-dev-server and django already will not accept request from any port that is not in EXTERNAL_HOST.

@timabbott
Copy link
Member

Right, but someone could open a connection directly to the webpack port of 9994, not going through run-dev.py. Basically, it's a security problem to have that option disabled on your laptop, but not really a problem on a remote dev server. And it's a functionality problem the other way around. So basically we want this change if and only if --interface=''

@priyank-p
Copy link
Member Author

We can also pass in a list of domains to whitelist ie .zulipdev.org, but is there a way to get hostnames to pass down without a flag? (From the EXTERNAL_HOST if we can would be great).

@timabbott
Copy link
Member

I'm less excited about a domain whitelist than just having this setting be controlled by an option passed from run-dev.py, if that's a possibility.

Webpack dev server by default does host checking for requests. so
in dev enviorment if the the request came for zulipdev.com it would not
send js files which caused dev envoirment to not work.
@zulipbot zulipbot added size: S and removed size: XS labels Apr 24, 2018
@priyank-p
Copy link
Member Author

@timabbott i updated this PR so that the tools/run-dev.py accepts --disable-host-check as well as tools/webpack.

This is flag is flipped on if interface='' but do we also want to do it where interface is None and the user is zulipdev and/or vagrant?

@timabbott
Copy link
Member

Yes, we do. I think the right solution is to make the option conditional on the final value of options.interface, rather than a separate run-dev.py option. I did that tweak and merged this as
bc454ba. Can you verify that works for you?

@timabbott timabbott closed this Apr 24, 2018
@priyank-p priyank-p deleted the webpack-host-check branch April 24, 2018 21:20
@priyank-p
Copy link
Member Author

This works for me, but I think we also need to let people who have droplets confirm this.

@timabbott
Copy link
Member

Great; can you post in the original thread with @shubhamdhama that inspired this PR to make sure other folks test this?

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

Successfully merging this pull request may close these issues.

3 participants