rwf2 / Rocket

A web framework for Rust.
https://rocket.rs
Other
24.41k stars 1.56k forks source link

Redirect all paths containing trailing slashes #1212

Closed jl-a closed 4 years ago

jl-a commented 4 years ago

(A similar question has been asked before (https://github.com/SergioBenitez/Rocket/issues/702) but only asked about a single path.)

How can I redirect any path containing a trailing slash to the same path but without the slash?

It's standard practice to avoid duplicate URLs with public facing webpages by redirecting them to either all include or all exclude a trailing slash. Manually writing a redirect for every existing path that could contain a trailing slash is generally unfeasible. I would like to match all paths that contain a trailing slash and perform a 301 redirect to the same path, but without the slash.

Including the trailing slash in the request route like so:

#[ get( "/<path..>/" ) ]
fn catch_all( path: PathBuf ) -> Redirect {
    ...
}

throws the error paths cannot contain empty segments which is understandable. Is it possible to match all paths with a trailing slash, and if so, how?

I'm aware I could do this with a proxy server like nginx, but I'm working on a fairly small project and would like to do it in app if possible.

Using Rocket 0.4.2

jebrosen commented 4 years ago

Is it possible to match all paths with a trailing slash, and if so, how?

I believe you will need a fairing in order to check every request like this. Unfortunately request fairings cannot directly respond to a request, so you will need a workaround. One possibility (e.g. as used by rocket_cors) is to change the request URI inside of on_request to an internal #[get] route, which would performs the actual redirect.

Since on_request fires for every request, you will have to check that request.uri() actually has a trailing slash before doing that redirect.

kw217 commented 4 years ago

Also related to #1198 .

SergioBenitez commented 4 years ago

Another option is to implement a custom handler, which can do whatever it wants, that matches on all paths and redirects to a /-less path if the path ends in /. This would be as simple as:

impl Handler for Redirector {
    fn handle<'r>(&self, req: &'r Request, _: Data) -> Outcome<'r> {
        if req.uri().path().ends_with('/') {
            Outcome::from(req, Redirect::to(req.uri().to_normalized()))
        } else {
            Outcome::forward(data)
        }
    }
}

You'll also need an Into<Vec<Route>> implementation. See the Handler docs for a complete example.

kratenko commented 4 years ago

I wanna point out that this is not just a SEO issue, but it also breaks all relative paths in a returned html-page. A <img src="dog.jpg"> at http://example.org/somewhere requests https://example.org/dog.jpg, at http://example.org/somewhere/ requests https://example.org/somewhere/dog.jpg. So serving the same page under the two uris, with and without trailing slash, really breaks the page! This just gave me some serious issues, so I think this is more than just a question.

kw217 commented 4 years ago

Yes, that's why I made a PR! See #1253 .

SergioBenitez commented 4 years ago

I believe this is now resolved by #1253! Closing.

threema-danilo commented 3 years ago

The solution implemented in #1253 only applies to StaticFiles, but is not a general solution for handlers, right?

I would like to enforce trailing slashes for all endpoints. Based on the suggestion by @SergioBenitez I attempted creating this catch-all handler:

#[rocket::async_trait]
impl Handler for AppendSlashes {
    async fn handle<'r>(&self, req: &'r rocket::Request<'_>, data: rocket::Data<'r>) -> Outcome<'r> {
        let has_trailing_slash = req.uri().path().ends_with('/');
        if !has_trailing_slash {
            match req.uri().map_path(|p| format!("{}/", p)) {
                Some(fixed_path) => Outcome::from(req, Redirect::to(fixed_path)),
                None => Outcome::failure(Status::InternalServerError),
            }
        } else {
            Outcome::forward(data)
        }
    }
}

However, this results in a lifetime error that I can't quite figure out...

error[E0759]: `req` has lifetime `'life1` but it needs to satisfy a `'static` lifetime requirement
   --> src/api/handlers.rs:115:23
    |
112 |     async fn handle<'r>(&self, req: &'r rocket::Request<'_>, data: rocket::Data<'r>) -> Outcome<'r> {
    |                                     ----------------------- this data with lifetime `'life1`...
...
115 |             match req.uri().map_path(|p| format!("{}/", p)) {
    |                       ^^^ ...is captured here...
116 |                 Some(fixed_path) => Outcome::from(req, Redirect::to(fixed_path)),
    |                                                        ------------ ...and is required to live as long as `'static` here

Does anyone have a successful redirect-to-trailing-slashes setup?

jebrosen commented 3 years ago

However, this results in a lifetime error that I can't quite figure out...

@threema-danilo It seems like what you need here is Redirect::to(fixed_path.into_owned()).

<Origin as IntoOwned>::into_owned()

threema-danilo commented 3 years ago

@jebrosen unfortunately that doesn't seem to help... I must be overlooking something.

Here's a reproducer: https://github.com/dbrgn/rocket-appendslashes

jebrosen commented 3 years ago

@threema-danilo Whoops - I had the right link, but the wrong text. It should have said Redirect::to(fixed_path.into_owned()).

Glitchy-Tozier commented 2 years ago

I'm still not quite sure how to do this. This is what I have written so far, based on previous comments.

There are no errors, but it seems to only ever get activated when calling the root-route.

#[derive(Clone)]
struct RemoveSlashes;

#[rocket::async_trait]
impl Handler for RemoveSlashes {
    async fn handle<'r>(
        &self,
        req: &'r rocket::Request<'_>,
        data: rocket::Data<'r>,
    ) -> Outcome<'r> {
        if req.uri().path().ends_with('/') && req.uri().path().to_string().chars().count() > 1 {
            Outcome::from(
                req,
                Redirect::to(req.uri().clone().into_owned().into_normalized()),
            )
        } else {
            Outcome::forward(data)
        }
    }
}
impl From<RemoveSlashes> for Vec<Route> {
    fn from(rs: RemoveSlashes) -> Vec<Route> {
        vec![Route::new(Method::Get, "/", rs)]
    }
}

/// The analog to the usual `fn main()`.
#[launch]
fn rocket() -> _ {
    let rocket_build = rocket::build()
        .mount("/", RemoveSlashes)
        . …other routes
}

EDIT: Got it working by using

vec![Route::new(Method::Get, "/<..>", rs)]
levkk commented 1 year ago

Ran into this today as well, thank you for the solution! It would be cool to include this as a config. This is a very common SEO optimization and other frameworks, e.g. Django, have this option.

@Glitchy-Tozier For your example, to make it fully functional, one needs to create a Route::ranked instead and give it high priority, e.g. -20, so it's executed before any other normal route.

phayes commented 1 year ago

I also ran into this today. It looks like something changed recently in Rocket so that routing that used to support extraneous trailing / no longer support it. Was running into a bunch of 404s after updating rocket and it turned out this was the cause.

SergioBenitez commented 1 year ago

I also ran into this today. It looks like something changed recently in Rocket so that routing that used to support extraneous trailing / no longer support it. Was running into a bunch of 404s after updating rocket and it turned out this was the cause.

This changed recently on master, yes. It resolves this issue globally. See the "reconsider normalization" issue for more.

Glitchy-Tozier commented 1 year ago

I assume that's the issue: https://github.com/SergioBenitez/Rocket/issues/2512