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

Supply binding interface in .env #74

Open
windware-ono opened this issue Jan 25, 2018 · 4 comments
Open

Supply binding interface in .env #74

windware-ono opened this issue Jan 25, 2018 · 4 comments

Comments

@windware-ono
Copy link
Contributor

@windware-ono windware-ono commented Jan 25, 2018

Summary

.env file needs an entry to supply the binding interface for 'listen'.
It currently forces to listen on all interfaces exposing port 3000 globally without a firewall blocking it.

Also, you should check to make sure APP_PORT is an integer or else it starts out as listening on port NaN and the process stays alive.

Steps to Reproduce

  1. Install as normal
  2. node app

Additional info

  • Postleaf version: 1.0.0-beta.1
  • Node version: 8.9.4
  • Affected browsers:
  • Operating system: Ubuntu 16.04
@claviska
Copy link
Member

@claviska claviska commented Jan 27, 2018

Would you care to give an example? I.E. what's your preferred method to resolve this?

@windware-ono
Copy link
Contributor Author

@windware-ono windware-ono commented Jan 27, 2018

Perhaps, provide

APP_BIND=127.0.0.1

in .env and simply

app.listen(process.env.APP_PORT, () => {

becomes

app.listen(process.env.APP_PORT, process.env.APP_BIND, () => {

@claviska
Copy link
Member

@claviska claviska commented Jan 27, 2018

Ok. I'll be happy to accept a PR that looks for the APP_HOST environmental variable. If present, it binds to that host. Otherwise, the current behavior stays the same.

I'm suggesting APP_HOST in lieu of APP_BIND because its also the argument name for server.listen() in the docs and may be more clear to users.

@claviska
Copy link
Member

@claviska claviska commented Jan 29, 2018

Fixed in #77. Only thing left here:

  • Add APP_HOST to docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.