Skip to content

Commit

Permalink
Always perform percent-decoding
Browse files Browse the repository at this point in the history
  • Loading branch information
CathalMullan committed Aug 19, 2024
1 parent 8068ad9 commit dc3b3a0
Show file tree
Hide file tree
Showing 18 changed files with 57 additions and 311 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ workspace = true

[dependencies]
smallvec = "1.13"
percent-encoding = "2.3"

[dev-dependencies]
# Snapshots
Expand Down
4 changes: 2 additions & 2 deletions benches/matchit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for route in paths() {
let mut path = wayfind::path::Path::new(route);
let search = wayfind.search(&mut path).unwrap().unwrap();
let path = wayfind::path::Path::new(route).unwrap();
let search = wayfind.search(&path).unwrap();
let _ = search
.parameters
.iter()
Expand Down
4 changes: 2 additions & 2 deletions benches/path_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ fn benchmark(criterion: &mut Criterion) {

bencher.iter(|| {
for (index, path) in paths() {
let mut path = wayfind::path::Path::new(path);
let search = wayfind.search(&mut path).unwrap().unwrap();
let path = wayfind::path::Path::new(path).unwrap();
let search = wayfind.search(&path).unwrap();
assert_eq!(search.data.value, index);
let _ = search
.parameters
Expand Down
12 changes: 6 additions & 6 deletions examples/axum-fork/src/routing/path_router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,8 @@ where
}

let path = req.uri().path().to_owned();
let mut wayfind_path = Path::new(&path);
let result = match self.node.matches(&mut wayfind_path) {
let wayfind_path = Path::new(&path).expect("Invalid path!");
let result = match self.node.matches(&wayfind_path) {
Some(match_) => {
let id = match_.data.value;

Expand Down Expand Up @@ -366,8 +366,8 @@ where
}

pub(super) fn replace_endpoint(&mut self, path: &str, endpoint: Endpoint<S>) {
let mut wayfind_path = Path::new(path);
if let Some(match_) = self.node.matches(&mut wayfind_path) {
let wayfind_path = Path::new(path).expect("Invalid path!");
if let Some(match_) = self.node.matches(&wayfind_path) {
let id = match_.data.value;
self.routes.insert(id, endpoint);
return;
Expand Down Expand Up @@ -437,8 +437,8 @@ impl Node {
Ok(())
}

fn matches<'n, 'p>(&'n self, path: &'p mut Path) -> Option<Match<'n, 'p, RouteId>> {
self.inner.search(path).expect("Failed to match!")
fn matches<'n, 'p>(&'n self, path: &'p Path) -> Option<Match<'n, 'p, RouteId>> {
self.inner.search(path)
}
}

Expand Down
7 changes: 2 additions & 5 deletions examples/hyper/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,8 @@ async fn main() -> Result<(), anyhow::Error> {
let router = Arc::clone(&router);
async move {
let path = request.uri().path();
let mut wayfind_path = Path::new(path);
let matches = router
.search(&mut wayfind_path)
.expect("Failed to match!")
.expect("Failed to match!");
let wayfind_path = Path::new(path).expect("Invalid path!");
let matches = router.search(&wayfind_path).expect("Failed to match!");

let handler = &matches.data.value;
let parameters = &matches.parameters;
Expand Down
39 changes: 0 additions & 39 deletions src/decode.rs

This file was deleted.

1 change: 0 additions & 1 deletion src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,3 @@ pub mod decode;
pub mod delete;
pub mod insert;
pub mod route;
pub mod search;
10 changes: 9 additions & 1 deletion src/errors/decode.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::{error::Error, fmt::Display};
use std::{error::Error, fmt::Display, str::Utf8Error};

#[derive(Debug, PartialEq, Eq)]
pub enum DecodeError {
Utf8Error(Utf8Error),
InvalidEncoding,
}

Expand All @@ -10,7 +11,14 @@ impl Error for DecodeError {}
impl Display for DecodeError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Utf8Error(error) => error.fmt(f),
Self::InvalidEncoding => write!(f, "Invalid Encoding"),
}
}
}

impl From<Utf8Error> for DecodeError {
fn from(error: Utf8Error) -> Self {
Self::Utf8Error(error)
}
}
10 changes: 9 additions & 1 deletion src/errors/insert.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use super::route::RouteError;
use super::{decode::DecodeError, route::RouteError};
use std::{error::Error, fmt::Display};

#[derive(Debug, PartialEq, Eq)]
pub enum InsertError {
RouteError(RouteError),
DecodeError(DecodeError),
EncodedPath,
DuplicatePath,
UnknownConstraint,
Expand All @@ -15,6 +16,7 @@ impl Display for InsertError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::RouteError(error) => error.fmt(f),
Self::DecodeError(error) => error.fmt(f),
Self::EncodedPath => write!(f, "Encoded Path"),
Self::DuplicatePath => write!(f, "Duplicate Path"),
Self::UnknownConstraint => write!(f, "Unknown Constraint"),
Expand All @@ -27,3 +29,9 @@ impl From<RouteError> for InsertError {
Self::RouteError(error)
}
}

impl From<DecodeError> for InsertError {
fn from(error: DecodeError) -> Self {
Self::DecodeError(error)
}
}
23 changes: 0 additions & 23 deletions src/errors/search.rs

This file was deleted.

1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
pub mod constraints;
pub mod decode;
pub mod errors;
pub mod node;
pub mod parts;
Expand Down
28 changes: 9 additions & 19 deletions src/path.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,19 @@
use crate::{decode::percent_decode, errors::decode::DecodeError};
use crate::errors::decode::DecodeError;
use std::borrow::Cow;

pub struct Path<'path> {
original: &'path [u8],
decoded: Option<Vec<u8>>,
decoded: Cow<'path, str>,
}

impl<'path> Path<'path> {
#[must_use]
pub const fn new(path: &'path str) -> Self {
Self {
original: path.as_bytes(),
decoded: None,
}
}

pub fn percent_decode(&mut self) -> Result<(), DecodeError> {
if self.decoded.is_none() {
self.decoded = Some(percent_decode(self.original)?);
}

Ok(())
pub fn new(path: &'path str) -> Result<Self, DecodeError> {
Ok(Self {
decoded: percent_encoding::percent_decode_str(path).decode_utf8()?,
})
}

#[must_use]
pub fn as_bytes(&self) -> &[u8] {
self.decoded.as_deref().unwrap_or(self.original)
pub fn decoded_bytes(&'path self) -> &'path [u8] {
self.decoded.as_bytes()
}
}
52 changes: 10 additions & 42 deletions src/router.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use crate::{
constraints::Constraint,
errors::{
constraint::ConstraintError, delete::DeleteError, insert::InsertError, search::SearchError,
},
errors::{constraint::ConstraintError, delete::DeleteError, insert::InsertError},
node::{search::Match, Node, NodeData, NodeKind},
parts::{Part, Parts},
path::Path,
Expand All @@ -19,7 +17,6 @@ use std::{
pub struct Router<T> {
root: Node<T>,
constraints: HashMap<Vec<u8>, fn(&str) -> bool>,
percent_encoding: bool,
}

impl<T> Router<T> {
Expand All @@ -41,7 +38,6 @@ impl<T> Router<T> {
quick_dynamic: false,
},
constraints: HashMap::new(),
percent_encoding: false,
};

router.constraint::<u8>().unwrap();
Expand Down Expand Up @@ -79,28 +75,8 @@ impl<T> Router<T> {
}
}

pub fn percent_encoding(&mut self, enabled: bool) {
self.percent_encoding = enabled;
}

// FIXME: This will likely match when it shouldn't.
// I guess the correct approach would be to percent-decode the input, and see if it's equal or not.
fn contains_percent_encoding(input: &str) -> bool {
let chars: Vec<char> = input.chars().collect();
for window in chars.windows(3) {
if let [a, b, c] = window {
if *a == '%' && b.is_ascii_hexdigit() && c.is_ascii_hexdigit() {
return true;
}
}
}

false
}

pub fn insert(&mut self, route: &str, value: T) -> Result<(), InsertError> {
// Ensure this isn't already encoded.
if Self::contains_percent_encoding(route) {
if route.as_bytes() != Path::new(route)?.decoded_bytes() {
return Err(InsertError::EncodedPath);
}

Expand Down Expand Up @@ -133,25 +109,17 @@ impl<T> Router<T> {

pub fn search<'router, 'path>(
&'router self,
path: &'path mut Path,
) -> Result<Option<Match<'router, 'path, T>>, SearchError> {
if self.percent_encoding {
path.percent_decode()?;
}

path: &'path Path,
) -> Option<Match<'router, 'path, T>> {
let mut parameters = smallvec![];
let Some(node) = self
let node = self
.root
.search(path.as_bytes(), &mut parameters, &self.constraints)
else {
return Ok(None);
};

let Some(data) = node.data.as_ref() else {
return Ok(None);
};
.search(path.decoded_bytes(), &mut parameters, &self.constraints)?;

Ok(Some(Match { data, parameters }))
Some(Match {
data: node.data.as_ref()?,
parameters,
})
}
}

Expand Down
5 changes: 2 additions & 3 deletions tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ pub fn assert_router_match<'a, T: PartialEq + Debug>(
input: &'a str,
expected: Option<ExpectedMatch<'_, 'a, T>>,
) {
let mut path = Path::new(input);
let Some(Match { data, parameters }) = router.search(&mut path).expect("Failed to match!")
else {
let path = Path::new(input).expect("Invalid path!");
let Some(Match { data, parameters }) = router.search(&path) else {
assert!(expected.is_none(), "No match found for input: {input}");
return;
};
Expand Down
Loading

0 comments on commit dc3b3a0

Please sign in to comment.