Skip to content

Commit 4a07df0

Browse files
Az107Az107
authored
Fix/secure parsing (#31)
* add basic http request validations * Update .gitignore * Prevent overload the server with infinite o malformed headers * 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.
1 parent 0351edc commit 4a07df0

2 files changed

Lines changed: 103 additions & 53 deletions

File tree

‎src/hteapot/mod.rs‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,6 @@ impl Hteapot {
188188
let _ = socket_data.stream.shutdown(Shutdown::Both);
189189
return None;
190190
}
191-
192191
// If the request is not yet complete, read data from the stream into a buffer.
193192
// This ensures that the server can handle partial or chunked requests.
194193
if !status.request.done {
@@ -211,7 +210,17 @@ impl Hteapot {
211210
return None;
212211
}
213212
status.ttl = Instant::now();
214-
let _ = status.request.append(buffer[..m].to_vec());
213+
let r = status.request.append(buffer[..m].to_vec());
214+
if r.is_err() {
215+
// Early return response if not valid request is sended
216+
let error_msg = r.err().unwrap();
217+
let response =
218+
HttpResponse::new(HttpStatus::BadRequest, error_msg, None).to_bytes();
219+
let _ = socket_data.stream.write(&response);
220+
let _ = socket_data.stream.flush();
221+
let _ = socket_data.stream.shutdown(Shutdown::Both);
222+
return None;
223+
}
215224
}
216225
}
217226
}
@@ -224,10 +233,9 @@ impl Hteapot {
224233

225234
let keep_alive = request
226235
.headers
227-
.get("Connection")
228-
.map(|v| v == "keep-alive")
236+
.get("connection") //all headers are turn lowercase in the builder
237+
.map(|v| v.to_lowercase() == "keep-alive")
229238
.unwrap_or(false);
230-
231239
if !status.write {
232240
let mut response = action(request);
233241
if keep_alive {

‎src/hteapot/request.rs‎

Lines changed: 90 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
1+
// Written by Alberto Ruiz 2025-01-01
2+
// This module handles the request struct and a builder for it
3+
// This implementation has some issues and fixes are required for security
4+
// Refactor is recomended, but for now can work with the fixes
5+
16
use super::HttpMethod;
2-
use std::{collections::HashMap, net::TcpStream, str};
7+
use std::{cmp::min, collections::HashMap, net::TcpStream, str};
8+
9+
const MAX_HEADER_SIZE: usize = 1024 * 16;
10+
const MAX_HEADER_COUNT: usize = 100;
311

12+
#[derive(Debug)]
413
pub struct HttpRequest {
514
pub method: HttpMethod,
615
pub path: String,
@@ -24,7 +33,7 @@ impl HttpRequest {
2433

2534
pub fn default() -> Self {
2635
HttpRequest {
27-
method: HttpMethod::GET,
36+
method: HttpMethod::Other(String::new()),
2837
path: String::new(),
2938
args: HashMap::new(),
3039
headers: HashMap::new(),
@@ -44,39 +53,6 @@ impl HttpRequest {
4453
};
4554
}
4655

47-
// pub fn body(&mut self) -> Option<Vec<u8>> {
48-
// if self.has_body() {
49-
// let mut stream = self.stream.as_ref().unwrap();
50-
// let content_length = self.headers.get("Content-Length")?;
51-
// let content_length: usize = content_length.parse().unwrap();
52-
// if content_length > self.body.len() {
53-
// let _ = stream.flush();
54-
// let mut total_read = 0;
55-
// self.body.resize(content_length, 0);
56-
// while total_read < content_length {
57-
// match stream.read(&mut self.body[total_read..]) {
58-
// Ok(0) => {
59-
// break;
60-
// }
61-
// Ok(n) => {
62-
// total_read += n;
63-
// }
64-
// Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => {
65-
// continue;
66-
// }
67-
// Err(_e) => {
68-
// break;
69-
// }
70-
// }
71-
// }
72-
// }
73-
74-
// Some(self.body.clone())
75-
// } else {
76-
// None
77-
// }
78-
// }
79-
8056
pub fn set_stream(&mut self, stream: TcpStream) {
8157
self.stream = Some(stream);
8258
}
@@ -96,6 +72,8 @@ impl HttpRequest {
9672
pub struct HttpRequestBuilder {
9773
request: HttpRequest,
9874
buffer: Vec<u8>,
75+
header_done: bool,
76+
header_size: usize,
9977
body_size: usize,
10078
pub done: bool,
10179
}
@@ -111,6 +89,8 @@ impl HttpRequestBuilder {
11189
body: Vec::new(),
11290
stream: None,
11391
},
92+
header_size: 0,
93+
header_done: false,
11494
body_size: 0,
11595
buffer: Vec::new(),
11696
done: false,
@@ -125,21 +105,63 @@ impl HttpRequestBuilder {
125105
}
126106
}
127107

128-
pub fn append(&mut self, buffer: Vec<u8>) -> Option<HttpRequest> {
129-
self.buffer.extend(buffer);
130-
self.buffer.retain(|&b| b != 0);
108+
fn read_body_len(&mut self) -> Option<()> {
109+
let body_left = self.body_size.saturating_sub(self.request.body.len());
110+
let to_take = min(body_left, self.buffer.len());
111+
let to_append = &self.buffer[..to_take];
112+
self.request.body.extend_from_slice(to_append);
113+
self.buffer.drain(..to_take);
114+
115+
if body_left > 0 {
116+
return None;
117+
} else {
118+
self.done = true;
119+
return Some(());
120+
}
121+
}
122+
123+
fn read_body_chunk(&mut self) -> Option<()> {
124+
//TODO: this will support chunked body in the future
125+
todo!()
126+
}
127+
128+
fn read_body(&mut self) -> Option<()> {
129+
return self.read_body_len();
130+
}
131+
132+
pub fn append(&mut self, chunk: Vec<u8>) -> Result<(), &'static str> {
133+
if !self.header_done && self.buffer.len() > MAX_HEADER_SIZE {
134+
return Err("Entity Too large");
135+
}
136+
let chunk_size = chunk.len();
137+
self.buffer.extend(chunk);
138+
if self.header_done {
139+
self.read_body();
140+
return Ok(());
141+
} else {
142+
self.header_size += chunk_size;
143+
if self.header_size > MAX_HEADER_SIZE {
144+
return Err("Entity Too large");
145+
}
146+
}
131147
while let Some(pos) = self.buffer.windows(2).position(|w| w == b"\r\n") {
132148
let line = self.buffer.drain(..pos).collect::<Vec<u8>>();
133149
self.buffer.drain(..2);
134150

135-
let line_str = String::from_utf8_lossy(&line);
151+
let line_str = match str::from_utf8(line.as_slice()) {
152+
Ok(v) => v.to_string(),
153+
Err(_e) => return Err("No utf-8"),
154+
};
136155

137156
if self.request.path.is_empty() {
138157
let parts: Vec<&str> = line_str.split_whitespace().collect();
139158
if parts.len() < 2 {
140-
return None;
159+
return Ok(());
141160
}
142161

162+
if parts.len() != 3 {
163+
return Err("Invalid method + path + version request");
164+
}
143165
self.request.method = HttpMethod::from_str(parts[0]);
144166
let path_parts: Vec<&str> = parts[1].split('?').collect();
145167
self.request.path = path_parts[0].to_string();
@@ -158,21 +180,41 @@ impl HttpRequestBuilder {
158180
.collect();
159181
}
160182
} else if !line_str.is_empty() {
161-
if let Some((key, value)) = line_str.split_once(": ") {
162-
if key.to_lowercase() == "content-length" {
183+
if let Some((key, value)) = line_str.split_once(":") {
184+
//Check the number of headers, if the actual headers exceed that number
185+
//drop the connection
186+
if self.request.headers.len() > MAX_HEADER_COUNT {
187+
return Err("Header number exceed allowed");
188+
}
189+
let key = key.trim().to_lowercase();
190+
let value = value.trim();
191+
if key == "content-length" {
192+
if self.request.headers.get("content-length").is_some()
193+
|| self
194+
.request
195+
.headers
196+
.get("transfer-encoding")
197+
.map(|te| te == "chunked")
198+
.unwrap_or(false)
199+
{
200+
return Err("Duplicated content-length");
201+
}
163202
self.body_size = value.parse().unwrap_or(0);
164203
}
165204
self.request
166205
.headers
167206
.insert(key.to_string(), value.to_string());
168207
}
208+
} else {
209+
self.header_done = true;
210+
self.read_body();
211+
return Ok(());
169212
}
170213
}
171-
self.request.body.append(&mut self.buffer.clone());
172-
if self.request.body.len() == self.body_size {
173-
self.done = true;
174-
return Some(self.request.clone());
175-
}
176-
None
214+
Ok(())
177215
}
178216
}
217+
218+
#[cfg(test)]
219+
#[test]
220+
fn basic_request() {}

0 commit comments

Comments
 (0)