Skip to content

Commit

Permalink
Ensure all routers percent-decode before matching
Browse files Browse the repository at this point in the history
  • Loading branch information
CathalMullan committed Aug 19, 2024
1 parent 55e8da9 commit afb8b9d
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 72 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ actix-router = "0.5.3"
matchit = "0.8.3"
ntex-router = "0.5.3"
path-tree = "0.8.1"
regex = "1.10.6"
route-recognizer = "0.3.1"
routefinder = "0.5.4"
xitca-router = "0.3.0"
Expand Down
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ In a router of 130 routes, benchmark matching 4 paths.
| xitca-router | 415.02 ns |
| ntex-router | 1.6291 µs |
| route-recognizer | 4.3608 µs |
| regex | 4.5123 µs |
| routefinder | 6.2077 µs |
| actix-router | 20.722 µs |

Expand All @@ -158,7 +157,6 @@ In a router of 320 routes, benchmark matching 80 paths.
| ntex-router | 28.003 µs |
| route-recognizer | 87.400 µs |
| routefinder | 95.115 µs |
| regex | 117.12 µs |
| actix-router | 176.11 µs |

## Inspirations
Expand Down
46 changes: 17 additions & 29 deletions benches/matchit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use codspeed_criterion_compat::{criterion_group, criterion_main, Criterion};
use matchit_routes::paths;
use percent_encoding::percent_decode;

pub mod matchit_routes;

Expand Down Expand Up @@ -39,7 +40,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for route in paths() {
let mut path = actix_router::Path::new(route);
let route = percent_decode(route.as_bytes()).decode_utf8().unwrap();
let mut path = actix_router::Path::new(route.as_ref());
actix.recognize(&mut path).unwrap();
let _ = path
.iter()
Expand All @@ -57,7 +59,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for route in paths() {
let at = matchit.at(route).unwrap();
let route = percent_decode(route.as_bytes()).decode_utf8().unwrap();
let at = matchit.at(route.as_ref()).unwrap();
let _ = at
.params
.iter()
Expand All @@ -76,7 +79,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for route in paths() {
let mut path = ntex_router::Path::new(route);
let route = percent_decode(route.as_bytes()).decode_utf8().unwrap();
let mut path = ntex_router::Path::new(route.as_ref());
ntex.recognize(&mut path).unwrap();
let _ = path
.iter()
Expand All @@ -94,7 +98,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for route in paths() {
let route = path_tree.find(route).unwrap();
let route = percent_decode(route.as_bytes()).decode_utf8().unwrap();
let route = path_tree.find(route.as_ref()).unwrap();
let _ = route
.1
.params_iter()
Expand All @@ -104,27 +109,6 @@ fn benchmark(criterion: &mut Criterion) {
});
});

group.bench_function("matchit benchmarks/regex", |bencher| {
let regex_set = regex::RegexSet::new(routes!(regex)).unwrap();
let regexes: Vec<_> = routes!(regex)
.into_iter()
.map(|pattern| regex::Regex::new(pattern).unwrap())
.collect();

bencher.iter(|| {
for route in paths() {
let matches = regex_set.matches(route).into_iter().collect::<Vec<_>>();
let index = matches.first().unwrap();
let captures = regexes[*index].captures(route).unwrap();
let _ = regexes[*index]
.capture_names()
.flatten()
.filter_map(|name| captures.name(name).map(|m| (name, m.as_str())))
.collect::<Vec<(&str, &str)>>();
}
});
});

group.bench_function("matchit benchmarks/route-recognizer", |bencher| {
let mut route_recognizer = route_recognizer::Router::new();
for route in routes!(colon) {
Expand All @@ -133,7 +117,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for route in paths() {
let recognize = route_recognizer.recognize(route).unwrap();
let route = percent_decode(route.as_bytes()).decode_utf8().unwrap();
let recognize = route_recognizer.recognize(route.as_ref()).unwrap();
let _ = recognize
.params()
.iter()
Expand All @@ -151,7 +136,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for route in paths() {
let best_match = routefinder.best_match(route).unwrap();
let route = percent_decode(route.as_bytes()).decode_utf8().unwrap();
let best_match = routefinder.best_match(route.as_ref()).unwrap();
let _ = best_match
.captures()
.iter()
Expand All @@ -169,12 +155,14 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for route in paths() {
let at = xitca.at(route).unwrap();
let _ = at
let route = percent_decode(route.as_bytes()).decode_utf8().unwrap();
let at = xitca.at(route.as_ref()).unwrap();
let params = at
.params
.iter()
.map(|p| (p.0, p.1))
.collect::<Vec<(&str, &str)>>();
println!("{params:?}");
}
});
});
Expand Down
6 changes: 1 addition & 5 deletions benches/matchit_routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pub fn paths() -> impl IntoIterator<Item = &'static str> {
"/user/repos",
"/repos/rust-lang/rust/stargazers",
"/orgs/rust-lang/public_members/nikomatsakis",
"/repos/rust-lang/rust/releases/1.51.0",
"/repos/rust-lang/rust/releases/1%2E51%2E0",
]
}

Expand All @@ -18,10 +18,6 @@ macro_rules! routes {
routes!(finish => "{p1}", "{p2}", "{p3}", "{p4}")
}};

(regex) => {{
routes!(finish => "(?<p1>.*)", "(?<p2>.*)", "(?<p3>.*)", "(?<p4>.*)")
}};

(finish => $p1:literal, $p2:literal, $p3:literal, $p4:literal) => {{
[
concat!("/authorizations"),
Expand Down
44 changes: 15 additions & 29 deletions benches/path_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use codspeed_criterion_compat::{criterion_group, criterion_main, Criterion};
use path_tree_routes::paths;
use percent_encoding::percent_decode;

pub mod path_tree_routes;

Expand Down Expand Up @@ -40,7 +41,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for (index, path) in paths() {
let mut path = actix_router::Path::new(path);
let path = percent_decode(path.as_bytes()).decode_utf8().unwrap();
let mut path = actix_router::Path::new(path.as_ref());
let n = router.recognize(&mut path).unwrap();
assert_eq!(*n.0, index);
let _ = path
Expand All @@ -59,7 +61,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for (index, path) in paths() {
let n = matcher.at(path).unwrap();
let path = percent_decode(path.as_bytes()).decode_utf8().unwrap();
let n = matcher.at(path.as_ref()).unwrap();
assert_eq!(*n.value, index);
let _ = n
.params
Expand All @@ -79,7 +82,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for (index, path) in paths() {
let mut path = ntex_router::Path::new(path);
let path = percent_decode(path.as_bytes()).decode_utf8().unwrap();
let mut path = ntex_router::Path::new(path.as_ref());
let n = router.recognize(&mut path).unwrap();
assert_eq!(*n.0, index);
let _ = path
Expand All @@ -98,7 +102,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for (index, path) in paths() {
let n = tree.find(path).unwrap();
let path = percent_decode(path.as_bytes()).decode_utf8().unwrap();
let n = tree.find(path.as_ref()).unwrap();
assert_eq!(*n.0, index);
let _ =
n.1.params_iter()
Expand All @@ -108,28 +113,6 @@ fn benchmark(criterion: &mut Criterion) {
});
});

group.bench_function("path-tree benchmarks/regex", |bencher| {
let regex_set = regex::RegexSet::new(routes!(regex)).unwrap();
let regexes: Vec<_> = routes!(regex)
.into_iter()
.map(|pattern| regex::Regex::new(pattern).unwrap())
.collect();

bencher.iter(|| {
for (index, path) in paths() {
let matches = regex_set.matches(path).into_iter().collect::<Vec<_>>();
assert!(matches.contains(&index));
let i = matches.first().unwrap();
let captures = regexes[*i].captures(path).unwrap();
let _ = regexes[*i]
.capture_names()
.flatten()
.filter_map(|name| captures.name(name).map(|m| (name, m.as_str())))
.collect::<Vec<(&str, &str)>>();
}
});
});

group.bench_function("path-tree benchmarks/route-recognizer", |bencher| {
let mut router = route_recognizer::Router::<usize>::new();
for (index, route) in routes!(colon).iter().enumerate() {
Expand All @@ -138,7 +121,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for (index, path) in paths() {
let n = router.recognize(path).unwrap();
let path = percent_decode(path.as_bytes()).decode_utf8().unwrap();
let n = router.recognize(path.as_ref()).unwrap();
assert_eq!(**n.handler(), index);
let _ = n
.params()
Expand All @@ -157,7 +141,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for (index, path) in paths() {
let n = router.best_match(path).unwrap();
let path = percent_decode(path.as_bytes()).decode_utf8().unwrap();
let n = router.best_match(path.as_ref()).unwrap();
assert_eq!(*n, index);
let _ = n
.captures()
Expand All @@ -176,7 +161,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for (index, path) in paths() {
let n = xitca.at(path).unwrap();
let path = percent_decode(path.as_bytes()).decode_utf8().unwrap();
let n = xitca.at(path.as_ref()).unwrap();
assert_eq!(*n.value, index);
let _ = n
.params
Expand Down
6 changes: 1 addition & 5 deletions benches/path_tree_routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn paths() -> impl IntoIterator<Item = (usize, &'static str)> {
"/issues",
"/legacy/issues/search/rust-lang/rust/987/1597",
"/legacy/repos/search/1597",
"/legacy/user/email/rust@rust-lang.org",
"/legacy/user/email/rust%40rust-lang.org",
"/legacy/user/search/1597",
"/licenses",
"/licenses/mit",
Expand Down Expand Up @@ -96,10 +96,6 @@ macro_rules! routes {
routes!(finish => "{p1}", "{p2}", "{p3}", "{p4}")
}};

(regex) => {{
routes!(finish => "(?<p1>.*)", "(?<p2>.*)", "(?<p3>.*)", "(?<p4>.*)")
}};

(finish => $p1:literal, $p2:literal, $p3:literal, $p4:literal) => {{
[
concat!("/app"),
Expand Down

0 comments on commit afb8b9d

Please sign in to comment.