From 3ab39ff42c5fba59908ba15f99981bec056fa155 Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Fri, 2 Sep 2022 00:13:25 -0700 Subject: [PATCH] Add a feature to ignore empty path segments (// in path). When the new `ignore-empty-path-segments` feature is enabled, multiple leading slashes at the start of the path are ignored (the first segment starts after them), and multiple slashes at the end of any matched segment are ignored (so the next segment starts after them). In addition, the `end` filter will ignore multiple slashes that follow the final segment. Both the `fullpath` and `tail` filters will still return any multiple slashes in their matched text, but their `segments` iterator will ignore the empty segments (in fact it does this today even without this feature being set). This change does _not_ include this feature in the list of default features. This change includes path filter testing on paths with multiple slashes in them. This change fixes seanmonstar/warp#738. --- Cargo.toml | 7 + src/filters/path.rs | 1 + src/route.rs | 25 ++- tests/path_empty_segments.rs | 293 +++++++++++++++++++++++++++++++++++ 4 files changed, 318 insertions(+), 8 deletions(-) create mode 100644 tests/path_empty_segments.rs diff --git a/Cargo.toml b/Cargo.toml index 53a0c8879..3d7db295f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,6 +58,9 @@ default = ["multipart", "websocket"] websocket = ["tokio-tungstenite"] tls = ["tokio-rustls"] +# allow double slashes in paths +ignore-empty-path-segments = [] + # Enable compression-related filters compression = ["compression-brotli", "compression-gzip"] compression-brotli = ["async-compression/brotli"] @@ -79,6 +82,10 @@ required-features = ["multipart"] name = "ws" required-features = ["websocket"] +[[test]] +name = "path_empty_segments" +required-features = ["ignore-empty-path-segments"] + [[example]] name = "compression" required-features = ["compression"] diff --git a/src/filters/path.rs b/src/filters/path.rs index 179a8d1c9..0ba6ed9bd 100644 --- a/src/filters/path.rs +++ b/src/filters/path.rs @@ -371,6 +371,7 @@ impl Peek { /// Get an iterator over the segments of the peeked path. pub fn segments(&self) -> impl Iterator { + // NOTE: this ignores empty segments regardless of feature settings self.as_str().split('/').filter(|seg| !seg.is_empty()) } } diff --git a/src/route.rs b/src/route.rs index afbac4d8b..a9d3f0a0a 100644 --- a/src/route.rs +++ b/src/route.rs @@ -43,12 +43,17 @@ enum BodyState { impl Route { pub(crate) fn new(req: Request, remote_addr: Option) -> RefCell { - let segments_index = if req.uri().path().starts_with('/') { - // Skip the beginning slash. - 1 - } else { - 0 - }; + let mut segments_index: usize = 0; + if req.uri().path().starts_with('/') { + segments_index += 1; + #[cfg(feature = "ignore-empty-path-segments")] + { + let path = req.uri().path().as_bytes(); + while segments_index < path.len() && path[segments_index] == b'/' { + segments_index += 1; + } + } + } RefCell::new(Route { body: BodyState::Ready, @@ -93,15 +98,19 @@ impl Route { pub(crate) fn set_unmatched_path(&mut self, index: usize) { let index = self.segments_index + index; - let path = self.req.uri().path(); + let path = self.req.uri().path().as_bytes(); if path.is_empty() { // malformed path return; } else if path.len() == index { self.segments_index = index; } else { - debug_assert_eq!(path.as_bytes()[index], b'/'); + debug_assert_eq!(path[index], b'/'); self.segments_index = index + 1; + #[cfg(feature = "ignore-empty-path-segments")] + while self.segments_index < path.len() && path[self.segments_index] == b'/' { + self.segments_index += 1; + } } } diff --git a/tests/path_empty_segments.rs b/tests/path_empty_segments.rs new file mode 100644 index 000000000..ac69ad21b --- /dev/null +++ b/tests/path_empty_segments.rs @@ -0,0 +1,293 @@ +#![deny(warnings)] +extern crate warp; + +use warp::Filter; + +#[tokio::test] +async fn path() { + let _ = pretty_env_logger::try_init(); + + let foo = warp::path("foo"); + let bar = warp::path(String::from("bar")); + let foo_bar = foo.and(bar.clone()); + let foo_bar_end = foo.and(bar.clone()).and(warp::path::end()); + + // /foo + let fffoo_req = || warp::test::request().path("///foo"); + + assert!(fffoo_req().matches(&foo).await); + assert!(!fffoo_req().matches(&bar).await); + assert!(!fffoo_req().matches(&foo_bar).await); + + // /foo/bar + let ffoo_bar_req = || warp::test::request().path("//foo/bar"); + let foo_bbar_req= || warp::test::request().path("/foo//bar"); + let foo_bar_reqq= || warp::test::request().path("/foo/bar//"); + + assert!(ffoo_bar_req().matches(&foo).await); + assert!(!ffoo_bar_req().matches(&bar).await); + assert!(ffoo_bar_req().matches(&foo_bar).await); + assert!(foo_bbar_req().matches(&foo).await); + assert!(foo_bbar_req().matches(&foo_bar).await); + assert!(foo_bar_reqq().matches(&foo).await); + assert!(foo_bar_reqq().matches(&foo_bar).await); + assert!(foo_bar_reqq().matches(&foo_bar_end).await); +} + +#[tokio::test] +async fn end() { + let _ = pretty_env_logger::try_init(); + + let foo = warp::path("foo"); + let end = warp::path::end(); + let foo_end = foo.and(end); + + assert!( + warp::test::request().path("///").matches(&end).await, + "end() matches ///" + ); + + assert!( + warp::test::request() + .path("http://localhost:1234") + .matches(&end) + .await, + "end() matches /" + ); + + assert!( + warp::test::request() + .path("http://localhost:1234?q=2") + .matches(&end) + .await, + "end() matches empty path" + ); + + assert!( + warp::test::request() + .path("localhost:1234") + .matches(&end) + .await, + "end() matches authority-form" + ); + + assert!( + !warp::test::request().path("///foo").matches(&end).await, + "end() doesn't match ///foo" + ); + + assert!( + warp::test::request().path("///foo").matches(&foo_end).await, + "path().and(end()) matches ///foo" + ); + + assert!( + warp::test::request().path("///foo/").matches(&foo_end).await, + "path().and(end()) matches ///foo/" + ); + + assert!( + warp::test::request().path("///foo///").matches(&foo_end).await, + "path().and(end()) matches ///foo///" + ) +} + +#[tokio::test] +async fn tail() { + let tail = warp::path::tail(); + + // matches full path + let ex = warp::test::request() + .path("///42//vroom") + .filter(&tail) + .await + .unwrap(); + assert_eq!(ex.as_str(), "42//vroom"); + + // matches index + let ex = warp::test::request().path("///").filter(&tail).await.unwrap(); + assert_eq!(ex.as_str(), ""); + + // doesn't include query + let ex = warp::test::request() + .path("//foo/bar//?baz=quux") + .filter(&tail) + .await + .unwrap(); + assert_eq!(ex.as_str(), "foo/bar//"); + + // doesn't include previously matched prefix + let and = warp::path("foo").and(tail); + let ex = warp::test::request() + .path("///foo///bar//") + .filter(&and) + .await + .unwrap(); + assert_eq!(ex.as_str(), "bar//"); + + // sets unmatched path index to end + let m = tail.and(warp::path("foo")); + assert!(!warp::test::request().path("//foo//bar//").matches(&m).await); + + let m = tail.and(warp::path::end()); + assert!(warp::test::request().path("//foo/bar//").matches(&m).await); + + let ex = warp::test::request() + .path("localhost") + .filter(&tail) + .await + .unwrap(); + assert_eq!(ex.as_str(), "/"); +} + +#[tokio::test] +async fn full_path() { + let full_path = warp::path::full(); + + let foo = warp::path("foo"); + let bar = warp::path("bar"); + let param = warp::path::param::(); + + // matches full request path + let ex = warp::test::request() + .path("///42//vroom///") + .filter(&full_path) + .await + .unwrap(); + assert_eq!(ex.as_str(), "///42//vroom///"); + + // matches index + let ex = warp::test::request() + .path("//") + .filter(&full_path) + .await + .unwrap(); + assert_eq!(ex.as_str(), "//"); + + // does not include query + let ex = warp::test::request() + .path("////foo///bar//?baz=quux") + .filter(&full_path) + .await + .unwrap(); + assert_eq!(ex.as_str(), "////foo///bar//"); + + // includes previously matched prefix + let and = foo.and(full_path); + let ex = warp::test::request() + .path("///foo///bar//") + .filter(&and) + .await + .unwrap(); + assert_eq!(ex.as_str(), "///foo///bar//"); + + // includes following matches + let and = full_path.and(foo); + let ex = warp::test::request() + .path("//foo///bar//") + .filter(&and) + .await + .unwrap(); + assert_eq!(ex.as_str(), "//foo///bar//"); + + // includes previously matched param + let and = foo.and(param).and(full_path); + let (_, ex) = warp::test::request() + .path("///foo///123") + .filter(&and) + .await + .unwrap(); + assert_eq!(ex.as_str(), "///foo///123"); + + // does not modify matching + let m = full_path.and(foo).and(bar); + assert!(warp::test::request().path("///foo///bar//").matches(&m).await); + + // doesn't panic on authority-form + let ex = warp::test::request() + .path("localhost:1234") + .filter(&full_path) + .await + .unwrap(); + assert_eq!(ex.as_str(), "/"); +} + +#[tokio::test] +async fn peek() { + let peek = warp::path::peek(); + + let foo = warp::path("foo"); + let bar = warp::path("bar"); + let param = warp::path::param::(); + + // matches full request path + let ex = warp::test::request() + .path("///42///vroom//") + .filter(&peek) + .await + .unwrap(); + assert_eq!(ex.as_str(), "42///vroom//"); + + // matches index + let ex = warp::test::request().path("///").filter(&peek).await.unwrap(); + assert_eq!(ex.as_str(), ""); + + // does not include query + let ex = warp::test::request() + .path("///foo///bar//?baz=quux") + .filter(&peek) + .await + .unwrap(); + assert_eq!(ex.as_str(), "foo///bar//"); + + // does not include previously matched prefix + let and = foo.and(peek); + let ex = warp::test::request() + .path("///foo///bar//") + .filter(&and) + .await + .unwrap(); + assert_eq!(ex.as_str(), "bar//"); + + // includes following matches + let and = peek.and(foo); + let ex = warp::test::request() + .path("///foo///bar//") + .filter(&and) + .await + .unwrap(); + assert_eq!(ex.as_str(), "foo///bar//"); + + // does not include previously matched param + let and = foo.and(param).and(peek); + let (_, ex) = warp::test::request() + .path("///foo///123//") + .filter(&and) + .await + .unwrap(); + assert_eq!(ex.as_str(), ""); + + // does not modify matching + let and = peek.and(foo).and(bar); + assert!(warp::test::request().path("///foo///bar//").matches(&and).await); +} + +#[tokio::test] +async fn peek_segments() { + let peek = warp::path::peek(); + + // matches full request path + let ex = warp::test::request() + .path("///42///vroom//") + .filter(&peek) + .await + .unwrap(); + + assert_eq!(ex.segments().collect::>(), &["42", "vroom"]); + + // matches index + let ex = warp::test::request().path("/").filter(&peek).await.unwrap(); + + let segs = ex.segments().collect::>(); + assert_eq!(segs, Vec::<&str>::new()); +}