From 1cc98da5874b0735de3ee739bf1f7e1e85f7c49f Mon Sep 17 00:00:00 2001 From: Alberto Ruiz <17555470+Az107@users.noreply.github.com> Date: Thu, 10 Apr 2025 11:49:13 +0200 Subject: [PATCH 1/6] add basic http request validations --- src/hteapot/mod.rs | 17 +++++++++-- src/hteapot/request.rs | 69 ++++++++++++++++++------------------------ 2 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/hteapot/mod.rs b/src/hteapot/mod.rs index 588d832..144863a 100644 --- a/src/hteapot/mod.rs +++ b/src/hteapot/mod.rs @@ -180,7 +180,6 @@ impl Hteapot { ) -> Option<()> { let status = socket_data.status.as_mut()?; - // Fix by miky-rola 2025-04-08 // Check if the TTL (time-to-live) for the connection has expired. // If the connection is idle for longer than `KEEP_ALIVE_TTL` and no data is being written, // the connection is gracefully shut down to free resources. @@ -189,6 +188,7 @@ impl Hteapot { return None; } + // Fix by miky-rola 2025-04-08 // If the request is not yet complete, read data from the stream into a buffer. // This ensures that the server can handle partial or chunked requests. if !status.request.done { @@ -211,7 +211,20 @@ impl Hteapot { return None; } status.ttl = Instant::now(); - let _ = status.request.append(buffer[..m].to_vec()); + let r = status.request.append(buffer[..m].to_vec()); + if r.is_err() { + // Fix by Alberto Ruiz 2025-04-10 + // Early return response if not valid request is sended + + println!("Invalid request"); + let error_msg = r.err().unwrap(); + let response = + HttpResponse::new(HttpStatus::BadRequest, error_msg, None).to_bytes(); + let _ = socket_data.stream.write(&response); + let _ = socket_data.stream.flush(); + let _ = socket_data.stream.shutdown(Shutdown::Both); + return None; + } } } } diff --git a/src/hteapot/request.rs b/src/hteapot/request.rs index f00262f..4888dcb 100644 --- a/src/hteapot/request.rs +++ b/src/hteapot/request.rs @@ -44,39 +44,6 @@ impl HttpRequest { }; } - // pub fn body(&mut self) -> Option> { - // if self.has_body() { - // let mut stream = self.stream.as_ref().unwrap(); - // let content_length = self.headers.get("Content-Length")?; - // let content_length: usize = content_length.parse().unwrap(); - // if content_length > self.body.len() { - // let _ = stream.flush(); - // let mut total_read = 0; - // self.body.resize(content_length, 0); - // while total_read < content_length { - // match stream.read(&mut self.body[total_read..]) { - // Ok(0) => { - // break; - // } - // Ok(n) => { - // total_read += n; - // } - // Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => { - // continue; - // } - // Err(_e) => { - // break; - // } - // } - // } - // } - - // Some(self.body.clone()) - // } else { - // None - // } - // } - pub fn set_stream(&mut self, stream: TcpStream) { self.stream = Some(stream); } @@ -125,21 +92,27 @@ impl HttpRequestBuilder { } } - pub fn append(&mut self, buffer: Vec) -> Option { + pub fn append(&mut self, buffer: Vec) -> Result, &'static str> { self.buffer.extend(buffer); self.buffer.retain(|&b| b != 0); while let Some(pos) = self.buffer.windows(2).position(|w| w == b"\r\n") { let line = self.buffer.drain(..pos).collect::>(); self.buffer.drain(..2); - let line_str = String::from_utf8_lossy(&line); + let line_str = match str::from_utf8(line.as_slice()) { + Ok(v) => v.to_string(), + Err(_e) => return Err("No utf-8"), + }; if self.request.path.is_empty() { let parts: Vec<&str> = line_str.split_whitespace().collect(); if parts.len() < 2 { - return None; + return Ok(None); } + if parts.len() != 3 { + return Err("Invalid method + path + version request"); + } self.request.method = HttpMethod::from_str(parts[0]); let path_parts: Vec<&str> = parts[1].split('?').collect(); self.request.path = path_parts[0].to_string(); @@ -158,8 +131,20 @@ impl HttpRequestBuilder { .collect(); } } else if !line_str.is_empty() { - if let Some((key, value)) = line_str.split_once(": ") { - if key.to_lowercase() == "content-length" { + if let Some((key, value)) = line_str.split_once(":") { + let key = key.trim().to_lowercase(); + let value = value.trim(); + if key == "content-length" { + if self.request.headers.get("content-length").is_some() + || self + .request + .headers + .get("transfer-encoding") + .map(|te| te == "chunked") + .unwrap_or(false) + { + return Err("Duplicated content-length"); + } self.body_size = value.parse().unwrap_or(0); } self.request @@ -171,8 +156,12 @@ impl HttpRequestBuilder { self.request.body.append(&mut self.buffer.clone()); if self.request.body.len() == self.body_size { self.done = true; - return Some(self.request.clone()); + return Ok(Some(self.request.clone())); } - None + Ok(None) } } + +#[cfg(test)] +#[test] +fn basic_request() {} From a0393c8322817683f9560b4c9601bd4caf728d45 Mon Sep 17 00:00:00 2001 From: Alberto Ruiz <17555470+Az107@users.noreply.github.com> Date: Thu, 10 Apr 2025 12:16:15 +0200 Subject: [PATCH 2/6] Update .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index f8338f4..a47df83 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ src/.DS_Store .idea/modules.xml .idea/HTeaPot.iml *.log +/battle_test From 9c4b6bbe03673d9eea3388a6c91dbfaad5d2bc62 Mon Sep 17 00:00:00 2001 From: Alberto Ruiz <17555470+Az107@users.noreply.github.com> Date: Thu, 10 Apr 2025 13:47:33 +0200 Subject: [PATCH 3/6] Prevent overload the server with infinite o malformed headers --- src/hteapot/request.rs | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/hteapot/request.rs b/src/hteapot/request.rs index 4888dcb..8caab0b 100644 --- a/src/hteapot/request.rs +++ b/src/hteapot/request.rs @@ -1,6 +1,8 @@ use super::HttpMethod; use std::{collections::HashMap, net::TcpStream, str}; +const MAX_HEADER_SIZE: usize = 1024 * 16; + pub struct HttpRequest { pub method: HttpMethod, pub path: String, @@ -24,7 +26,7 @@ impl HttpRequest { pub fn default() -> Self { HttpRequest { - method: HttpMethod::GET, + method: HttpMethod::Other(String::new()), path: String::new(), args: HashMap::new(), headers: HashMap::new(), @@ -63,6 +65,7 @@ impl HttpRequest { pub struct HttpRequestBuilder { request: HttpRequest, buffer: Vec, + header_done: bool, body_size: usize, pub done: bool, } @@ -78,6 +81,7 @@ impl HttpRequestBuilder { body: Vec::new(), stream: None, }, + header_done: false, body_size: 0, buffer: Vec::new(), done: false, @@ -92,9 +96,28 @@ impl HttpRequestBuilder { } } + fn read_body(&mut self) -> Option<()> { + self.request.body.append(&mut self.buffer.clone()); + if self.request.body.len() == self.body_size { + self.done = true; + return Some(()); + } else { + return None; + } + } + pub fn append(&mut self, buffer: Vec) -> Result, &'static str> { + if !self.header_done && self.buffer.len() > MAX_HEADER_SIZE { + return Err("Entity Too large"); + } self.buffer.extend(buffer); self.buffer.retain(|&b| b != 0); + if self.header_done { + match self.read_body() { + Some(_) => return Ok(Some(self.request.clone())), + None => return Ok(None), + } + } while let Some(pos) = self.buffer.windows(2).position(|w| w == b"\r\n") { let line = self.buffer.drain(..pos).collect::>(); self.buffer.drain(..2); @@ -151,13 +174,14 @@ impl HttpRequestBuilder { .headers .insert(key.to_string(), value.to_string()); } + } else { + self.header_done = true; + match self.read_body() { + Some(_) => return Ok(Some(self.request.clone())), + None => return Ok(None), + } } } - self.request.body.append(&mut self.buffer.clone()); - if self.request.body.len() == self.body_size { - self.done = true; - return Ok(Some(self.request.clone())); - } Ok(None) } } From da416394988ad1ec3bc96cba619a918bde2e745a Mon Sep 17 00:00:00 2001 From: Alberto Ruiz <17555470+Az107@users.noreply.github.com> Date: Fri, 11 Apr 2025 23:18:21 +0200 Subject: [PATCH 4/6] Fix parsing errors and bugs - Now the headers have a maximum (100). - The body read function now will not consume excessive RAM for a malformed request. - Fix keep-alive detection. --- src/hteapot/mod.rs | 5 ++-- src/hteapot/request.rs | 65 ++++++++++++++++++++++++++++++------------ 2 files changed, 49 insertions(+), 21 deletions(-) diff --git a/src/hteapot/mod.rs b/src/hteapot/mod.rs index 144863a..14cec6f 100644 --- a/src/hteapot/mod.rs +++ b/src/hteapot/mod.rs @@ -237,10 +237,9 @@ impl Hteapot { let keep_alive = request .headers - .get("Connection") - .map(|v| v == "keep-alive") + .get("connection") //all headers are turn lowercase in the builder + .map(|v| v.to_lowercase() == "keep-alive") .unwrap_or(false); - if !status.write { let mut response = action(request); if keep_alive { diff --git a/src/hteapot/request.rs b/src/hteapot/request.rs index 8caab0b..4c7f6ef 100644 --- a/src/hteapot/request.rs +++ b/src/hteapot/request.rs @@ -1,8 +1,15 @@ +// Written by Alberto Ruiz 2025-01-01 +// This module handles the request struct and a builder for it +// This implementation has some issues and fixes are required for security +// Refactor is recomended, but for now can work with the fixes + use super::HttpMethod; -use std::{collections::HashMap, net::TcpStream, str}; +use std::{cmp::min, collections::HashMap, net::TcpStream, str}; const MAX_HEADER_SIZE: usize = 1024 * 16; +const MAX_HEADER_COUNT: usize = 100; +#[derive(Debug)] pub struct HttpRequest { pub method: HttpMethod, pub path: String, @@ -66,6 +73,7 @@ pub struct HttpRequestBuilder { request: HttpRequest, buffer: Vec, header_done: bool, + header_size: usize, body_size: usize, pub done: bool, } @@ -81,6 +89,7 @@ impl HttpRequestBuilder { body: Vec::new(), stream: None, }, + header_size: 0, header_done: false, body_size: 0, buffer: Vec::new(), @@ -96,26 +105,43 @@ impl HttpRequestBuilder { } } - fn read_body(&mut self) -> Option<()> { - self.request.body.append(&mut self.buffer.clone()); - if self.request.body.len() == self.body_size { + fn read_body_len(&mut self) -> Option<()> { + let body_left = self.body_size.saturating_sub(self.request.body.len()); + let to_take = min(body_left, self.buffer.len()); + let to_append = &self.buffer[..to_take]; + self.request.body.extend_from_slice(to_append); + self.buffer.drain(..to_take); + + if body_left > 0 { + return None; + } else { self.done = true; return Some(()); - } else { - return None; } } - pub fn append(&mut self, buffer: Vec) -> Result, &'static str> { + fn read_body_chunk(&mut self) -> Option<()> { + //TODO: this will support chunked body in the future + todo!() + } + + fn read_body(&mut self) -> Option<()> { + return self.read_body_len(); + } + + pub fn append(&mut self, chunk: Vec) -> Result<(), &'static str> { if !self.header_done && self.buffer.len() > MAX_HEADER_SIZE { return Err("Entity Too large"); } - self.buffer.extend(buffer); - self.buffer.retain(|&b| b != 0); + let chunk_size = chunk.len(); + self.buffer.extend(chunk); if self.header_done { - match self.read_body() { - Some(_) => return Ok(Some(self.request.clone())), - None => return Ok(None), + self.read_body(); + return Ok(()); + } else { + self.header_size += chunk_size; + if self.header_size > MAX_HEADER_SIZE { + return Err("Entity Too large"); } } while let Some(pos) = self.buffer.windows(2).position(|w| w == b"\r\n") { @@ -130,7 +156,7 @@ impl HttpRequestBuilder { if self.request.path.is_empty() { let parts: Vec<&str> = line_str.split_whitespace().collect(); if parts.len() < 2 { - return Ok(None); + return Ok(()); } if parts.len() != 3 { @@ -155,6 +181,11 @@ impl HttpRequestBuilder { } } else if !line_str.is_empty() { if let Some((key, value)) = line_str.split_once(":") { + //Check the number of headers, if the actual headers exceed that number + //drop the connection + if self.request.headers.len() > MAX_HEADER_COUNT { + return Err("Header number exceed allowed"); + } let key = key.trim().to_lowercase(); let value = value.trim(); if key == "content-length" { @@ -176,13 +207,11 @@ impl HttpRequestBuilder { } } else { self.header_done = true; - match self.read_body() { - Some(_) => return Ok(Some(self.request.clone())), - None => return Ok(None), - } + self.read_body(); + return Ok(()); } } - Ok(None) + Ok(()) } } From d3a8157fe634cc30aea226e65287260d0c046c92 Mon Sep 17 00:00:00 2001 From: Alberto Ruiz <17555470+Az107@users.noreply.github.com> Date: Sat, 12 Apr 2025 12:39:37 +0200 Subject: [PATCH 5/6] Update .gitignore --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index a47df83..f8338f4 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,3 @@ src/.DS_Store .idea/modules.xml .idea/HTeaPot.iml *.log -/battle_test From fbceab39eed3f66ed4ce1237c6fc0bc9fc6dea25 Mon Sep 17 00:00:00 2001 From: Alberto Ruiz <17555470+Az107@users.noreply.github.com> Date: Sat, 12 Apr 2025 12:42:26 +0200 Subject: [PATCH 6/6] remove unnecesary println! and comments --- src/hteapot/mod.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/hteapot/mod.rs b/src/hteapot/mod.rs index 14cec6f..87ef9c2 100644 --- a/src/hteapot/mod.rs +++ b/src/hteapot/mod.rs @@ -180,6 +180,7 @@ impl Hteapot { ) -> Option<()> { let status = socket_data.status.as_mut()?; + // Fix by miky-rola 2025-04-08 // Check if the TTL (time-to-live) for the connection has expired. // If the connection is idle for longer than `KEEP_ALIVE_TTL` and no data is being written, // the connection is gracefully shut down to free resources. @@ -187,8 +188,6 @@ impl Hteapot { let _ = socket_data.stream.shutdown(Shutdown::Both); return None; } - - // Fix by miky-rola 2025-04-08 // If the request is not yet complete, read data from the stream into a buffer. // This ensures that the server can handle partial or chunked requests. if !status.request.done { @@ -213,10 +212,7 @@ impl Hteapot { status.ttl = Instant::now(); let r = status.request.append(buffer[..m].to_vec()); if r.is_err() { - // Fix by Alberto Ruiz 2025-04-10 // Early return response if not valid request is sended - - println!("Invalid request"); let error_msg = r.err().unwrap(); let response = HttpResponse::new(HttpStatus::BadRequest, error_msg, None).to_bytes();