http-server-rs / http-server

Simple and configurable command-line HTTP server
https://crates.io/crates/http-server
Apache License 2.0
168 stars 20 forks source link

[Issue Report] Remove all .unwrap()s #424

Open Antosser opened 5 months ago

Antosser commented 5 months ago

This project contains A LOT of unwraps, which can be easily avoided and removed, making the code much rustier. I'd like to do that after #423 gets merged.

To handle all possible errors, we would need a proper Result type; Result<T, Box<dyn Error>> won't do. We could use anyhow, but I'd suggest color_eyre because of the fancier look when an error happens. Let me know what you think

Here's a list of all unwraps or expects I found using ripgrep:

tests\gzip.rs
10:        let headers = request.headers_mut().unwrap();
15:                http::HeaderValue::from_str(accept_encoding).unwrap(),
21:            .request(request.body(Body::empty()).unwrap())
23:            .unwrap()

tests\cors.rs
9:        client.get(url.parse().unwrap()).await.unwrap()

benches\file_explorer.rs
13:    HTTP_CLIENT.get(uri.parse().unwrap()).await.unwrap();
17:    let rt = Runtime::new().unwrap();
25:    let rt = Runtime::new().unwrap();
34:    let rt = Runtime::new().unwrap();

tests\basic_auth.rs
10:        client.get(url.parse().unwrap()).await.unwrap()
18:        let headers = request.headers_mut().unwrap();
22:            HeaderValue::from_str(credentials.as_http_header().as_str()).unwrap(),
27:            .request(request.body(Body::empty()).unwrap())
29:            .unwrap()

src\utils\url_encode.rs
16:            let component = component.to_str().unwrap();
46:        let file_path = PathBuf::from_str(file_path).unwrap();
59:        let file_path = file_path.to_str().unwrap();

src\cli.rs
81:            host: "127.0.0.1".parse().unwrap(),
85:            root_dir: PathBuf::from_str("./").unwrap(),
88:            tls_cert: PathBuf::from_str("cert.pem").unwrap(),
89:            tls_key: PathBuf::from_str("key.rsa").unwrap(),
119:            host: "0.0.0.0".parse().unwrap(),
136:            host: "192.168.0.1".parse().unwrap(),
148:            root_dir: PathBuf::from_str("~/User/sources/http-server").unwrap(),
235:            tls_cert: PathBuf::from_str("~/secrets/cert").unwrap(),
236:            tls_key: PathBuf::from_str("~/secrets/key").unwrap(),

tests\defacto.rs
9:        client.get(url.parse().unwrap()).await.unwrap()

src\addon\proxy.rs
25:        let upstream = Uri::from_str(upstream).unwrap();
41:            HeaderValue::from_str(self.upstream.authority().unwrap().as_str()).unwrap(),
46:        tokio::spawn(async move { client.request(outogoing).await.unwrap() })
48:            .unwrap()
66:        let via_header = HeaderValue::from_str(&via_header_str).unwrap();
69:            let current_via_header = current_via_header.to_str().unwrap();
84:                HeaderValue::from_str(proxies_list.as_str()).unwrap(),
139:            .unwrap();
152:            HeaderValue::from_str(self.upstream.authority().unwrap().as_str()).unwrap(),
156:        headers.remove(USER_AGENT).unwrap();
163:        let scheme = self.upstream.scheme_str().unwrap();
164:        let authority = self.upstream.authority().unwrap().as_str();
176:            .unwrap()
206:        let via_header_value = headers.get(&HeaderName::from_static("via")).unwrap();
207:        let via_header_value = via_header_value.to_str().unwrap();
220:            HeaderValue::from_str("HTTP/1.1 GoodProxy").unwrap(),
232:        let via_header_value = headers.get(&HeaderName::from_static("via")).unwrap();
233:        let via_header_value = via_header_value.to_str().unwrap();
247:            (CONNECTION, HeaderValue::from_str("keep-alive").unwrap()),

src\addon\cors.rs
98:                HeaderValue::from_str("true").unwrap(),
107:                HeaderValue::from_str(allow_headers.as_str()).unwrap(),
116:                HeaderValue::from_str(allow_methods.as_str()).unwrap(),
123:                HeaderValue::from_str(allow_origin.as_str()).unwrap(),
132:                HeaderValue::from_str(expose_headers.as_str()).unwrap(),
139:                HeaderValue::from_str(max_age.to_string().as_str()).unwrap(),
148:                HeaderValue::from_str(request_headers.as_str()).unwrap(),
155:                HeaderValue::from_str(request_method.as_str()).unwrap(),
375:        assert_eq!(cors, Cors::try_from(config).unwrap());

src\addon\compression\gzip.rs
93:                let mut buffer_cursor = aggregate(body).await.unwrap();
106:                HeaderValue::from_str("gzip").unwrap(),
140:                HeaderValue::from_str("gzip, deflate").unwrap(),
151:        let response = response_builder.body(response_body).unwrap();
160:        let compressed = compress(raw).unwrap();
175:            .unwrap();
183:                .unwrap(),
198:            let mut buffer_cursor = aggregate(body).await.unwrap();
207:            .unwrap();
213:            let mut buffer_cursor = aggregate(compressed_body).await.unwrap();

src\utils\error.rs
19:            .unwrap(),
21:        .unwrap()

src\addon\file_server\query_params.rs
71:        let have = QueryParams::from_str(uri_string).unwrap();

src\config\util\tls.rs
35:        path.to_str().unwrap()
38:    let cert_bytes = &rustls_pemfile::certs(&mut buf_reader).unwrap()[0];
45:        .with_context(|| format!("Unable to find the TLS keys on: {}", path.to_str().unwrap()))?;
49:            let path = path.to_str().unwrap();
54:            let path = path.to_str().unwrap();

src\config\mod.rs
47:        let root_dir = current_dir().unwrap();
73:        let root_dir = if cli_arguments.root_dir.to_str().unwrap() == "./" {
74:            current_dir().unwrap()
76:            let root_dir = cli_arguments.root_dir.to_str().unwrap();
112:                    cli_arguments.username.unwrap(),
113:                    cli_arguments.password.unwrap(),
126:            let proxy_url = cli_arguments.proxy.unwrap();

src\server\mod.rs
48:            let https_config = config.tls.clone().unwrap();
73:            server_task.await.unwrap();
92:            if self.config.address.ip() == Ipv4Addr::from_str("0.0.0.0").unwrap() {
122:        let server = https_server_builder.make_server(address).await.unwrap();
137:            if self.config.address.ip() == Ipv4Addr::from_str("0.0.0.0").unwrap() {

src\config\file.rs
57:    let path = PathBuf::from_str(value).unwrap();
58:    let canon = fs::canonicalize(path).unwrap();
88:        let config = ConfigFile::parse_toml(file_contents).unwrap();
89:        let mut root_dir = std::env::current_dir().unwrap();
112:        ConfigFile::parse_toml(file_contents).unwrap();
129:        let root_dir = Some(std::env::current_dir().unwrap());
131:            cert: PathBuf::from_str("cert_123.pem").unwrap(),
132:            key: PathBuf::from_str("key_123.pem").unwrap(),
135:        let config = ConfigFile::parse_toml(file_contents).unwrap();
140:        assert_eq!(config.tls.unwrap(), tls);
157:        let root_dir = Some(std::env::current_dir().unwrap());
159:            cert: PathBuf::from_str("cert_123.pem").unwrap(),
160:            key: PathBuf::from_str("key_123.pem").unwrap(),
163:        let config = ConfigFile::parse_toml(file_contents).unwrap();
168:        assert_eq!(config.tls.unwrap(), tls);
205:        let config = ConfigFile::parse_toml(file_contents).unwrap();
206:        let root_dir = Some(std::env::current_dir().unwrap());
211:        assert_eq!(config.cors.unwrap(), cors);
232:        let root_dir = Some(std::env::current_dir().unwrap());
253:        let config = ConfigFile::parse_toml(file_contents).unwrap();
258:        assert_eq!(config.cors.unwrap(), cors);
270:        let config = ConfigFile::parse_toml(file_contents).unwrap();
272:        assert!(config.compression.unwrap().gzip);
285:        let config = ConfigFile::parse_toml(file_contents).unwrap();
289:        let basic_auth = config.basic_auth.unwrap();
302:        let config = ConfigFile::parse_toml(file_contents).unwrap();
316:        let config = ConfigFile::parse_toml(file_contents).unwrap();
320:        let proxy = config.proxy.unwrap();
332:        let config = ConfigFile::parse_toml(file_contents).unwrap();
335:        assert!(config.graceful_shutdown.unwrap());

src\addon\file_server\scoped_file_system.rs
153:        let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
167:        let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
175:        let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
183:        let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
196:            normalized.to_str().unwrap(),
203:        let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
204:        let resolved_entry = sfs.resolve(PathBuf::from("assets/logo.svg")).await.unwrap();
215:        let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
216:        let resolved_entry = sfs.resolve(PathBuf::from("assets/")).await.unwrap();
223:        let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
224:        let resolved_entry = sfs.resolve(PathBuf::from("assets")).await.unwrap();
231:        let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();

src\server\handler\mod.rs
49:            let middleware = Middleware::try_from(config).unwrap();
60:        let middleware = Middleware::try_from(config).unwrap();

src\addon\file_server\mod.rs
45:        let scoped_file_system = ScopedFileSystem::new(config.root_dir.clone()).unwrap();
59:        let template = std::str::from_utf8(template).unwrap();
63:            .unwrap();
224:            .unwrap();
254:            .unwrap()
377:        let path = entry_path.strip_prefix(root_dir).unwrap();
393:        let root_dir = PathBuf::from_str("/Users/bob/sources/http-server").unwrap();
396:                .unwrap();
421:            let sanitized_path = FileServer::parse_path(req_uri).unwrap().0;
422:            let wanted_path = PathBuf::from_str(want[idx]).unwrap();
430:        let root_dir = PathBuf::from_str("/Users/bob/sources/http-server").unwrap();
433:                .unwrap();
434:        let breadcrumbs = FileServer::breadcrumbs_from_path(&root_dir, &entry_path).unwrap();

src\server\handler\file_server.rs
32:            let response = self.file_server.resolve(req_path).await.unwrap();

src\server\middleware\cors.rs
27:    let cors = Cors::try_from(cors_config).unwrap();
EstebanBorai commented 5 months ago

Totally agree! Go for it!

EstebanBorai commented 5 months ago

I think having unwraps in tests is fine at least for the needs on this project because in tests we are validating specific scenarios.

If test runs on panic due to unwrap, its a good signal of either:

Both determines failure in any case so I would keep unwraps in tests for simplicity sake.

I cant say the same with end user code/source. No unwraps/cases of panic should exist there.

Antosser commented 5 months ago

@EstebanBorai Please review #423 first to avoid collisions

I think having unwraps in tests is fine at least for the needs on this project because in tests we are validating specific scenarios.

I agree