I have spent the last couple of evenings learning about Rust and trying it out. Wrote a simple cli calculator as a first thing and thought I would improve it by making it available over http.

I was actually a bit surprised to find that there was no http tooling in the standard library, and searching online gave me an overload of information on different libraries and frameworks.

I ended up implementing my own simple HTTP server, might as well as this is a learning project.

Now I have it working, and while it isn't perfect or done, I thought that this would be a good time to check what things I am doing wrong/badly.

Which is why I am here, would love to get some pointers on it all to make sure I am going in the right direction in the future.

The project is hosted here: https://github.com/Tebro/rsimple_http

you are viewing a single comment's thread
view the rest of the comments

[–] soulsource@discuss.tchncs.de 4 points 10 months ago* (last edited 10 months ago)

I didn't look beyond the main parts of the HTTP moduel, but what I've noticed basically immediately was that your pub fn start_server(address: &str, request_handler: fn(Request) -> Response) -> io::Result<()> uses a function pointer parameter.

This is overly restrictive. fn a()->b only accepts fns, and closures that do not capture their environment (see the book's chapter on closures). In order to accept closures that capture their environment, you could make that function generic: pub fn start_server(address: &str, request_handler: F) -> io::Result<()> where F : Fn(Request)->Response + Clone +'static.

The Clone requirement is necessary, because you need to pass a copy of the handler to each spawned thread.
The 'static lifetime is needed because you can't guarantee that the thread doesn't outlive the call to start_server.

Now, this can be improved further, because Rust offers a tool to guarantee that all threads are joined before run_server returns: Scoped Threads.

Scoped Threads make the 'static requirement unnecessary, because the function that contains them outlives them by definition.
In addition, the Clone requirement is not needed either, because due to the limited lifetimes, you can take request_handler by reference, and all references are Copy (which is a subtrait of Clone).

With scoped threads, a version of your function that could be generic over the request_handler could look something like this (I haven't tried to compile this, it might be utterly wrong):

pub fn start_server(address: &amp;str, request_handler: &amp;F) -> io::Result&amp;&lt;()> where F : Fn(Request) -> Response {
    let listener = TcpListener::bind(address)?;
    std::thread::scope(move |s| -> io::Result&lt;()>{
        for stream in listener.incoming() {
            let stream = stream?; // I think ? works here too
            s.spawn(move || {
                handle_connection(stream, &amp;request_handler);
            });
        };
        Ok(())
    })
}

Edit: Sorry for the messed up characters. & should of course just be the ampersand character, and < should just be the less-than character.