Simple Rust Webserver

Question

I started learning Rust today and for my first project created a simple webserver.

I don't have a clear vision about Rust programming and I tried to write it with my previous knowledge in Python and PHP.

use std::io::{Read, Write,BufReader, BufRead};
use std::net::{TcpListener, TcpStream};
use std::fs::File;
use std::path::Path;
fn main(){
 loop{
 let listener = TcpListener::bind("localhost:8000").unwrap();
 let stream = listener.accept().unwrap().0;
 read_request(stream);
 }
}
fn read_request(stream: TcpStream){
 let mut lines = String::new();
 let mut reader = BufReader::new(stream);
 reader.read_line(&mut lines);
 let mut vec_line = lines.split_whitespace();
 let mut requested_page = vec_line.nth(1).unwrap().to_string();
 requested_page = requested_page.replace("/","");
 if requested_page == "" {
 requested_page = String::from("index.html");
 }
 let mut response = String::new();
 let path = Path::new(&requested_page);
 println!("{}",requested_page);
 let mut status = 200;
 if !path.exists(){
 response = String::from("Not Found!");
 status = 404;
 } else {
 let mut file = File::open(&requested_page).expect("Unable to open file");
 file.read_to_string(&mut response);
 }
 send_response(reader.into_inner(), &response.to_string(), status);
}
fn send_response(mut stream: TcpStream, res :&str, status :u32){
 let response = format!("{}{}{}{}","HTTP/1.1 ",status," OK\n\n",res);
 stream.write_all(response.as_bytes()).unwrap();
}

Answer 1

1. Learn to love rustfmt:

   1. Spaces go between comma-separated items:

      -use std::io::{Read, Write,BufReader, BufRead};
      +use std::io::{Read, Write, BufReader, BufRead};
      

      -let response = format!("{}{}{}{}","HTTP/1.1 ",status," OK\n\n",res);
      +let response = format!("{}{}{}{}", "HTTP/1.1 ", status, " OK\n\n", res);
      

   2. Spaces go before braces:

      -fn main(){
      +fn main() {
      

      -loop{
      +loop {
      

   3. Colons attach on the left side of type definitions:

      -fn send_response(mut stream: TcpStream, res :&str, status :u32){
      +fn send_response(mut stream: TcpStream, res: &str, status: u32) {
      

2. Pay attention to compiler warnings. One of the entire purposes of using a compiled language is to have the compiler watch your back. If you ignore it, then what good is it?

   warning: unused result which must be used, #[warn(unused_must_use)] on by default
    --> src/main.rs:19:5
    |
   19 | reader.read_line(&mut lines);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   warning: unused result which must be used, #[warn(unused_must_use)] on by default
    --> src/main.rs:35:9
    |
   35 | file.read_to_string(&mut response);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 
   

3. Introduce more functions to break up the distinct parts of read_request. From my inspection, there's a part that parses the request, a part that handles the request by reading a file, and a part that responds (which already exists). Note that doing this contrains the number of lines of code a variable is mutable and avoids the need to recover the inner TcpStream from the BufReader.

4. vec_line is poorly named; it's not a Vec. I am happy to see your use of nth; many people would collect into a Vec, which is inefficient.

5. There's no reason to convert the result of nth into a String, you can call replace on a &str equally as well.

6. Avoid using mut just to be able to reassign a variable (e.g. the result of replace). Instead, just bind the variable a second time. This reduces the amount and scope of mutability.

7. requested_page == "" should be requested_page.is_empty()

8. Why would you convert a String to a String using to_string? Just take a reference to the existing string.

9. There's no need to create a string to write to the stream; just write! directly to the stream.

10. The code always prints "OK", regardless of the status.

11. Checking if a file exists before reading it introduces a race where the file can be deleted between the check and the read. It's better to open the file and check any resulting error.

12. Instead of creating mutable variables (response, status) with defaults, consider only setting them in the appropriate branches. Again, this reduces the number of mutable variables and the scopes of mutability.

13. Once response and status are moved to a function, you can just return them directly.

14. read_request does more than just read, so it should be renamed.

15. The current code doesn't require a TcpStream except in the outermost loop. It only needs anything that implements Read and Write. Using generics will allow you to more easily test those functions in isolation.

use std::io::{self, Read, Write, BufReader, BufRead};
use std::net::TcpListener;
use std::fs::File;
fn main() {
 loop {
 let listener = TcpListener::bind("localhost:8000").unwrap();
 let stream = listener.accept().unwrap().0;
 handle_request(stream);
 }
}
fn handle_request<S>(mut stream: S)
 where S: Read + Write
{
 let requested_page = parse_request(&mut stream);
 let (response, status) = read_file(&requested_page);
 send_response(stream, &response, status);
}
fn parse_request<R>(stream: R) -> String
 where R: Read
{
 let mut line = String::new();
 let mut reader = BufReader::new(stream);
 reader.read_line(&mut line).unwrap();
 let mut words = line.split_whitespace();
 let requested_page = words.nth(1).unwrap();
 let requested_page = requested_page.replace("/", "");
 if requested_page.is_empty() {
 String::from("index.html")
 } else {
 requested_page
 }
}
fn read_file(requested_page: &str) -> (String, u32) {
 match File::open(&requested_page) {
 Ok(mut file) => {
 let mut s = String::new();
 file.read_to_string(&mut s).unwrap();
 (s, 200)
 }
 Err(ref e) if e.kind() == io::ErrorKind::NotFound => (String::from("Not Found!"), 404),
 Err(e) => panic!("Unable to open file: {}", e),
 }
}
fn send_response<W>(mut stream: W, res: &str, status: u32)
 where W: Write
{
 write!(&mut stream, "{}{}{}{}", "HTTP/1.1 ", status, " OK\n\n", res).unwrap();
}

There are way too many uses of unwrap. At the very least, use expect so you have some clue about what caused the program to terminate. Even better, use Result for error handling.

As a basic step, returning a Box<Error> is extremely simple and allows you to consolidate your error handling:

use std::io::{self, Read, Write, BufReader, BufRead};
use std::net::TcpListener;
use std::fs::File;
type Error = Box<std::error::Error>;
type Result<T> = std::result::Result<T, Error>;
fn main() {
 loop {
 if let Err(e) = handle_one("localhost:8000") {
 panic!("An error occurred, I'm exiting! {}", e);
 }
 }
}
fn handle_one(address: &str) -> Result<()> {
 let listener = TcpListener::bind(address)?;
 let stream = listener.accept()?.0;
 handle_request(stream)
}
fn handle_request<S>(mut stream: S) -> Result<()>
 where S: Read + Write
{
 let requested_page = parse_request(&mut stream)?;
 let (response, status) = read_file(&requested_page)?;
 send_response(stream, &response, status)
}
fn parse_request<R>(stream: R) -> Result<String>
 where R: Read,
{
 let mut line = String::new();
 let mut reader = BufReader::new(stream);
 reader.read_line(&mut line)?;
 let mut words = line.split_whitespace();
 let requested_page = words.nth(1).ok_or("request missing path")?;
 let requested_page = requested_page.replace("/", "");
 if requested_page.is_empty() {
 Ok(String::from("index.html"))
 } else {
 Ok(requested_page)
 }
}
fn read_file(requested_page: &str) -> Result<(String, u32)> {
 match File::open(&requested_page) {
 Ok(mut file) => {
 let mut s = String::new();
 file.read_to_string(&mut s)?;
 Ok((s, 200))
 },
 Err(ref e) if e.kind() == io::ErrorKind::NotFound => {
 Ok((String::from("Not Found!"), 404))
 }
 Err(e) => Err(e.into()),
 }
}
fn send_response<W>(mut stream: W, res: &str, status: u32) -> Result<()>
 where W: Write,
{
 write!(&mut stream, "{}{}{}{}", "HTTP/1.1 ", status, " OK\n\n", res)?;
 Ok(())
}