Added new --host option and updated relevant tests

[victorandcode]

Hello! This PR should add the new option --host as requested on 1412. Based on the comments I assumed perhaps HMRServer.js had to be updated but after testing, that change seems unecessary.

Rundown of my changes

• A new cli option called --host which receives the host to listen on (defaults to localhost)
• Updated server tests

How I tested

1. Created an Ubuntu guest virtual machine
2. Run yarn build in the host machine after changing the code
3. Run bin\cli.js -p 999 --host <LOCAL_INTERFACE_IP> <SOME_LOCAL_ENTRY_POINT> in the host machine
4. Connected successfully from the guest machine to the host machine via <LOCAL_INTERFACE_IP>:999

[codecov-io]

Codecov Report

Merging #1482 into master will increase coverage by 1.4%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #1482     +/-   ##
=========================================
+ Coverage   86.41%   87.82%   +1.4%     
=========================================
  Files          80       80             
  Lines        4585     4401    -184     
=========================================
- Hits         3962     3865     -97     
+ Misses        623      536     -87

Impacted Files	Coverage Δ
src/Bundler.js	93.42% <100%> (ø)	⬆️
src/Server.js	93.1% <100%> (+0.12%)	⬆️
src/workerfarm/errorUtils.js	64.7% <0%> (-17.65%)	⬇️
src/visitors/dependencies.js	78.63% <0%> (-10.26%)	⬇️
src/assets/ReasonAsset.js	92.3% <0%> (-7.7%)	⬇️
src/transforms/babel.js	87.5% <0%> (-7.41%)	⬇️
src/worker.js	94.73% <0%> (-5.27%)	⬇️
src/utils/PromiseQueue.js	90.78% <0%> (-3.45%)	⬇️
src/assets/GLSLAsset.js	93.1% <0%> (-3.2%)	⬇️
src/utils/config.js	86.95% <0%> (-1.45%)	⬇️
... and 21 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc10531...a94085d. Read the comment docs.

[DeMoorJasper]

Wasn't the main issue with websockets?

[victorandcode]

@DeMoorJasper after doing some testing I'm not sure if websockets would be the main issue
I've performed the following test and it seems to be working ok (please let me know if you can think of a different scenario that's not considered in the changes I made):

1. Run the following command from a mac on my local network bin\cli.js -p 1234 --host <LOCAL_INTERFACE_IP> index.html
2. Connected to <LOCAL_INTERFACE_IP>:1234 from another windows machine in my local network
3. Changed a css file (references by index.html) from the mac machine
   Result: Windows machine updated styles without reloading the page

[DeMoorJasper]

@victor-cordova alright sounds good :) Just want to make sure

@kwmiebach does this PR fix the original issue?

[mohsen1]

Specifying host seems pretty useful 👌🏽

[devongovett]

by default the server already listens on all hosts. is there a usecase to restrict that for some reason?

[victorandcode]

@devongovett I believe the use case would be the one described here Comment. What do you think?

[devongovett]

there is already --hmr-hostname for specifying the hostname of the HMR server.

[victorandcode]

@devongovett completely right, sorry I missed that one. I'll close the PR.

[kwmiebach]

I did not find in my tests that the server listens by default on all interfaces. It only listens on localhost, which is normally 127.0.0.1.

Btw. if it would do that, that would be a security issue.

And shouldn't hostname and interface/IP address be distinguished? A server cannot 'listen' on a hostname, it can only listen on an interface/IP address.

(But it could resolve a hostname to an IP address and listen on that IP address. Or it could listen on an interface but only answer requests with a specific hostname on that interface. The latter would only be necessary in specific development environments)

[kwmiebach]

@devongovett you said that already in another thread but it is not true. it only listens on localhost

[devongovett]

According to the node docs:

If host is omitted, the server will accept connections on the unspecified IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address (0.0.0.0) otherwise.

[kwmiebach]

@devongovett thank you for looking into this. give me some time and I will repeat my tests

[kwmiebach]

I repeated my tests and I can confirm now that parcel listens on all interfaces.

I was wrong, sorry for the confusion and work it cost some of you.

But: Maybe the feature that @victor-cordova implemented in his pull request could still be useful.

The default behaviour to listen on all interfaces could be seen as a security issue in some circumstances. In fact many development servers only listen to localhost by default. But at least it would now be possible to choose the interface where parcel listens, so a developer that does not want to expose the dev server to the whole network, could choose not to.

---

And there is something I noticed:

When you start parcel, it tells you it is only listening to localhost, which is wrong, as everybody knows now. This is definitely a security issue, because parcel listens to more interfaces than it tells you. Example:

$ parcel index.html
Server running at http://localhost:1234 
✨  Built in 1.91s.

This would be easy to fix here, where the string localhost is hardcoded:

parcel/src/Server.js

Line 121 in 3988ffc

`${useHTTPS ? 'https' : 'http'}://localhost:${server.address().port}`

[DeMoorJasper]

@kwmiebach I agree this --host flag would still be usefull for security reasons.
Also the output should probably print listening on port 1234 or Listening on 0.0.0.0:1234 the latter might be confusing for some users though.

It has been requested a lot as well and if we close this again, I think the question will pop up again.

I'm wondering however if we could merge --hmr-hostname and --hostname to just --hostname as if one of those flags get used the user would probably set both of them to the same value anyway

[victorandcode]

I agree with @DeMoorJasper in that the new flag would be useful for security reasons. I'm not sure if merging the two options would be a good idea at this point. Mainly because it would be a breaking change for people already using --hmr-hostname (unless --hmr-hostname gets deprecated). Also, I really don't know if most people prefer having a separate hostname and hmr-hostname (for me it would make sense though).

[devongovett]

Going to close this. The parcel server is not meant to be used in production. I don't really see why this feature would be useful for a development server.

The --hmr-hostname option was added not to control the interfaces the server listens on, but to fix cases where window.location.hostname did not work to get the hostname from the client. See #492.

[ross-pfahler]

I think this would be useful for cases that certain people have with running local servers on different hostnames.

As you know @DeMoorJasper, certain use cases at large companies require a specific hostname, eg localhost.corp.bigcompany.com:1234 to be used due to CORS restrictions.

I'm hoping we can reconsider this PR -- would be helpful in reducing confusion!