From 10769ac8cc1c55d9c7fbc3ceac0e9c11342e5f0a Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 9 Apr 2024 15:33:01 +0200 Subject: [PATCH] fix(server): log error when failing to read file (#1788) * fix(server): log error when failing to read file As part of the QUIC Interop testcases, a server needs to be able to respond to a request with the content of a local file. Previously, when failing to read a file, the server would simply reply with the predefined `SELF::MESSAGE`. Now the server logs the error and moves on. * Send 404 --- neqo-bin/src/server/mod.rs | 75 +++++++++++++------------------- neqo-bin/src/server/old_https.rs | 33 ++++++++------ 2 files changed, 50 insertions(+), 58 deletions(-) diff --git a/neqo-bin/src/server/mod.rs b/neqo-bin/src/server/mod.rs index e3067ecdf0..3490b3e9b3 100644 --- a/neqo-bin/src/server/mod.rs +++ b/neqo-bin/src/server/mod.rs @@ -10,8 +10,7 @@ use std::{ cmp::min, collections::HashMap, fmt::{self, Display}, - fs::OpenOptions, - io::{self, Read}, + fs, io, net::{SocketAddr, ToSocketAddrs}, path::PathBuf, pin::Pin, @@ -188,28 +187,11 @@ impl Args { } } -fn qns_read_response(filename: &str) -> Option> { - let mut file_path = PathBuf::from("/www"); - file_path.push(filename.trim_matches(|p| p == '/')); - - OpenOptions::new() - .read(true) - .open(&file_path) - .map_err(|_e| qerror!("Could not open {}", file_path.display())) - .ok() - .and_then(|mut f| { - let mut data = Vec::new(); - match f.read_to_end(&mut data) { - Ok(sz) => { - qinfo!("{} bytes read from {}", sz, file_path.display()); - Some(data) - } - Err(e) => { - qerror!("Error reading data: {e:?}"); - None - } - } - }) +fn qns_read_response(filename: &str) -> Result, io::Error> { + let path: PathBuf = ["/www", filename.trim_matches(|p| p == '/')] + .iter() + .collect(); + fs::read(path) } trait HttpServer: Display { @@ -344,27 +326,32 @@ impl HttpServer for SimpleServer { continue; } - let mut response = - if let Some(path) = headers.iter().find(|&h| h.name() == ":path") { - if args.shared.qns_test.is_some() { - if let Some(data) = qns_read_response(path.value()) { - ResponseData::from(data) - } else { - ResponseData::from(Self::MESSAGE) - } - } else if let Ok(count) = - path.value().trim_matches(|p| p == '/').parse::() - { - ResponseData::repeat(Self::MESSAGE, count) - } else { - ResponseData::from(Self::MESSAGE) + let Some(path) = headers.iter().find(|&h| h.name() == ":path") else { + stream + .cancel_fetch(neqo_http3::Error::HttpRequestIncomplete.code()) + .unwrap(); + continue; + }; + + let mut response = if args.shared.qns_test.is_some() { + match qns_read_response(path.value()) { + Ok(data) => ResponseData::from(data), + Err(e) => { + qerror!("Failed to read {}: {e}", path.value()); + stream + .send_headers(&[Header::new(":status", "404")]) + .unwrap(); + stream.stream_close_send().unwrap(); + continue; } - } else { - stream - .cancel_fetch(neqo_http3::Error::HttpRequestIncomplete.code()) - .unwrap(); - continue; - }; + } + } else if let Ok(count) = + path.value().trim_matches(|p| p == '/').parse::() + { + ResponseData::repeat(Self::MESSAGE, count) + } else { + ResponseData::from(Self::MESSAGE) + }; stream .send_headers(&[ diff --git a/neqo-bin/src/server/old_https.rs b/neqo-bin/src/server/old_https.rs index 505a16578f..38f3fdc3a7 100644 --- a/neqo-bin/src/server/old_https.rs +++ b/neqo-bin/src/server/old_https.rs @@ -8,7 +8,7 @@ use std::{ cell::RefCell, collections::HashMap, fmt::Display, path::PathBuf, rc::Rc, time::Instant, }; -use neqo_common::{event::Provider, hex, qdebug, qinfo, qwarn, Datagram}; +use neqo_common::{event::Provider, hex, qdebug, qerror, qinfo, qwarn, Datagram}; use neqo_crypto::{generate_ech_keys, random, AllowZeroRtt, AntiReplay, Cipher}; use neqo_http3::Error; use neqo_transport::{ @@ -142,20 +142,25 @@ impl Http09Server { Regex::new(r"GET +/(\d+)(?:\r)?\n").unwrap() }; let m = re.captures(msg); - let resp = match m.and_then(|m| m.get(1)) { - None => { - self.save_partial(stream_id, buf, conn); - return; - } - Some(path) => { - let path = path.as_str(); - qdebug!("Path = '{path}'"); - if args.shared.qns_test.is_some() { - qns_read_response(path) - } else { - let count = path.parse().unwrap(); - Some(vec![b'a'; count]) + let Some(path) = m.and_then(|m| m.get(1)) else { + self.save_partial(stream_id, buf, conn); + return; + }; + + let resp = { + let path = path.as_str(); + qdebug!("Path = '{path}'"); + if args.shared.qns_test.is_some() { + match qns_read_response(path) { + Ok(data) => Some(data), + Err(e) => { + qerror!("Failed to read {path}: {e}"); + Some(b"404".to_vec()) + } } + } else { + let count = path.parse().unwrap(); + Some(vec![b'a'; count]) } }; self.write(stream_id, resp, conn);