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

Add support for listening on Unix socket #594

Closed
wants to merge 3 commits into from

Conversation

messense
Copy link
Contributor

@messense messense commented Mar 28, 2018

TODO:

  • Safely delete existing socket on startup
  • Emit error when both address and port are specified(But what if I want to listen on both tcp and unix sockets?)
  • TLS over unix socket

Closes #545

@messense
Copy link
Contributor Author

I am using the hello_world example to test it, added a Rocket.toml file with address set to unix:///tmp/hello.sock in development section, run cargo run to start the server, then use curl to send a simple request to it:

$ cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `/Users/messense/Projects/Rocket/target/debug/hello_world`
🔧  Configured for development.
    => address: unix:///tmp/hello.sock
    => port: 8000
    => log: normal
    => workers: 1
    => secret key: generated
    => limits: forms = 32KiB, json* = 1MiB, msgpack* = 1MiB
    => keep-alive: 5s
    => tls: disabled
    => [extra] hi: "Hello!"
    => [extra] is_extra: true
🛰  Mounting '/':
    => GET / (hello)
🚀  Rocket has launched from http://unix:///tmp/hello.sock:0
$ curl --unix-socket /tmp/hello.sock http://localhost// -v
*   Trying /tmp/hello.sock...
* Connected to localhost (/tmp/hello.sock) port 80 (#0)
> GET // HTTP/1.1
> Host: localhost
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Content-Type: text/plain; charset=utf-8
< Server: Rocket
< Content-Length: 13
< Date: Wed, 28 Mar 2018 05:40:28 GMT
<
* Connection #0 to host localhost left intact
Hello, world!

@@ -16,6 +17,8 @@ pub enum NetStream {
Http(HttpStream),
#[cfg(feature = "tls")]
Https(HttpsStream),
#[cfg(unix)]
UnixHttp(UnixSocketStream),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this, but it is needed to make concrete_stream work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's all very frustrating.

Copy link
Member

@SergioBenitez SergioBenitez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this deal with sockets that don't already exist? What about those that do? I'd like for configuration to fail if a socket already exists and for Rocket to create the socket if it doesn't.

lib/src/rocket.rs Outdated Show resolved Hide resolved
lib/src/rocket.rs Outdated Show resolved Hide resolved
@@ -16,6 +17,8 @@ pub enum NetStream {
Http(HttpStream),
#[cfg(feature = "tls")]
Https(HttpsStream),
#[cfg(unix)]
UnixHttp(UnixSocketStream),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's all very frustrating.

lib/src/config/config.rs Outdated Show resolved Hide resolved
lib/src/data/data.rs Outdated Show resolved Hide resolved
lib/src/data/data.rs Outdated Show resolved Hide resolved
lib/src/rocket.rs Outdated Show resolved Hide resolved
lib/src/unix_socket.rs Outdated Show resolved Hide resolved
@SergioBenitez
Copy link
Member

Awesome, @messense! This is really great. Excited to get this in. :)

@messense
Copy link
Contributor Author

messense commented Mar 28, 2018

How does this deal with sockets that don't already exist?

It will create a new one.

if the socket file exists, it crashes with Address already in use error.

     Running `/Users/messense/Projects/Rocket/target/debug/hello_world`
🔧  Configured for development.
    => address: unix:///tmp/hello.sock
    => port: 8000
    => log: normal
    => workers: 1
    => secret key: generated
    => limits: forms = 32KiB, json* = 1MiB, msgpack* = 1MiB
    => keep-alive: 5s
    => tls: disabled
    => [extra] hi: "Hello!"
    => [extra] is_extra: true
🛰  Mounting '/':
    => GET / (hello)
Error: Rocket failed to launch due to an I/O error.
thread 'main' panicked at 'Address already in use (os error 48)', lib/src/error.rs:207:17
note: Run with `RUST_BACKTRACE=1` for a backtrace.

I think it's the exact behavior of std::unix::net::UnixListener::bind method.

@messense
Copy link
Contributor Author

messense commented Mar 28, 2018

Thanks for the detailed review, I hope to address them soon.

lib/src/config/config.rs Outdated Show resolved Hide resolved
@messense messense force-pushed the unix-socket branch 6 times, most recently from cf53a31 to 4acd52a Compare April 3, 2018 05:54
lib/src/config/error.rs Outdated Show resolved Hide resolved
lib/src/config/builder.rs Outdated Show resolved Hide resolved
lib/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@SergioBenitez SergioBenitez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like more code that depends on whether we're on Unix or not to be contained in the unix module. The cfgs sprinkled around make it harder to follow what the intended flow is.

lib/src/config/address.rs Outdated Show resolved Hide resolved
lib/src/config/builder.rs Outdated Show resolved Hide resolved
lib/src/config/address.rs Outdated Show resolved Hide resolved
lib/src/config/address.rs Outdated Show resolved Hide resolved
lib/src/config/config.rs Outdated Show resolved Hide resolved
lib/src/config/error.rs Outdated Show resolved Hide resolved
lib/src/config/error.rs Outdated Show resolved Hide resolved
lib/src/lib.rs Outdated Show resolved Hide resolved
lib/src/rocket.rs Outdated Show resolved Hide resolved
lib/src/rocket.rs Outdated Show resolved Hide resolved
lib/src/config/config.rs Outdated Show resolved Hide resolved
lib/src/config/config.rs Outdated Show resolved Hide resolved
lib/src/config/config.rs Outdated Show resolved Hide resolved
lib/src/rocket.rs Outdated Show resolved Hide resolved
@messense messense force-pushed the unix-socket branch 6 times, most recently from e62be17 to 466350e Compare April 6, 2018 07:51
@messense
Copy link
Contributor Author

messense commented Jul 8, 2019

Unix domain socket support for Windows is added in messense@47c9dea

Tested on Windows 10 version 1903

@SergioBenitez
Copy link
Member

SergioBenitez commented Jul 8, 2019

What happens if you try to use UDS on a unsupported version of Windows? Does Rocket still compile on unsupported Windows versions? Is it simply a runtime error? Which error message do we print if so?

@messense
Copy link
Contributor Author

messense commented Jul 9, 2019

Windows Unix domain socket is created with WSASocketW API using AF_UNIX address family, WSASocketW is an existing Windows API and AF_UNIX is just a int in winapi crate. Rocket should compile fine on unsupported Windows versions but return WSAEAFNOSUPPORT meaning that The specified address family is not supported.

Which error message do we print if so?

I don't have an older Windows system so I can't easily verify that. Help wanted. See #594 (comment)

@messense
Copy link
Contributor Author

messense commented Jul 9, 2019

Side note: curl on Windows seems not supporting unix socket yet, but you can use curl in WSL to access a Windows Unix socket for example: curl --unix-socket /mnt/c/ProgramData/hello.sock http://localhost// -v. Really cool.

@messense
Copy link
Contributor Author

I've created a Windows Server 2016 on Azure which does not support UDS, it failed to launch with error message:

image

Error: Rocket failed to bind network socket to given address/port.
thread 'main' panicked at 'An address incompatible with the requested protocol was used. (os error 10047)', core\lib\src\error.rs:197:17

What do you think? @SergioBenitez @jebrosen

@jebrosen
Copy link
Collaborator

That looks reasonable. We could detect that error and print a custom message but it might not be worth the effort. We can note in the documentation that this only works on Windows 1809 or later.

@messense
Copy link
Contributor Author

We could detect that error and print a custom message but it might not be worth the effort.

Added the detection code here: 3c9e579

@cyberb
Copy link

cyberb commented Sep 26, 2019

Do you plan to merge this?

@jebrosen
Copy link
Collaborator

I need to review this again, but I'm generally in favor of this. If it's backwards-compatible we could do it in 0.4, but I suspect the changes to Address aren't.

For 0.5 the socket and connection code will need to be rewritten, but it can probably be done somewhat more simply. Furthermore, I would like to make the connection handling extensible such that one could implement unix socket handling, FastCGI, and other I/O all without modifying rocket.

@luciusmagn
Copy link

Hello, what's the status on this? I'd love to use Rocket over unix socket.

Thank you

@kotovalexarian
Copy link

kotovalexarian commented Oct 17, 2020

Furthermore, I would like to make the connection handling extensible such that one could implement unix socket handling, FastCGI, and other I/O all without modifying rocket.

In Ruby ecosystem this is done by separating common HTTP server interface (Rack), HTTP server implementations (Puma, Unicorn, etc) and HTTP applications (which may use Ruby on Rails, Sinatra, etc, or no frameworks at all). In other words, every HTTP application is a Rack application, every HTTP server is a Rack server, every HTTP middleware is a Rack middleware. Everything is built around Rack, which is a very thin and common interface.

@vanackere
Copy link

@jebrosen , @SergioBenitez : what is the current status and how would you prefer to have unix-socket support implemented ?
I also have the need for unix socket support for Rocket and wouldn't mind helping if possible...

@jebrosen
Copy link
Collaborator

#594 (comment) is still accurate. Unix socket support requires roughly these steps:

  • The configuration system needs to support specifying a unix socket address. In master, the configuration system has been revamped so this will need some tweaks.
  • The I/O layer needs to support it. In 0.5, this should now only require an impl Listener for tokio::net::UnixListener similar to impl Listener for tokio::net::TcpListener (https://github.com/SergioBenitez/Rocket/blob/master/core/http/src/listener.rs#L164-L180).
  • When the configured address is a unix socket, rocket must set it up (e.g. deleting a stale socket file) and start listening. This can work largely the same as in this PR, but some of the files have been renamed or moved.

At this point I don't believe there are any more significant changes planned for those areas, and I'd be happy to work on and/or review this soon.

@SergioBenitez, any preferences for timing this breaking change to configuration or handling it in an non-breaking way? As far as I can see, the other two changes could be done separately and even in a minor release.

@cyberb
Copy link

cyberb commented Mar 4, 2021

I know open-source people hate this, but can I offer $50 (I do not have a pound sterling symbol on my keyboard) for this?

@SergioBenitez
Copy link
Member

SergioBenitez commented Jun 30, 2021

This is now wholly outdated. On the one hand, prioritizing this earlier would have prevented staleness. On the other, there are still unresolved questions, and the implementation now would look nothing like the implementation then.

Nevertheless, I've implemented support for a Unix domain socket listener in a local domain branch. The holdback is configuration: how do we configure which listener to use?

There's #1070, which if implemented, would let us say "well, do so programatically." But this isn't particularly satisfying. Starting a Rocket application that listens on a UDS should be as simple as:

ROCKET_ADDRESS="unix:/path/to/socket" cargo run

This is nice and tenable, but the problem is that it ignores the existence of the port configuration parameter. What happens to it if the address is a UDS? The approaches to consider are:

  1. Ignore it. Allow setting a port, but if the address type doesn't use a port, simply don't use it.
  2. Error. If a non-default port is provided for an address type that doesn't make use of it, emit an error. This seems difficult to implement correctly.
  3. Remove port entirely and make it a part of address. So what is today ROCKET_PORT=9000 cargo run would become ROCKET_ADDRESS=127.0.0.1:9000 cargo run. This feels like the "most correct" solution but 1) will almost certainly break almost every deployed Rocket application, and 2) makes changing just the port impossible.

I'm inclined to suggest we follow 1.

In any case, I'm closing out this particular PR as the implementation here has no path forward given the divergence upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for listening on Unix sockets
10 participants