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

Not allowing a trailing slash is a bad idea #82

Open
bvinc opened this issue May 23, 2015 · 7 comments
Open

Not allowing a trailing slash is a bad idea #82

bvinc opened this issue May 23, 2015 · 7 comments

Comments

@bvinc
Copy link

bvinc commented May 23, 2015

A trailing slash in a URL is important. A URL with a trailing slash is a completely different URL from a URL without a trailing slash. For example, relative links are treated differently.

The router should allow users to treat these URLs differently.

More information:
https://cdivilly.wordpress.com/2014/03/11/why-trailing-slashes-on-uris-are-important/

@bvinc
Copy link
Author

bvinc commented May 23, 2015

Separately, the 301 Moved responses are using absolute URLs, but those URLs are wrong in the case that the router is under a mount. For example, when requesting http://localhost/mount/route/, I get redirected to http://localhost/route.

Hopefully, we can agree to remove the 301 Moved responses entirely and this won't be an issue.

@bvinc
Copy link
Author

bvinc commented Jun 16, 2015

Looking at how other routers handle this issue, the general consensus is that:

  • Urls with and without trailing slashes should be handled differently. A handler can exist with a trailing slash, and a different handler can exist without the trailing slash. Both should work independently.
  • An option, named something like RedirectTrailingSlash should exist. If it is set to true, and a requested route isn't found, but a route is found by adding (or removing) a trailing slash, a redirect is issued.
  • This option should probably default to true.

@reem
Copy link
Member

reem commented Jun 16, 2015

Sorry for not posting on this for so long. I think you are right, and the current behavior is incorrect, at least as a default. We should move to implement something along the lines of what you outlined in your last post.

@reem
Copy link
Member

reem commented Jul 4, 2015

This should be fixed by #84, @bvinc does that patch address your concerns?

@freiguy1
Copy link

I'm now redirected to a url without the trailing slash, however if my router is behind a mount, the mounted part of the url disappears.

For instance:

let mut router = Router::new();
router.get("/bar", ...somehandler...);

let mut mount = Mount::new();
mount.mount("/foo", router);

If I navigate to http://url.com/foo/bar/, I'll get redirected to http://url.com/bar.

@reem
Copy link
Member

reem commented Aug 28, 2015

Hmm, this is because router doesn't current respect mount's OriginalUrl extension... I will have to think about this.

@dbrgn
Copy link

dbrgn commented Mar 10, 2016

I have the same problem. This is the proof-of-error:

extern crate iron;
extern crate router;
extern crate mount;

use iron::{Iron, Request, Response, IronResult, status};
use router::Router;
use mount::Mount;
use std::net::Ipv4Addr;

fn handler(_: &mut Request) -> IronResult<Response> {
    println!("Handled request");
    Ok(Response::with((status::Ok, "Ok")))
}

fn main() {
    let mut router = Router::new();
    router.get("/", handler);
    router.get("/foo", handler);

    let mut mount = Mount::new();
    mount.mount("/api/v1/", router);

    Iron::new(mount).http((Ipv4Addr::new(127, 0, 0, 1), 8080)).unwrap();
}

A request to /api/v1/foo works, but a request to /api/v1/foo/ results in a redirect to /. The mount is ignored.

This issue should probably be labelled as bug.

Regarding the trailing slash in general, I think it should be configurable whether or not it should redirect. The config options could be something like this:

pub enum TrailingSlashConfig {
    // When the trailing slash is missing and no route matches, append the trailing slash
    Require,
    // When a trailing slash is present and no route matches, remove the trailing slash
    Strip,
    // Handle present and absent trailing slashes with the same handler, no redirects
    Ignore,
    // Handle present and absent trailing slashes as two different endpoints
    Discern,
}

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

Successfully merging a pull request may close this issue.

4 participants