search

pyvec / elsa

Helper module for hosting Frozen-Flask based websites on GitHub pages
Other
28 stars 49 forks source link

Add --host option for serving and use Flask default if not specified #67

Closed encukou closed 4 years ago

encukou commented 4 years ago

As mentioned in Flask docs, "0.0.0.0" should be used for externally visible servers. It is not a good default for security reasons.

Change the default to Flask's own default (currently 127.0.0.1), and provide a --host option to change it.

I'm also adding some test improvements (though one batch is left in the big commit). Let me know if I should make separate PRs.

Unfortunately, there's no easy way to test the difference between binding to 127.0.0.1 and 0.0.0.0. That would requite knowing an external IP of the current machine, and possibly opening up firewalls.

Related to: https://github.com/pyvec/elsa/issues/66

encukou commented 4 years ago

@hroncok, it took a few attempts to get the tests green, and some changes aren't straightforward. Could you check the new commits?

hroncok commented 4 years ago

All good! Thanks.

frenzymadness commented 4 years ago

Unfortunately, there's no easy way to test the difference between binding to 127.0.0.1 and 0.0.0.0. That would requite knowing an external IP of the current machine, and possibly opening up firewalls.

I have an idea how to test it. Localhost theoretically has 16 millions of IP addresses 127.0.0.0/8 allowing to run multiple apps on the same machine listening on the same port. So, if Elsa listens on 0.0.0.0 the app should be accessible via multiple addresses like 127.0.0.1, 127.0.0.2, or even 127.100.200.254 but when --host 127.0.0.1 is specified, only this one IP address should work.

encukou commented 4 years ago

According to Wikipedia:

any packet sent to [127.0.0.0/8] is looped back. The address 127.0.0.1 is the standard address for IPv4 loopback traffic; the rest are not supported by all operating systems.

I'll keep the idea in mind for the future, but I'll avoid more complications. Hopefully this is now tested enough. Thanks!