Getting into rust and could use some pointers

Greetings to all.

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: github.com/Tebro/rsimple_http

RunAwayFrog,

My quick notes which are tailored to beginners:

Use Option::ok_or_else() and Result::map_err() instead of let … else.

  • let … else didn’t always exist. And you might find that some old timers are slightly triggered by it.
  • Functional style is generally preferred, as long as it doesn’t effectively become a code obfuscater, like over-using Options as iterators (yes Options are iterators).
  • Familiarize yourself with the https://doc.rust-lang.org/nightly/std/ops/trait.Try.html

Type inference and generic params


<span style="color:#323232;">let headers: HashMap = header_pairs
</span><span style="color:#323232;">        .iter()
</span><span style="color:#323232;">        .map(|line| line.split_once(":").unwrap())
</span><span style="color:#323232;">        .map(|(k, v)| (k.trim().to_string(), v.trim().to_string()))
</span><span style="color:#323232;">        .collect();
</span>

(Borken sanitization will probably butcher this code, good thing the problem will be gone in Lemmy 0.19)

Three tips here:

  1. You don’t need to annotate the type here because it can be inferred since headers will be returned as a struct field, the type of which is already known.
  2. In this pattern, you should know that you can provide the collected type as a generic param to collect() itself. That may prove useful in other scenarios.
  3. You should know that you can collect to a if the iterator items are Results/Options. So that .unwrap() is not an ergonomic necessity 😉

Minor point

  • Use .into() or .to_owned() for &amp;str => String conversions.
    • Again, some pre-specialization old timers may get slightly triggered by it.

make good use of the crate echo system

  • It’s important to make good use of the crate echo system, and talking to people is important in doing that correctly and efficiently.
    • This post is a good start.
    • More specifically, the http crate is the compatibility layer used HTTP rust implementations. Check it out and maybe incorporate it into your experimental/educational code.

Alright, I will stop myself here.

tebro,

Thanks! Really good points here, will have to find some time to apply them.

soulsource,
@soulsource@discuss.tchncs.de avatar

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: &amp;str, request_handler: fn(Request) -> Response) -> io::Result&lt;()> 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: &amp;str, request_handler: F) -> io::Result&lt;()> 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):


<span style="font-weight:bold;color:#a71d5d;">pub fn </span><span style="font-weight:bold;color:#795da3;">start_server</span><span style="color:#323232;">(address: </span><span style="font-weight:bold;color:#a71d5d;">&</span><span style="color:#323232;">amp;</span><span style="font-weight:bold;color:#a71d5d;">str</span><span style="color:#323232;">, request_handler: </span><span style="font-weight:bold;color:#a71d5d;">&</span><span style="color:#323232;">amp;F) -> io::Result</span><span style="font-weight:bold;color:#a71d5d;">&</span><span style="color:#323232;">amp;</span><span style="font-weight:bold;color:#a71d5d;">&</span><span style="color:#323232;">lt;()</span><span style="font-weight:bold;color:#a71d5d;">> where</span><span style="color:#323232;"> F : </span><span style="color:#0086b3;">Fn</span><span style="color:#323232;">(Request) -> Response {
</span><span style="color:#323232;">    </span><span style="font-weight:bold;color:#a71d5d;">let</span><span style="color:#323232;"> listener </span><span style="font-weight:bold;color:#a71d5d;">= </span><span style="color:#323232;">TcpListener::bind(address)</span><span style="font-weight:bold;color:#a71d5d;">?</span><span style="color:#323232;">;
</span><span style="color:#323232;">    std::thread::scope(</span><span style="font-weight:bold;color:#a71d5d;">move |</span><span style="color:#323232;">s</span><span style="font-weight:bold;color:#a71d5d;">| </span><span style="color:#323232;">-> io::Result</span><span style="font-weight:bold;color:#a71d5d;">&</span><span style="color:#323232;">lt;()</span><span style="font-weight:bold;color:#a71d5d;">></span><span style="color:#323232;">{
</span><span style="color:#323232;">        </span><span style="font-weight:bold;color:#a71d5d;">for</span><span style="color:#323232;"> stream </span><span style="font-weight:bold;color:#a71d5d;">in</span><span style="color:#323232;"> listener.</span><span style="color:#62a35c;">incoming</span><span style="color:#323232;">() {
</span><span style="color:#323232;">            </span><span style="font-weight:bold;color:#a71d5d;">let</span><span style="color:#323232;"> stream </span><span style="font-weight:bold;color:#a71d5d;">=</span><span style="color:#323232;"> stream</span><span style="font-weight:bold;color:#a71d5d;">?</span><span style="color:#323232;">; </span><span style="font-style:italic;color:#969896;">// I think ? works here too
</span><span style="color:#323232;">            s.</span><span style="color:#62a35c;">spawn</span><span style="color:#323232;">(</span><span style="font-weight:bold;color:#a71d5d;">move || </span><span style="color:#323232;">{
</span><span style="color:#323232;">                </span><span style="color:#62a35c;">handle_connection</span><span style="color:#323232;">(stream, </span><span style="font-weight:bold;color:#a71d5d;">&</span><span style="color:#323232;">amp;request_handler);
</span><span style="color:#323232;">            });
</span><span style="color:#323232;">        };
</span><span style="color:#323232;">        </span><span style="color:#0086b3;">Ok</span><span style="color:#323232;">(())
</span><span style="color:#323232;">    })
</span><span style="color:#323232;">}
</span>

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

jameseb,

A few things I noticed:

  • In http::request::parse(), do you actually need a BufReader? It would be better to make it generic over something implementing BufRead, that allows what you have but also makes tests and examples easier since you wouldn’t have to open a TCP connection just to do something that is essentially string parsing.
  • In http::response::Response::to_string(), that match on lines 78-85 makes me uneasy, because you are silently changing the status code if it isn’t one you recognise. It would be better to signal an error. It would be even better to just check when the status code is set (perhaps with a status code enum to list the ones you support, since what you have isn’t all the defined codes) so that you can’t fail when converting to a string.
  • Consider whether you need a special to_string() method at all, or whether you can just implement Display (which gives you to_string() for free via the ToString trait).
  • You are using String as an error type pretty much everywhere. The better approach is to create an enum representing all the possible errors, so that a user of your library can match against them. Make the enum implement Error and Display and it will fit fine into the rest of the error handling infrastructure. There are crates like thiserror that can reduce the boilerplate around this.
  • You have an io.rs that doesn’t appear to be connected to anything.
  • You have a main.rs, which seems off in something that sounds like it should be purely a library crate. You probably want that to be an example or an integration test instead.

That’s all I could see with a quick look. In terms of general advice: remember to look at warnings, run cargo clippy, and look at the API guidelines.

tebro,

Thanks for the great points.

  • Using the BufRead trait sounds like a good improvement!
  • Yes, this is a stupid temp thing that I have to fix once I get better errors in place. Which you also had some good ideas on :)
  • Good idea, should be helpful
  • As mentioned above, this sounds great!
  • Yup, left over from the initial CLI application
  • Yeah it is there as an example, need to look into how examples are better set up
jameseb,

I’m glad my points we’re helpful!

There is some documentation on examples in the Cargo book. The basic procedure is to put it in an examples directory alongside the src directory (and in its own subfolder if it has multiple files), and you can add an entry for it in the Cargo.toml (although it should automatically detect it if you put it in the examples directory, so that is only strictly necessary if you want to change the default settings).

tebro,

Yeah I moved it over and it got a lot nicer, nice to have this type of thing built in to cargo.

anlumo,

Please don’t use pointers in Rust!

Anyways, my feedback:

Your http module just wraps another inline module, this is unnecessary. Usually inline modules are only useful for tests. Your readline implementation doesn’t account for systems with two-character line endings (Windows and DOS) and I’m also unclear on why you need that for stdin.

Concerning http in the standard library, Rust has learned from the mess in python, which has multiple implementations in the standard library that are all outdated but can’t be updated due to needing to handle backwards compatibility. Rust only has the basic stuff there, and handles other needs by external dependencies, which are managed by a great package manager (again, unlike python) on crates.io.

tebro,

ah the IO module is left over from the initial CLI calculator. Will have to clean that out at some point.

And the inline server module is also left over from when I was writing everything in the same file first before splitting out.

Good catches! Thanks

KISSmyOS,

*ptr

Fal,
@Fal@yiffit.net avatar

I’m surprised that you’re surprised that there’s not http servers in the standard library

tebro,

I guess that it makes sense. I’ve been doing Go for the past two years.

stifle867,

You’re not crazy. HTTP is commonly part of standard libraries nowadays.

taladar,

Only in languages that do not consider long term support before adding stuff to the standard library.

There really isn’t a good reason to add things like protocols that change significantly over time in a standard library.

Jummit,

I doubt TCP/UDP or basic HTTP requests will change much, but I guess it depends on how high-level the API is.

sugar_in_your_tea,

HTTP has changed quite a bit from HTTP/1.0 -> 2.0.

  • All
  • Subscribed
  • Moderated
  • Favorites
  • •
  • rust@programming.dev
  • DreamBathrooms
  • everett
  • osvaldo12
  • magazineikmin
  • thenastyranch
  • rosin
  • normalnudes
  • Youngstown
  • Durango
  • slotface
  • ngwrru68w68
  • kavyap
  • mdbf
  • InstantRegret
  • JUstTest
  • ethstaker
  • GTA5RPClips
  • tacticalgear
  • Leos
  • anitta
  • modclub
  • khanakhh
  • cubers
  • cisconetworking
  • megavids
  • provamag3
  • tester
  • lostlight
  • All magazines
    •