diff --git a/src/web/mod.rs b/src/web/mod.rs index 34683c328..5e6ab9f74 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -728,24 +728,24 @@ mod test { wrapper(|env| { let web = env.frontend(); for krate in &["std", "alloc", "core", "proc_macro", "test"] { - let target = format!("https://doc.rust-lang.org/stable/{}/", krate); + let target = format!("https://doc.rust-lang.org/stable/{}/?", krate); // with or without slash assert_redirect(&format!("/{}", krate), &target, web)?; assert_redirect(&format!("/{}/", krate), &target, web)?; } - let target = "https://doc.rust-lang.org/stable/proc_macro/"; + let target = "https://doc.rust-lang.org/stable/proc_macro/?"; // with or without slash assert_redirect("/proc-macro", target, web)?; assert_redirect("/proc-macro/", target, web)?; - let target = "https://doc.rust-lang.org/nightly/nightly-rustc/"; + let target = "https://doc.rust-lang.org/nightly/nightly-rustc/?"; // with or without slash assert_redirect("/rustc", target, web)?; assert_redirect("/rustc/", target, web)?; - let target = "https://doc.rust-lang.org/nightly/nightly-rustc/rustdoc/"; + let target = "https://doc.rust-lang.org/nightly/nightly-rustc/rustdoc/?"; // with or without slash assert_redirect("/rustdoc", target, web)?; assert_redirect("/rustdoc/", target, web)?; diff --git a/src/web/releases.rs b/src/web/releases.rs index c74652169..2677586e7 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -545,7 +545,7 @@ impl_webpage! { pub fn search_handler(req: &mut Request) -> IronResult { let url = req.url.as_ref(); - let params: HashMap<_, _> = url.query_pairs().collect(); + let mut params: HashMap<_, _> = url.query_pairs().collect(); let query = params .get("query") .map(|q| q.to_string()) @@ -555,30 +555,41 @@ pub fn search_handler(req: &mut Request) -> IronResult { // check if I am feeling lucky button pressed and redirect user to crate page // if there is a match. Also check for paths to items within crates. - if params.contains_key("i-am-feeling-lucky") || query.contains("::") { + if params.remove("i-am-feeling-lucky").is_some() || query.contains("::") { // redirect to a random crate if query is empty if query.is_empty() { return redirect_to_random_crate(req, &mut conn); } - let (krate, query) = match query.split_once("::") { - Some((krate, query)) => (krate.to_string(), format!("?search={query}")), - None => (query.clone(), "".to_string()), + let mut queries = std::collections::BTreeMap::new(); + + let krate = match query.split_once("::") { + Some((krate, query)) => { + queries.insert("search", query); + krate.to_string() + } + None => query.clone(), }; // since we never pass a version into `match_version` here, we'll never get // `MatchVersion::Exact`, so the distinction between `Exact` and `Semver` doesn't // matter if let Ok(matchver) = match_version(&mut conn, &krate, None) { + params.remove("query"); + queries.extend(params.iter().map(|(k, v)| (k.as_ref(), v.as_ref()))); let (version, _) = matchver.version.into_parts(); let krate = matchver.corrected_name.unwrap_or(krate); let base = redirect_base(req); let url = if matchver.rustdoc_status { let target_name = matchver.target_name; + let path = format!("{base}/{krate}/{version}/{target_name}/"); ctry!( req, - Url::parse(&format!("{base}/{krate}/{version}/{target_name}/{query}")) + Url::from_generic_url(ctry!( + req, + iron::url::Url::parse_with_params(&path, queries) + )) ) } else { ctry!(req, Url::parse(&format!("{base}/crate/{krate}/{version}"))) @@ -827,7 +838,7 @@ mod tests { assert_redirect( "/releases/search?query=some_random_crate&i-am-feeling-lucky=1", - "/some_random_crate/1.0.0/some_random_crate/", + "/some_random_crate/1.0.0/some_random_crate/?", web, )?; Ok(()) @@ -877,7 +888,22 @@ mod tests { )?; assert_redirect( "/releases/search?query=some_random_crate::some::path", - "/some_random_crate/1.0.0/some_random_crate/?search=some::path", + "/some_random_crate/1.0.0/some_random_crate/?search=some%3A%3Apath", + web, + )?; + Ok(()) + }) + } + + #[test] + fn search_coloncolon_path_redirects_to_crate_docs_and_keeps_query() { + wrapper(|env| { + let web = env.frontend(); + env.fake_release().name("some_random_crate").create()?; + + assert_redirect( + "/releases/search?query=some_random_crate::somepath&go_to_first=true", + "/some_random_crate/1.0.0/some_random_crate/?go_to_first=true&search=somepath", web, )?; Ok(()) diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 96985c9ee..a0eabfbd0 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -42,18 +42,25 @@ static DOC_RUST_LANG_ORG_REDIRECTS: Lazy> = Lazy::new(|| { pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult { fn redirect_to_doc( req: &Request, - mut url_str: String, + url_str: String, permanent: bool, path_in_crate: Option<&str>, ) -> IronResult { - if let Some(query) = req.url.query() { - url_str.push('?'); - url_str.push_str(query); - } else if let Some(path) = path_in_crate { - url_str.push_str("?search="); - url_str.push_str(path); + let mut queries: std::collections::BTreeMap< + std::borrow::Cow<'_, str>, + std::borrow::Cow<'_, str>, + > = std::collections::BTreeMap::new(); + if let Some(path) = path_in_crate { + queries.insert("search".into(), path.into()); } - let url = ctry!(req, Url::parse(&url_str)); + queries.extend(req.url.as_ref().query_pairs()); + let url = ctry!( + req, + Url::from_generic_url(ctry!( + req, + iron::url::Url::parse_with_params(&url_str, queries) + )) + ); let (status_code, max_age) = if permanent { (status::MovedPermanently, 86400) } else { @@ -858,8 +865,8 @@ mod test { "/dummy/0.3.0/all.html", web, )?; - assert_redirect("/dummy/0.3.0/", base, web)?; - assert_redirect("/dummy/0.3.0/index.html", base, web)?; + assert_redirect("/dummy/0.3.0/", &format!("{}?", base), web)?; + assert_redirect("/dummy/0.3.0/index.html", &format!("{}?", base), web)?; Ok(()) }); } @@ -1176,7 +1183,7 @@ mod test { .create()?; let web = env.frontend(); - assert_redirect("/fake%2Dcrate", "/fake-crate/latest/fake_crate/", web)?; + assert_redirect("/fake%2Dcrate", "/fake-crate/latest/fake_crate/?", web)?; Ok(()) }); @@ -1206,37 +1213,37 @@ mod test { let web = env.frontend(); - assert_redirect("/dummy_dash", "/dummy-dash/latest/dummy_dash/", web)?; - assert_redirect("/dummy_dash/*", "/dummy-dash/0.2.0/dummy_dash/", web)?; - assert_redirect("/dummy_dash/0.1.0", "/dummy-dash/0.1.0/dummy_dash/", web)?; + assert_redirect("/dummy_dash", "/dummy-dash/latest/dummy_dash/?", web)?; + assert_redirect("/dummy_dash/*", "/dummy-dash/0.2.0/dummy_dash/?", web)?; + assert_redirect("/dummy_dash/0.1.0", "/dummy-dash/0.1.0/dummy_dash/?", web)?; assert_redirect( "/dummy-underscore", - "/dummy_underscore/latest/dummy_underscore/", + "/dummy_underscore/latest/dummy_underscore/?", web, )?; assert_redirect( "/dummy-underscore/*", - "/dummy_underscore/0.2.0/dummy_underscore/", + "/dummy_underscore/0.2.0/dummy_underscore/?", web, )?; assert_redirect( "/dummy-underscore/0.1.0", - "/dummy_underscore/0.1.0/dummy_underscore/", + "/dummy_underscore/0.1.0/dummy_underscore/?", web, )?; assert_redirect( "/dummy-mixed_separators", - "/dummy_mixed-separators/latest/dummy_mixed_separators/", + "/dummy_mixed-separators/latest/dummy_mixed_separators/?", web, )?; assert_redirect( "/dummy_mixed_separators/*", - "/dummy_mixed-separators/0.2.0/dummy_mixed_separators/", + "/dummy_mixed-separators/0.2.0/dummy_mixed_separators/?", web, )?; assert_redirect( "/dummy-mixed-separators/0.1.0", - "/dummy_mixed-separators/0.1.0/dummy_mixed_separators/", + "/dummy_mixed-separators/0.1.0/dummy_mixed_separators/?", web, )?; @@ -1773,13 +1780,18 @@ mod test { )?; assert_redirect( "/some_random_crate::some::path", - "/some_random_crate/latest/some_random_crate/?search=some::path", + "/some_random_crate/latest/some_random_crate/?search=some%3A%3Apath", + web, + )?; + assert_redirect( + "/some_random_crate::some::path?go_to_first=true", + "/some_random_crate/latest/some_random_crate/?go_to_first=true&search=some%3A%3Apath", web, )?; assert_redirect( "/std::some::path", - "https://doc.rust-lang.org/stable/std/?search=some::path", + "https://doc.rust-lang.org/stable/std/?search=some%3A%3Apath", web, )?;