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

* should match empty paths #45

Open
yoshuawuyts opened this issue Feb 12, 2021 · 8 comments
Open

* should match empty paths #45

yoshuawuyts opened this issue Feb 12, 2021 · 8 comments
Labels

Comments

@yoshuawuyts
Copy link
Member

Ref: http-rs/tide#783

curl http://localhost:8080/test
ok
curl http://localhost:8080/
404
curl http://localhost:8080
404
@jbr
Copy link
Member

jbr commented Feb 12, 2021

happy to take this on, although i'm not sure it's a bug in the semver sense (i think this is 0.x semver-minor in that it's a change in expected behavior as well as actual behavior, and would be semver-major if this were > 0)

@jbr jbr self-assigned this Feb 12, 2021
@jbr jbr removed their assignment Aug 10, 2021
@willhodges
Copy link

This is still an issue. The wildcard is commonly understood to mean zero or more repetitions of an item, in this case path segments.

@willhodges
Copy link

@yoshuawuyts Do you mind if I take a crack at this sometime in the next few weeks?

@jbr
Copy link
Member

jbr commented May 5, 2022

Is this an issue on head? The router has changed between the last release and head

@willhodges
Copy link

willhodges commented May 9, 2022

@jbr Yes, still an issue on head. The NFA is not configured to allow an empty string for the colon wildcard as a valid acceptance state. I added some quick tests in lib.rs to verify this as well:

    #[test]
    fn empty_wildcard_colon() {
        let mut router = Router::new();

        router.add("/a/:b", "ab".to_string());

        let m = router.recognize("/a").unwrap();
        assert_eq!(*m.handler, "ab".to_string());
    }

    #[test]
    fn empty_wildcard_colon2() {
        let mut router = Router::new();

        router.add("/a/:b", "ab".to_string());

        let m = router.recognize("/a/").unwrap();
        assert_eq!(*m.handler, "ab".to_string());
    }
failures:

---- tests::empty_wildcard_colon stdout ----
thread 'tests::empty_wildcard_colon' panicked at 'called `Result::unwrap()` on an `Err` value: "The string was exhausted before reaching an acceptance state"', src/lib.rs:503:40

---- tests::empty_wildcard_colon2 stdout ----
thread 'tests::empty_wildcard_colon2' panicked at 'called `Result::unwrap()` on an `Err` value: "The string was exhausted before reaching an acceptance state"', src/lib.rs:513:41

@jbr
Copy link
Member

jbr commented May 9, 2022

Ahh my mistake, I hadn't noticed that we were in the route recognizer crate, not tide. However, this original issue was about star routes, not colon routes. Wanting /a/:b to match /a would be a different issue from this one.

However, tide moved to a different routing crate (routefinder) because route-recognizer is not particularly easy to change. It might be possible to add this behavior to routefinder. If that's something you'd want, feel free to open an issue over there, but it might be a week or two before I get to it, and it might need to be an opt-in feature (probably new syntax for the parser, like /a/:b? or similar).

@willhodges
Copy link

No worries.

I'm currently working on a project using Yew, which uses route-recognizer for routing. It might be possible to convince them to switch to routefinder since the route specification syntax is similar (identical?). I would love the ability to specify a /:b? in my routes, as this mirrors the route matching in Rocket, which I'm also using: https://rocket.rs/v0.5-rc/guide/requests/#multiple-segments. Their multi-segment wildcard allows matching an empty path, which simplifies routing in an application that deals with file paths where you'd also want to accept the root path as valid.

@jbr
Copy link
Member

jbr commented May 9, 2022

@willhodges I'd be very excited to hear if yew was interested in switching to routefinder! Although the syntax is identical, there are several small differences: Routefinder only supports a single wildcard (*) per route, unnamed parameters are not supported, and the prioritization between routes may not be identical (I am not aware of any differences, but retaining route priority wasn't as important as making the priority behave as I thought was desirable). * does match the empty path in routefinder, which is another difference from route-recognizer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants