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.

Sign up for GitHub

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

Jump to bottom

Using unix domain socket #147

Closed
encendre opened this issue Apr 4, 2020 · 6 comments · Fixed by #150
Closed

Using unix domain socket #147

encendre opened this issue Apr 4, 2020 · 6 comments · Fixed by #150
Milestone
1.0

Comments

@encendre
Copy link

encendre commented Apr 4, 2020 •
edited

Hello, is there any way to use thruster with unix domain sockets?
If they is no way to do that now, I wonder if is possible to add a method .build_from_incoming which use hyper::Server::builder instead hyper::Server::bind for create the underlying hyper::Server

or maybe just a new type of server thruster::UsdHyperServer which impl the ThrusterServer trait but ignoring the port argument, like this:

use thruster::{
    context::basic_hyper_context::{
        generate_context, BasicHyperContext as Ctx, HyperRequest,
    },
    async_middleware, middleware_fn,
    App, Context, ThrusterServer,
    MiddlewareNext, MiddlewareResult,
};

use hyper::{
    service::{make_service_fn, service_fn},
    Body, Request, Response, Server,
    server::accept,
};

use std::sync::Arc;
use async_trait::async_trait;
use tokio::net::UnixListener;

pub struct UdsHyperServer<T: 'static + Context + Send> {
    app: App<HyperRequest, T>,
}

impl<T: 'static + Context + Send> UdsHyperServer<T> { }

#[async_trait]
impl<T: Context<Response = Response<Body>> + Send> ThrusterServer for UdsHyperServer<T> {
    type Context = T;
    type Response = Response<Body>;
    type Request = HyperRequest;

    fn new(app: App<Self::Request, T>) -> Self {
        UdsHyperServer { app }
    }

    async fn build(mut self, path: &str, _port: u16) {
        self.app._route_parser.optimize();

        let arc_app = Arc::new(self.app);

        async move {
            let service = make_service_fn(|_| {
                let app = arc_app.clone();
                async {
                    Ok::<_, hyper::Error>(service_fn(move |req: Request<Body>| {
                        let matched = app.resolve_from_method_and_path(
                            &req.method().to_string(),
                            &req.uri().to_string(),
                        );

                        let req = HyperRequest::new(req);
                        app.resolve(req, matched)
                    }))
                }
            });

            let mut listener = UnixListener::bind(path).unwrap();
            let incoming = listener.incoming();
            let incoming = accept::from_stream(incoming);

            let server = Server::builder(incoming).serve(service);

            server.await?;

            Ok::<_, hyper::Error>(())
        }
        .await
        .expect("hyper server failed");
    }
}

#[middleware_fn]
async fn plaintext(mut context: Ctx, _next: MiddlewareNext<Ctx>) -> MiddlewareResult<Ctx> {
    let val = "Hello, World!";
    context.body(val);
    Ok(context)
}

fn main() {
    println!("Starting server...");

    let mut app = App::<HyperRequest, Ctx>::create(generate_context);

    app.get("/plaintext", async_middleware!(Ctx, [plaintext]));

    let server = UdsHyperServer::new(app);
    server.start("/tmp/thruster.sock", 4321);

    // test the server with the following command:
    // curl --unix-socket /tmp/thruster.sock http://host/plaintext
}
@trezm
Copy link
Collaborator

trezm commented Apr 5, 2020

Yep, I very much like this functionality. I have a few thoughts about this. It looks to me like tokio models the incoming streams as separate structs. I don't have enough knowledge about why they're doing this, probably because of tcp vs unix data streams, but I bet they have good reasons.

With that in mind, I'd like to propose adding build_on_unix_socket to the HyperServer impl for now.

    async fn build_on_unix_socket(self, socket_addr: &str);

This could then be used just like start does with build internally, that is

        let server = HyperServer::new(app);
        tokio::runtime::Runtime::new()
            .unwrap()
            .block_on(server.build_on_unix_socket("/tmp/thruster.sock"))

Thoughts? This seems less invasive than adding it to a trait, and less work than maintaining a third set of ThrusterServer implementations.

@trezm trezm added this to the 1.0 milestone Apr 5, 2020
@encendre
Copy link
Author

encendre commented Apr 5, 2020 •
edited

I've been thinking about it.
I think a better solution would be to implement a method that allows the application to build on any hyper server configuration.

async fn build_on_hyper_builder(mut self, builder: hyper::server::Builder)

and then we just have to use this new method for writing the build method required by the ThrusterServer trait, like this

    async fn build(mut self, host: &str, port: u16) {
        let addr = (host, port).to_socket_addrs().unwrap().next().unwrap();
        let builder = Server::bind(&addr);
        self.build_on_hyper_builder(builder);
    }

This seems it covers more use cases, is still easy to build on unix socket for my use case

            let mut listener = tokio::net::UnixListener::bind("/tmp/thruster.sock").unwrap();
            let incoming = listener.incoming();
            let incoming = hyper::server::accept::from_stream(incoming);
            let builder = Server::builder(incoming);
            
            let server = HyperServer::new(app);
            tokio::runtime::Runtime::new()
                .unwrap()
                .block_on(server.build_on_hyper_builder(builder));

@trezm
Copy link
Collaborator

trezm commented Apr 5, 2020 via email

@encendre
Copy link
Author

encendre commented Apr 6, 2020

I tried to implement it but since I'm pretty new to rust I didn't succeed.
I have successfully compiled the crate adding the new method with and a few tricks that seem unnatural to me...
But the example I wrote doesn't work.

@trezm
Copy link
Collaborator

trezm commented Apr 8, 2020

I can try this out today, not to worry! I'll post the PR here when it's done so you can compare with what you were doing and hopefully see where you were going astray :)

trezm added a commit that referenced this issue Apr 9, 2020
@trezm
feat: Add unix hyper server
fefc0ab 
Adds a new server, `UnixHyperServer` that allows users to use local Unix sockets. Resolves [#147](#147)
@trezm trezm mentioned this issue Apr 9, 2020
Merged
@trezm
Copy link
Collaborator

trezm commented Apr 9, 2020

So after some noodling, I realized that hyper doesn't actually natively support unix sockets! I had to use hyperlocal in order to get all of the traits on the correct Unix streams.

With that in mind, I thought it easiest to add hyperlocal as an optional dependency and hide the whole thing behind a feature flag. This means that I actually went with the original solution you proposed @encendre, of implementing another server and ignoring the port. Not the most elegant solution, but will definitely work for now!

@trezm trezm closed this as completed in #150 Apr 9, 2020
trezm added a commit that referenced this issue Apr 9, 2020
@trezm
feat: Add unix hyper server (#150)
ef77cf1 
Adds a new server, `UnixHyperServer` that allows users to use local Unix sockets. Resolves [#147](#147)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0
Development

Successfully merging a pull request may close this issue.

feat: Add unix hyper server thruster-rs/Thruster
2 participants
@trezm @encendre