Fork changes #15
Fork changes #15
Conversation
Format clippy
Thread pool resizing
Add fuzzer
| # Why not PR? | ||
| * Author has been inactive since Dec 2020, and I really like this framework and want to see it improved. :) |
There was a problem hiding this comment.
Change this as you see fit or let me know to remove it
| @@ -0,0 +1,98 @@ | |||
| use criterion::{black_box, criterion_group, criterion_main, Criterion}; | |||
There was a problem hiding this comment.
This file benchmarks the parsing of HTTP requests with this library
| @@ -0,0 +1,31 @@ | |||
| [package] | |||
There was a problem hiding this comment.
This is for fuzzing of the requests, meaning we can check invalid requests to see if they cause panics
| let cookie = Cookie::parse(cookie).map_err(|e| Error::Other(e.to_string()))?; | ||
| let name = cookie.name().to_owned(); | ||
| let val = util::percent_decode(cookie.value()).unwrap(); | ||
| req.cookies.push((name, val)); | ||
| if let Ok(cookies) = Cookie::parse(&cookie.into_owned()) { | ||
| for cookie in cookies { | ||
| req.cookies.push(( | ||
| cookie.get_name().to_string(), | ||
| cookie.get_value().to_string(), | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
Old cookie crate was for parsing singular cookies, not the entire header at once
| pub fn user_agent(&self) -> Option<String> { | ||
| self.header("User-Agent").map(|ua| ua.to_string()) | ||
| } | ||
| /// Return the Do Not Track header | ||
| #[must_use] | ||
| pub fn do_not_track(&self) -> bool { | ||
| match self.header("DNT") { | ||
| Some(dnt) => dnt == "1", | ||
| None => false, | ||
| } | ||
| } | ||
| /// Return the Accept-Language, if exists | ||
| #[must_use] | ||
| pub fn accept_language(&self) -> Option<String> { | ||
| self.header("Accept-Language").map(|al| al.to_string()) | ||
| } | ||
| /// Return the content-type, if exists | ||
| #[must_use] | ||
| pub fn content_type(&self) -> Option<String> { | ||
| self.header("Content-Type").map(|ct| ct.to_string()) |
There was a problem hiding this comment.
These are a bit opinionated imo but I found them helpful. Feel free to remove
| /// Starts a new Vial server. Should always be invoked via the | ||
| /// [`vial::run!()`](macro.run.html) macro, since there is some setup | ||
| /// that needs to happen. | ||
| #[doc(hidden)] | ||
| pub fn run<T: ToSocketAddrs>(addr: T, router: Router, banner: Option<&str>) -> Result<()> { | ||
| let pool = ThreadPool::new(MAX_CONNECTIONS); | ||
| // If a range is supplied, the lower bound will be the core pool size while the upper bound will be a maximum pool size the pool is allowed to burst up to when the core threads are busy. | ||
| let pool = threadfin::builder().size(1..=128).build(); |
There was a problem hiding this comment.
Threadfin (https://lib.rs/threadfin) allows for far easier thread management!
| let stream = stream.try_clone()?; | ||
| // discard because: https://doc.rust-lang.org/std/net/struct.TcpStream.html#method.set_read_timeout | ||
| // "An Err is returned if the zero Duration is passed to this method." thus no need to check result | ||
| drop(stream.set_read_timeout(Some(Duration::from_millis(1000)))); |
There was a problem hiding this comment.
Slow loris attack 😞
| let next_pct = match inp.find('%') { | ||
| Some(l) if l < inp.len() - 2 => l, | ||
| Some(l) if l < if inp.len() > 1 { inp.len() - 2 } else { 0 } => l, |
There was a problem hiding this comment.
Bit of a botchy fix for the header issue but it caused thread panics so I quickly fixed it
| @@ -0,0 +1,93 @@ | |||
| #![allow(non_snake_case)] | |||
There was a problem hiding this comment.
Implement some tests for cookies
| assert_eq!(200, res.code()); | ||
| assert_eq!(771638, res.len()); | ||
| } | ||
| // Somehow disabling this godforsaken test resolves the sporadic test failure. |
There was a problem hiding this comment.
It's the library-wide unsafes! Tests are multithreaded... Start one test that spawns the rest sequentially
|
I'd be open to looking at these as individual PRs but I'm not going to review a mega-PR, sorry! |
Compression is borked so I've removed the feature from Cargo.toml but the code is there (dormant).