dutchcoders / transfer.sh

Easy and fast file sharing from the command-line.
https://github.com/dutchcoders/transfer.sh
MIT License
15.09k stars 1.54k forks source link

--listener should allow specifying the network interface too, not just the port #522

Closed mpl closed 1 year ago

mpl commented 1 year ago

As is customary with such an option for a server, when the interface is not specified, such as in --listener :8080, then we should listen on all interfaces, but otherwise it should be taken into account.

For example: --listener localhost:8080, or --listener 192.168.0.12:8080, etc.

mpl commented 1 year ago

Nevermind, I was fooled by your log line that says "listening on port:" (it should say "listening on addr" or something like that instead), but it actually works since you're passing the option straight as the Addr of the Server.

Sorry about the noise.

aspacca commented 1 year ago

Nevermind, I was fooled by your log line that says "listening on port:" (it should say "listening on addr"

I will change the log entry :)

mpl commented 1 year ago

@aspacca cool.

Btw, on that subject, I would advise you not to ignore the error at

https://github.com/dutchcoders/transfer.sh/blob/main/server/server.go#L549

because you will silently miss errors such as when the port is busy, or we're not allowed to listen on the interface, etc. For example, on my machine, log.Fatal(http.ListenAndServe("localhost:777", nil)) would result in 2023/01/05 10:53:19 listen tcp 127.0.0.1:777: bind: permission denied

But transfer.sh will be happily silent about it:

[transfer.sh]2023/01/05 10:45:06 listening on port: localhost:777
[transfer.sh]2023/01/05 10:45:06 ---------------------------

And the user will never know that the listening actually failed.

aspacca commented 1 year ago

good catch! thanks

btw, feel free to open a PR for it if you have time :)

mpl commented 1 year ago

sure, thanks. I might do that then ;)