My first addition was to add the ability to return any existing html file based on the user URI request as well as gracefully handle cases where no user defined 404.html exists. However, I have a feeling what I wrote can be done much "better". Any feedback would be appreciated. I only include the code I modified. The rest is the same as the listing at the end of chapter 20.3.
PS; No feedback on the use of unwrap() needed
fn handle_connection(mut stream: TcpStream) {
let mut buffer = [0; 1024];
stream.read(&mut buffer).unwrap();
let request = std::str::from_utf8(&buffer).unwrap();
let parts: Vec<&str> = request.split(' ').collect();
let uri = parts.get(1).unwrap().clone();
let mut file = String::from(".");
file.push_str(uri);
let (status, content) = get_content(&file);
let response = format!(
"{}\r\nContent-Length: {}\r\n\r\n{}",
status,
content.len(),
content
);
stream.write(response.as_bytes()).unwrap();
stream.flush().unwrap();
}
fn get_content(file: &str) -> (String, String) {
let mut status = String::from("HTTP/1.1 200 OK");
let content = fs::read_to_string(file);
let content = match content {
Ok(c) => c,
Err(_) => {
status = String::from("HTTP/1.1 404 Not Found");
match fs::read_to_string("./404.html") {
Ok(c) => c,
Err(_) => String::from("Not Found")
}
}
};
(status, content)
}
I'm finding it a bit hard to follow the flow of get_content. It'll read easier if you:
Get rid of the mut on status. Instead of setting status to one value and then changing it if the request fails you can have two different let bindings. As a general rule Rust doesn't require nearly as many mutable variables as other languages do. Multiple let bindings are often a better option.
I would turn the match block into the return value. Have the Ok and Err cases yield the two different string pairs that could be returned. That makes it the "either 200 or 404" nature of the function more apparent.
fn get_content(file: &str) -> (String, String) {
match fs::read_to_string(file) {
Ok(content) => {
let status = String::from("HTTP/1.1 200 OK");
(status, content)
}
Err(_) => {
let status = String::from("HTTP/1.1 404 Not Found");
let content = fs::read_to_string("./404.html")
.unwrap_or_else(|_| String::from("Not Found"));
(status, content)
}
}
}
I don't recommend it if you're concatenating many segments together since it'll wasteful temporary PathBuf objects, but for two segments it's fine.
Building paths with Path and PathBuf is nicer than bare string manipulation. But be careful to strip the leading / from the right hand path. If the path being added is absolute it replaces the left hand path instead of being appended.
