Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Matcher has problem matching text #438

Open
llacroix opened this issue Jun 7, 2019 · 4 comments
Open

Matcher has problem matching text #438

llacroix opened this issue Jun 7, 2019 · 4 comments

Comments

@llacroix
Copy link

llacroix commented Jun 7, 2019

Here's a route I made

    server.get("/test/:id", middleware!{|req| 
        let id = req.param("id").unwrap();
        format!("{}", id.to_string())
    });

If you try to browse the url /test/fun.js get param will only match fun and not fun.js. Also if you want to browse the url: /test/fun:time.js It will not match anything and fail with NotFound.
@llacroix
Copy link
Author

llacroix commented Jun 7, 2019

I guess it's related to this:

nickel.rs/src/router/into_matcher.rs

Line 18 in 2d0e3d0

static ref REGEX_VAR_SEQ: Regex = Regex::new(r":([,a-zA-Z0-9_-]*)").unwrap();

@llacroix
Copy link
Author

llacroix commented Jun 7, 2019

Ah no, it's the other line.

nickel.rs/src/router/into_matcher.rs

Line 52 in 2d0e3d0

format!("(?P<{}>[,a-zA-Z0-9%_-]*)", c.unwrap().as_str())

My guess is that somewhere it's matching the whole regex and matching even with dangling char. So that would explain why /test/fun.txt matches to ['id'] = "fun". But somewhere your global regex probably doesn't match for any possible char. And when we add :. It fails to match the whole route.

@llacroix
Copy link
Author

llacroix commented Jun 7, 2019 •
edited

nickel.rs/src/router/into_matcher.rs

Line 55 in d7c3093

let line_regex = format!("^{}{}$", named_captures, REGEX_PARAM_SEQ);

Ok found it. Well obviously you could make the regex use unicode aware strings so instead of matching a-zA-Z.... it could be as simple as [\w\[\]]* as a regex or something similar.

@llacroix
Copy link
Author

llacroix commented Jun 7, 2019

You probably want a named capture that look like this: (?P<{}>[\w\s\p{P}\p{Pd}\p{S}--/]*)
It will match any unicode char except "/" including punctuations and such. So it should correctly match anything between "/" and "/".

What I don't understand is why some char will prevent the whole regex from matching and some will match at least the first part of the regex. Because "/test/fun.js" should fail to match "^/test/(?P<id>[,a-zA-Z0-9%_-]*)(\\?[a-zA-Z0-9%_=&-]*)?$"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@llacroix