Skip to content

Commit

Permalink
Add a feature to ignore empty path segments (// in path).
Browse files Browse the repository at this point in the history
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#738.
  • Loading branch information
brotskydotcom committed Sep 2, 2022
1 parent 3cf1783 commit 3ab39ff
Show file tree
Hide file tree
Showing 4 changed files with 318 additions and 8 deletions.
7 changes: 7 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -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"]
Expand Down
1 change: 1 addition & 0 deletions src/filters/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ impl Peek {

/// Get an iterator over the segments of the peeked path.
pub fn segments(&self) -> impl Iterator<Item = &str> {
// NOTE: this ignores empty segments regardless of feature settings
self.as_str().split('/').filter(|seg| !seg.is_empty())
}
}
Expand Down
25 changes: 17 additions & 8 deletions src/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,17 @@ enum BodyState {

impl Route {
pub(crate) fn new(req: Request, remote_addr: Option<SocketAddr>) -> RefCell<Route> {
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,
Expand Down Expand Up @@ -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;
}
}
}

Expand Down
293 changes: 293 additions & 0 deletions tests/path_empty_segments.rs
Original file line number Diff line number Diff line change
@@ -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::<u32>();

// 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::<u32>();

// 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::<Vec<_>>(), &["42", "vroom"]);

// matches index
let ex = warp::test::request().path("/").filter(&peek).await.unwrap();

let segs = ex.segments().collect::<Vec<_>>();
assert_eq!(segs, Vec::<&str>::new());
}

0 comments on commit 3ab39ff

Please sign in to comment.