From c7639bc3be5b1779d573dc1d2c0d0396b4968554 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Tue, 4 Jan 2022 03:48:12 +0000 Subject: [PATCH] document quoter --- actix-files/src/service.rs | 8 +-- actix-router/src/url.rs | 141 +++++++++++++++++++++++++------------ 2 files changed, 100 insertions(+), 49 deletions(-) diff --git a/actix-files/src/service.rs b/actix-files/src/service.rs index 057dbe5a3..152e1855e 100644 --- a/actix-files/src/service.rs +++ b/actix-files/src/service.rs @@ -114,7 +114,7 @@ impl Service for FilesService { Box::pin(async move { if !is_method_valid { return Ok(req.into_response( - actix_web::HttpResponse::MethodNotAllowed() + HttpResponse::MethodNotAllowed() .insert_header(header::ContentType(mime::TEXT_PLAIN_UTF_8)) .body("Request did not meet this resource's requirements."), )); @@ -123,7 +123,7 @@ impl Service for FilesService { let real_path = match PathBufWrap::parse_path(req.match_info().path(), this.hidden_files) { Ok(item) => item, - Err(e) => return Ok(req.error_response(e)), + Err(err) => return Ok(req.error_response(err)), }; if let Some(filter) = &this.path_filter { @@ -131,9 +131,7 @@ impl Service for FilesService { if let Some(ref default) = this.default { return default.call(req).await; } else { - return Ok( - req.into_response(actix_web::HttpResponse::NotFound().finish()) - ); + return Ok(req.into_response(HttpResponse::NotFound().finish())); } } } diff --git a/actix-router/src/url.rs b/actix-router/src/url.rs index 10193dde8..fee8eaaf3 100644 --- a/actix-router/src/url.rs +++ b/actix-router/src/url.rs @@ -26,16 +26,6 @@ const ALLOWED: &[u8] = b"abcdefghijklmnopqrstuvwxyz const QS: &[u8] = b"+&=;b"; -#[inline] -fn bit_at(array: &[u8], ch: u8) -> bool { - array[(ch >> 3) as usize] & (1 << (ch & 7)) != 0 -} - -#[inline] -fn set_bit(array: &mut [u8], ch: u8) { - array[(ch >> 3) as usize] |= 1 << (ch & 7) -} - thread_local! { static DEFAULT_QUOTER: Quoter = Quoter::new(b"@:", b"%/+"); } @@ -96,7 +86,10 @@ impl ResourcePath for Url { /// A quoter pub struct Quoter { + /// Simple bit-map of safe values in the 0-127 ASCII range. safe_table: [u8; 16], + + /// Simple bit-map of protected values in the 0-127 ASCII range. protected_table: [u8; 16], } @@ -108,28 +101,32 @@ impl Quoter { }; // prepare safe table - for i in 0..128 { - if ALLOWED.contains(&i) { - set_bit(&mut quoter.safe_table, i); + for ch in 0..128 { + if ALLOWED.contains(&ch) { + set_bit(&mut quoter.safe_table, ch); } - if QS.contains(&i) { - set_bit(&mut quoter.safe_table, i); + + if QS.contains(&ch) { + set_bit(&mut quoter.safe_table, ch); } } - for ch in safe { - set_bit(&mut quoter.safe_table, *ch) + for &ch in safe { + set_bit(&mut quoter.safe_table, ch) } // prepare protected table - for ch in protected { - set_bit(&mut quoter.safe_table, *ch); - set_bit(&mut quoter.protected_table, *ch); + for &ch in protected { + set_bit(&mut quoter.safe_table, ch); + set_bit(&mut quoter.protected_table, ch); } quoter } + /// Re-quotes... ? + /// + /// Returns `None` when no modification to the original string was required. pub fn requote(&self, val: &[u8]) -> Option { let mut has_pct = 0; let mut pct = [b'%', 0, 0]; @@ -137,17 +134,19 @@ impl Quoter { let mut cloned: Option> = None; let len = val.len(); + while idx < len { let ch = val[idx]; if has_pct != 0 { pct[has_pct] = val[idx]; has_pct += 1; + if has_pct == 3 { has_pct = 0; let buf = cloned.as_mut().unwrap(); - if let Some(ch) = restore_ch(pct[1], pct[2]) { + if let Some(ch) = hex_pair_to_char(pct[1], pct[2]) { if ch < 128 { if bit_at(&self.protected_table, ch) { buf.extend_from_slice(&pct); @@ -161,6 +160,7 @@ impl Quoter { continue; } } + buf.push(ch); } else { buf.extend_from_slice(&pct[..]); @@ -168,6 +168,7 @@ impl Quoter { } } else if ch == b'%' { has_pct = 1; + if cloned.is_none() { let mut c = Vec::with_capacity(len); c.extend_from_slice(&val[..idx]); @@ -176,6 +177,7 @@ impl Quoter { } else if let Some(ref mut cloned) = cloned { cloned.push(ch) } + idx += 1; } @@ -183,22 +185,52 @@ impl Quoter { } } -#[inline] -fn from_hex(v: u8) -> Option { - if (b'0'..=b'9').contains(&v) { - Some(v - 0x30) // ord('0') == 0x30 - } else if (b'A'..=b'F').contains(&v) { - Some(v - 0x41 + 10) // ord('A') == 0x41 - } else if (b'a'..=b'f').contains(&v) { - Some(v - 0x61 + 10) // ord('a') == 0x61 - } else { - None +/// Converts an ASCII character in the hex-encoded set (`0-9`, `A-F`, `a-f`) to its integer +/// representation from `0x0`–`0xF`. +/// +/// - `0x30 ('0') => 0x0` +/// - `0x39 ('9') => 0x9` +/// - `0x41 ('a') => 0xA` +/// - `0x61 ('A') => 0xA` +/// - `0x46 ('f') => 0xF` +/// - `0x66 ('F') => 0xF` +fn from_ascii_hex(v: u8) -> Option { + match v { + b'0'..=b'9' => Some(v - 0x30), // ord('0') == 0x30 + b'A'..=b'F' => Some(v - 0x41 + 10), // ord('A') == 0x41 + b'a'..=b'f' => Some(v - 0x61 + 10), // ord('a') == 0x61 + _ => None, } } -#[inline] -fn restore_ch(d1: u8, d2: u8) -> Option { - from_hex(d1).and_then(|d1| from_hex(d2).map(move |d2| d1 << 4 | d2)) +/// Decode a ASCII hex-encoded pair to an integer. +/// +/// Returns `None` if either portion of the decoded pair does not evaluate to a valid hex value. +/// +/// - `0x33 ('3'), 0x30 ('0') => 0x30 ('0')` +/// - `0x34 ('4'), 0x31 ('1') => 0x41 ('A')` +/// - `0x36 ('6'), 0x31 ('1') => 0x61 ('a')` +fn hex_pair_to_char(d1: u8, d2: u8) -> Option { + let (d_high, d_low) = (from_ascii_hex(d1)?, from_ascii_hex(d2)?); + + // left shift high nibble by 4 bits + Some(d_high << 4 | d_low) +} + +/// Sets bit in given bit-map to 1=true. +/// +/// # Panics +/// Panics if `ch` index is out of bounds. +fn set_bit(array: &mut [u8], ch: u8) { + array[(ch >> 3) as usize] |= 0b1 << (ch & 0b111) +} + +/// Returns true if bit to true in given bit-map. +/// +/// # Panics +/// Panics if `ch` index is out of bounds. +fn bit_at(array: &[u8], ch: u8) -> bool { + array[(ch >> 3) as usize] & (0b1 << (ch & 0b111)) != 0 } #[cfg(test)] @@ -229,6 +261,16 @@ mod tests { let path = match_url(re, "/user/2345/test"); assert_eq!(path.get("id").unwrap(), "2345"); + } + + #[test] + fn protected_chars() { + let re = "/user/{id}/test"; + + let encoded = percent_encode(PROTECTED); + let path = match_url(re, format!("/user/{}/test", encoded)); + // characters in captured segment remain unencoded + assert_eq!(path.get("id").unwrap(), &encoded); // "%25" should never be decoded into '%' to guarantee the output is a valid // percent-encoded format @@ -239,13 +281,6 @@ mod tests { assert_eq!(path.get("id").unwrap(), "qwe%25rty"); } - #[test] - fn protected_chars() { - let encoded = percent_encode(PROTECTED); - let path = match_url("/user/{id}/test", format!("/user/{}/test", encoded)); - assert_eq!(path.get("id").unwrap(), &encoded); - } - #[test] fn non_protected_ascii() { let non_protected_ascii = ('\u{0}'..='\u{7F}') @@ -281,9 +316,9 @@ mod tests { for i in 0..256 { let c = i as u8; if hex.contains(&c) { - assert!(from_hex(c).is_some()) + assert!(from_ascii_hex(c).is_some()) } else { - assert!(from_hex(c).is_none()) + assert!(from_ascii_hex(c).is_none()) } } @@ -291,7 +326,25 @@ mod tests { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 10, 11, 12, 13, 14, 15, ]; for i in 0..hex.len() { - assert_eq!(from_hex(hex[i]).unwrap(), expected[i]); + assert_eq!(from_ascii_hex(hex[i]).unwrap(), expected[i]); } } + + #[test] + fn custom_quoter() { + let q = Quoter::new(b"", b"+"); + assert_eq!(q.requote(b"/a%25c").unwrap(), "/a%c"); + assert_eq!(q.requote(b"/a%2Bc").unwrap(), "/a%2Bc"); + + let q = Quoter::new(b"%+", b"/"); + assert_eq!(q.requote(b"/a%25b%2Bc").unwrap(), "/a%b+c"); + assert_eq!(q.requote(b"/a%2fb").unwrap(), "/a%2fb"); + assert_eq!(q.requote(b"/a%2Fb").unwrap(), "/a%2Fb"); + } + + #[test] + fn quoter_no_modification() { + let q = Quoter::new(b"", b""); + assert_eq!(q.requote(b"/abc/../efg"), None); + } }