docs.rs/std/<path>?<args> should redirect path and args to doc.rust-lang

[tinaun]

it trips me up when docs.rs/std/ works but docs.rs/std/io doesn't

[jyn514]

Mentoring instructions:

• In build_routes, for each redirect to doc.rust-lang.org, add a route for /{route}/*

docs.rs/src/web/routes.rs

Line 129 in 267adca

for redirect in DOC_RUST_LANG_ORG_REDIRECTS {

• Change RustLangRedirector to accept routes other than a &'static str. You will have to refactor it slightly since it currently always redirects to a fixed URL instead of basing it off the request.

docs.rs/src/web/rustdoc.rs

Line 54 in 267adca

impl RustLangRedirector {

• Around standard_library_tests, add a test that docs.rs/std/io redirects to https://doc.rust-lang.org/stable/std/io/. This can either be a new test or part of the existing test.

docs.rs/src/web/mod.rs

Line 659 in 267adca

fn standard_library_redirects() {

If you have any questions, feel free to ask here or on Discord!

[robinhundt]

This looks like something I'd definitely use myself. I'll start working on this.

[robinhundt]

Mhh, so something weird is going on. I first tried adding a new internal_page route just using the current RustLangRedirector:

    for redirect in DOC_RUST_LANG_ORG_REDIRECTS {
        routes.internal_page(
            &format!("/{}", redirect),
            super::rustdoc::RustLangRedirector::new(redirect),
        );
        routes.internal_page(
            &format!("/{}/*", redirect),
            super::rustdoc::RustLangRedirector::new(redirect),
        );
    }

https://github.com/robinhundt/docs.rs/blob/2638dca77ad0beee7c2399485f0ceb2bdcfc5a25/src/web/routes.rs#L127-L136

Unfortunately this doesn't seem to work. E.g. going to /std/io/ results in a crate not found. /std/ continues to redirect. The interesting thing is that both

routes.internal_page(
    &format!("/{}/:something", redirect),
    super::rustdoc::RustLangRedirector::new(redirect),
);

and

routes.internal_page(
    &format!("/{}/test", redirect),
    super::rustdoc::RustLangRedirector::new(redirect),
);

will redirect for the corresponding GET requests.
At first I thought it had something to do with the BlockBlacklistedPrefixes handler, but that is run in the handler chain before the routes handler and the 404 it returns should just be ignored. (By the way, I found some dead code that looks fishy here: https://github.com/robinhundt/docs.rs/blob/2638dca77ad0beee7c2399485f0ceb2bdcfc5a25/src/web/routes.rs#L174-L184 rustdoc_get is never not empty as far as I can tell).

So then I thought it might be a bug in the old version of the router lib that is used, but this small repo https://github.com/robinhundt/test-iron-router/blob/master/src/main.rs seems to suggest that that is not the case, since the wildcard works just fine there.

So I'm a little bit stumped as to why the handler is called for the normal or match pattern but not for the wildcard one.

[Nemo157]

It may still be a bug in the router, maybe related to http-rs/route-recognizer#20 causing the priorities between the different routes to be wrong (I'm actually surprised that docs.rs has not hit this yet, on my other iron using project I encountered it almost straight away).

[jyn514]

Try updating the router, then? It looks like that issue was fixed in a later version.

[Nemo157]

It's fixed in the underlying library the router uses, but afaik iron's router itself hasn't updated that library.

[jyn514]

Hmm, we might be able to use patch.crates-io but at that point it's getting a little hacky.

[robinhundt]

@Nemo157 you're right. It seems to be the route

    routes.rustdoc_page(
        "/:crate/:version",
        super::rustdoc::rustdoc_redirector_handler,
    )

that is more specific than /static/*.
I thought sending a PR for iron/router shouldn't be to difficult... turns out the master branch doesn't even compile when testing due to #![cfg_attr(test, deny(warnings))] and usage of Error::description and after disabling that there is a failing test case related to query parameter handling :/
So it seems that in order to fix this problem, first iron/router needs it's dependencies updated which would probably result in a new breaking release.

[jyn514]

Iron isn't really maintained, so I'm not sure how realistic it is to wait for that PR to be merged ... it's also unlikely they'd backport to 0.5, which we're still stuck on. I think this needs either a different approach in docs.rs or to wait for the switch to hyper (#747).

[Kixiron]

Trying to get fixes into iron is probably a lost cause, like Joshua said. We can't move to 0.6 because of mysterious panics and some fun compile errors so even if they got in, we couldn't benefit from them

[robinhundt]

Mhh, that's unfortunate. In the meantime we could use the pattern /static/:module to redirect to top-level modules of the standard library so that it at least works for that case. Although I'm not sure if it makes sense to introduce half-baked code for this, I assume, low priority feature, if there are major refactors due to the switch to hyper in the works anyway.

[Nemo157]

I'm working on this now so that we can have the new https://docs.rs/std::vec like urls too.