search

mattdesl / budo

:clapper: a dev server for rapid prototyping
MIT License
2.18k stars 106 forks source link

Bind to all IPs #135

Closed jacwright closed 7 years ago

jacwright commented 8 years ago

This allows the IP to be hit from any IP so livereload can be accessed from external IPs while the page can be loaded from internal ones.

This fixes issue https://github.com/mattdesl/budo/issues/134.

mattdesl commented 8 years ago

Thanks for the contribution! But with this PR the --host flag won't work at all.

jacwright commented 8 years ago

Why not? It should still accept connections on the host IP.

mattdesl commented 8 years ago

It seems like this PR basically removes the functionality of the --host flag, since it's no longer getting passed to the server.listen() function. https://nodejs.org/dist/latest-v5.x/docs/api/http.html#http_server_listen_port_hostname_backlog_callback

quantizor commented 8 years ago

I use -h 0.0.0.0 for this

jacwright commented 8 years ago

The host flag will still launch the correct URL when starting.

I thought the host flag was a first attempt to solve some of these issues.

mattdesl commented 8 years ago

Hmm I guess my question now is: when would a user actually want to specify a host to server.listen for a local development server? Why does that feature exist in tools like http-server?

quantizor commented 8 years ago

Like a non-local host? If they changed their /etc/hosts and were verifying functionality that only occurs if window.location.host is a certain value, likely.

jacwright commented 8 years ago

From the http-server page:

It is powerful enough for production usage, but it's simple and hackable enough to be used for testing, local development, and learning.

So maybe they support production usage. Their default is to bind to 0.0.0.0 like @yaycmyk is using. Maybe we kill this PR and make the default host be 0.0.0.0 instead of guessing the ip.

Once nice little feature http-server provides is listing all the IPs that are bound. This would help mobile to know where to connect when binding to everything. (https://github.com/indexzero/http-server/blob/master/bin/http-server#L130-L136)

mantoni commented 8 years ago

I recently learned that there is a security issue with listening on all ports. The feature makes sense in certain use cases, but maybe it’s a good thing that it’s not enabled by default.

Instead of changing the current behavior, it might make sense to improve the documentation.

jacwright commented 8 years ago

@mantoni I agree with your statement in general, but budo is "a dev server for rapid prototyping" which I believe qualifies for the use cases this feature makes sense for. I wouldn't want this for a production server.

mantoni commented 8 years ago

@jacwright Sure. It’s something I ran into as well and it took a little to figure it out. Since the same was recently reported as a security issue in one of my projects, I just wanted to put this here for consideration. Personally, I don’t have strong feelings.

mattdesl commented 7 years ago

Looking back at this issue again — I think the following would basically solve our issue while still allowing users to specify a host if they desire? (Example here)

   server.listen(opts.port, opts.host || '0.0.0.0', connect)

It's odd to me that passing an undefined host to http is somehow different than using the 2-parameter function. I haven't looked into the Node.js source, though.

mattdesl commented 7 years ago

I think with v10 this is no longer an issue. Please open a new ticket if there's anything still outstanding with the new changes. Thanks!