From fd412a8223168f86d0cd3aaf1ca81a9f67c8a15d Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Tue, 1 Feb 2022 00:26:34 +0300 Subject: [PATCH] `Quoter::requote` returns `Vec` (#2613) --- actix-router/CHANGES.md | 3 +++ actix-router/src/de.rs | 6 +++--- actix-router/src/quoter.rs | 43 ++++++++++++++++++++++++++++++-------- actix-router/src/url.rs | 8 +++---- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/actix-router/CHANGES.md b/actix-router/CHANGES.md index 6253b522a..1f22e5764 100644 --- a/actix-router/CHANGES.md +++ b/actix-router/CHANGES.md @@ -1,6 +1,9 @@ # Changes ## Unreleased - 2021-xx-xx +- `Quoter::requote` now returns `Option>`. [#2613] + +[#2613]: https://github.com/actix/actix-web/pull/2613 ## 0.5.0-rc.2 - 2022-01-21 diff --git a/actix-router/src/de.rs b/actix-router/src/de.rs index 27aa49ef2..efafd08db 100644 --- a/actix-router/src/de.rs +++ b/actix-router/src/de.rs @@ -52,7 +52,7 @@ macro_rules! parse_value { V: Visitor<'de>, { let decoded = FULL_QUOTER - .with(|q| q.requote(self.value.as_bytes())) + .with(|q| q.requote_str_lossy(self.value)) .map(Cow::Owned) .unwrap_or(Cow::Borrowed(self.value)); @@ -332,7 +332,7 @@ impl<'de> Deserializer<'de> for Value<'de> { where V: Visitor<'de>, { - match FULL_QUOTER.with(|q| q.requote(self.value.as_bytes())) { + match FULL_QUOTER.with(|q| q.requote_str_lossy(self.value)) { Some(s) => visitor.visit_string(s), None => visitor.visit_borrowed_str(self.value), } @@ -342,7 +342,7 @@ impl<'de> Deserializer<'de> for Value<'de> { where V: Visitor<'de>, { - match FULL_QUOTER.with(|q| q.requote(self.value.as_bytes())) { + match FULL_QUOTER.with(|q| q.requote_str_lossy(self.value)) { Some(s) => visitor.visit_byte_buf(s.into()), None => visitor.visit_borrowed_bytes(self.value.as_bytes()), } diff --git a/actix-router/src/quoter.rs b/actix-router/src/quoter.rs index 26ecc92cd..73b1e72dd 100644 --- a/actix-router/src/quoter.rs +++ b/actix-router/src/quoter.rs @@ -66,8 +66,13 @@ impl Quoter { /// Re-quotes... ? /// - /// Returns `None` when no modification to the original string was required. - pub fn requote(&self, val: &[u8]) -> Option { + /// Returns `None` when no modification to the original byte string was required. + /// + /// Non-ASCII bytes are accepted as valid input. + /// + /// Behavior for invalid/incomplete percent-encoding sequences is unspecified and may include removing + /// the invalid sequence from the output or passing it as it is. + pub fn requote(&self, val: &[u8]) -> Option> { let mut has_pct = 0; let mut pct = [b'%', 0, 0]; let mut idx = 0; @@ -121,7 +126,12 @@ impl Quoter { idx += 1; } - cloned.map(|data| String::from_utf8_lossy(&data).into_owned()) + cloned + } + + pub(crate) fn requote_str_lossy(&self, val: &str) -> Option { + self.requote(val.as_bytes()) + .map(|data| String::from_utf8_lossy(&data).into_owned()) } } @@ -201,14 +211,29 @@ mod tests { #[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"); + assert_eq!(q.requote(b"/a%25c").unwrap(), b"/a%c"); + assert_eq!(q.requote(b"/a%2Bc").unwrap(), b"/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"); - assert_eq!(q.requote(b"/a%0Ab").unwrap(), "/a\nb"); + assert_eq!(q.requote(b"/a%25b%2Bc").unwrap(), b"/a%b+c"); + assert_eq!(q.requote(b"/a%2fb").unwrap(), b"/a%2fb"); + assert_eq!(q.requote(b"/a%2Fb").unwrap(), b"/a%2Fb"); + assert_eq!(q.requote(b"/a%0Ab").unwrap(), b"/a\nb"); + assert_eq!(q.requote(b"/a%FE\xffb").unwrap(), b"/a\xfe\xffb"); + assert_eq!(q.requote(b"/a\xfe\xffb"), None); + } + + #[test] + fn non_ascii() { + let q = Quoter::new(b"%+", b"/"); + assert_eq!(q.requote(b"/a%FE\xffb").unwrap(), b"/a\xfe\xffb"); + assert_eq!(q.requote(b"/a\xfe\xffb"), None); + } + + #[test] + fn invalid_sequences() { + let q = Quoter::new(b"%+", b"/"); + assert_eq!(q.requote(b"/a%2x%2X%%").unwrap(), b"/a%2x%2X"); } #[test] diff --git a/actix-router/src/url.rs b/actix-router/src/url.rs index f8d94ae4a..e7dda3fca 100644 --- a/actix-router/src/url.rs +++ b/actix-router/src/url.rs @@ -15,14 +15,14 @@ pub struct Url { impl Url { #[inline] pub fn new(uri: http::Uri) -> Url { - let path = DEFAULT_QUOTER.with(|q| q.requote(uri.path().as_bytes())); + let path = DEFAULT_QUOTER.with(|q| q.requote_str_lossy(uri.path())); Url { uri, path } } #[inline] pub fn new_with_quoter(uri: http::Uri, quoter: &Quoter) -> Url { Url { - path: quoter.requote(uri.path().as_bytes()), + path: quoter.requote_str_lossy(uri.path()), uri, } } @@ -45,13 +45,13 @@ impl Url { #[inline] pub fn update(&mut self, uri: &http::Uri) { self.uri = uri.clone(); - self.path = DEFAULT_QUOTER.with(|q| q.requote(uri.path().as_bytes())); + self.path = DEFAULT_QUOTER.with(|q| q.requote_str_lossy(uri.path())); } #[inline] pub fn update_with_quoter(&mut self, uri: &http::Uri, quoter: &Quoter) { self.uri = uri.clone(); - self.path = quoter.requote(uri.path().as_bytes()); + self.path = quoter.requote_str_lossy(uri.path()); } }