From fe2ee9161fdb69439eccdc8450c4b178a4c96a11 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Tue, 30 Apr 2019 10:51:47 -0500 Subject: [PATCH 1/2] always redirect version-catchall URLs to their latest version --- src/web/crate_details.rs | 28 +++++++++++++++------ src/web/mod.rs | 53 ++++++++++++++++++++++++++++++++-------- src/web/releases.rs | 2 +- src/web/rustdoc.rs | 42 +++++++++++++++++++++++++------ 4 files changed, 100 insertions(+), 25 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 9c7b6aeb1..077058a55 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -2,11 +2,11 @@ use super::pool::Pool; -use super::{MetaData, duration_to_str, match_version, render_markdown}; +use super::{MetaData, duration_to_str, match_version, render_markdown, MatchVersion}; use super::error::Nope; use super::page::Page; use iron::prelude::*; -use iron::status; +use iron::{Url, status}; use std::collections::BTreeMap; use time; use rustc_serialize::json::{Json, ToJson}; @@ -229,14 +229,28 @@ pub fn crate_details_handler(req: &mut Request) -> IronResult { let conn = extension!(req, Pool); - match_version(&conn, &name, req_version) - .and_then(|version| CrateDetails::new(&conn, &name, &version)) - .ok_or(IronError::new(Nope::CrateNotFound, status::NotFound)) - .and_then(|details| { + match match_version(&conn, &name, req_version) { + MatchVersion::Exact(version) => { + let details = CrateDetails::new(&conn, &name, &version); + Page::new(details) .set_true("show_package_navigation") .set_true("javascript_highlightjs") .set_true("package_navigation_crate_tab") .to_resp("crate_details") - }) + } + MatchVersion::Semver(version) => { + let url = ctry!(Url::parse(&format!("{}://{}:{}/crate/{}/{}", + req.url.scheme(), + req.url.host(), + req.url.port(), + name, + version)[..])); + + Ok(super::redirect(url)) + } + MatchVersion::None => { + Err(IronError::new(Nope::CrateNotFound, status::NotFound)) + } + } } diff --git a/src/web/mod.rs b/src/web/mod.rs index 5b9fe199e..63761b7f6 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -50,8 +50,9 @@ use std::error::Error; use std::time::Duration; use std::path::PathBuf; use iron::prelude::*; -use iron::{self, Handler, status}; -use iron::headers::{CacheControl, CacheDirective, ContentType}; +use iron::{self, Handler, Url, status}; +use iron::headers::{Expires, HttpDate, CacheControl, CacheDirective, ContentType}; +use iron::modifiers::Redirect; use router::{Router, NoRoute}; use staticfile::Static; use handlebars_iron::{HandlebarsEngine, DirectorySource}; @@ -258,9 +259,34 @@ impl Handler for CratesfyiHandler { } } +/// Represents the possible results of attempting to load a version requirement. +enum MatchVersion { + /// `match_version` was given an exact version, which matched a saved crate version. + Exact(String), + /// `match_version` was given a semver version requirement, which matched the given saved crate + /// version. + Semver(String), + /// `match_version` was given a version requirement which did not match any saved crate + /// versions. + None, +} +impl MatchVersion { + /// Convert this `MatchVersion` into an `Option`, discarding whether the matched version came + /// from an exact version number or a semver requirement. + pub fn into_option(self) -> Option { + match self { + MatchVersion::Exact(v) | MatchVersion::Semver(v) => Some(v), + MatchVersion::None => None, + } + } +} -fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> Option { +/// Checks the database for crate releases that match the given name and version. +/// +/// `version` may be an exact version number or loose semver version requirement. The return value +/// will indicate whether the given version exactly matched a version number from the database. +fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> MatchVersion { // version is an Option<&str> from router::Router::get // need to decode first @@ -278,7 +304,7 @@ fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> Option let mut versions = Vec::new(); let rows = conn.query("SELECT versions FROM crates WHERE name = $1", &[&name]).unwrap(); if rows.len() == 0 { - return None; + return MatchVersion::None; } let versions_json: Json = rows.get(0).get(0); for version in versions_json.as_array().unwrap() { @@ -293,14 +319,14 @@ fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> Option // we can't expect users to use semver in query for version in &versions { if version == &req_version { - return Some(version.clone()); + return MatchVersion::Exact(version.clone()); } } // Now try to match with semver let req_sem_ver = match VersionReq::parse(&req_version) { Ok(v) => v, - Err(_) => return None, + Err(_) => return MatchVersion::None, }; // we need to sort versions first @@ -312,7 +338,7 @@ fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> Option // but check result just in case let version = match Version::parse(&version) { Ok(v) => v, - Err(_) => return None, + Err(_) => return MatchVersion::None, }; versions_sem.push(version); } @@ -325,16 +351,16 @@ fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> Option // semver is acting weird for '*' (any) range if a crate only have pre-release versions // return first version if requested version is '*' if req_version == "*" && !versions_sem.is_empty() { - return Some(format!("{}", versions_sem[0])); + return MatchVersion::Semver(format!("{}", versions_sem[0])); } for version in &versions_sem { if req_sem_ver.matches(&version) { - return Some(format!("{}", version)); + return MatchVersion::Semver(format!("{}", version)); } } - None + MatchVersion::None } @@ -431,7 +457,14 @@ fn duration_to_str(ts: time::Timespec) -> String { } +/// Creates a `Response` which redirects to the given path on the scheme/host/port from the given +/// `Request`. +fn redirect(url: Url) -> Response { + let mut resp = Response::with((status::Found, Redirect(url))); + resp.headers.set(Expires(HttpDate(time::now()))); + resp +} fn style_css_handler(_: &mut Request) -> IronResult { let mut response = Response::with((status::Ok, STYLE_CSS)); diff --git a/src/web/releases.rs b/src/web/releases.rs index 1605e19c5..0fd7a9d5a 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -484,7 +484,7 @@ pub fn search_handler(req: &mut Request) -> IronResult { } - if let Some(version) = match_version(&conn, &query, None) { + if let Some(version) = match_version(&conn, &query, None).into_option() { // FIXME: This is a super dirty way to check if crate have rustdocs generated. // match_version should handle this instead of this code block. // This block is introduced to fix #163 diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 5bba8387e..a48ddfc32 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -9,7 +9,7 @@ use iron::prelude::*; use iron::{status, Url}; use iron::modifiers::Redirect; use router::Router; -use super::match_version; +use super::{match_version, MatchVersion}; use super::error::Nope; use super::page::Page; use rustc_serialize::json::{Json, ToJson}; @@ -120,7 +120,7 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult { let conn = extension!(req, Pool); - let version = match match_version(&conn, &crate_name, req_version) { + let version = match match_version(&conn, &crate_name, req_version).into_option() { Some(v) => v, None => return Err(IronError::new(Nope::CrateNotFound, status::NotFound)), }; @@ -153,10 +153,10 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { let router = extension!(req, Router); let name = router.find("crate").unwrap_or("").to_string(); - let version = router.find("version"); + let url_version = router.find("version"); + let version; // pre-declaring it to enforce drop order relative to `req_path` let conn = extension!(req, Pool); - let version = try!(match_version(&conn, &name, version) - .ok_or(IronError::new(Nope::ResourceNotFound, status::NotFound))); + let mut req_path = req.url.path(); // remove name and version from path @@ -164,6 +164,24 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { req_path.remove(0); } + version = match match_version(&conn, &name, url_version) { + MatchVersion::Exact(v) => v, + MatchVersion::Semver(v) => { + // to prevent cloudfront caching the wrong artifacts on URLs with loose semver + // versions, redirect the browser to the returned version instead of loading it + // immediately + let url = ctry!(Url::parse(&format!("{}://{}:{}/{}/{}/{}", + req.url.scheme(), + req.url.host(), + req.url.port(), + name, + v, + req_path.join("/"))[..])); + return Ok(super::redirect(url)); + } + MatchVersion::None => return Err(IronError::new(Nope::ResourceNotFound, status::NotFound)), + }; + // docs have "rustdoc" prefix in database req_path.insert(0, "rustdoc"); @@ -240,7 +258,7 @@ pub fn badge_handler(req: &mut Request) -> IronResult { let conn = extension!(req, Pool); let options = match match_version(&conn, &name, Some(&version)) { - Some(version) => { + MatchVersion::Exact(version) => { let rows = ctry!(conn.query("SELECT rustdoc_status FROM releases INNER JOIN crates ON crates.id = releases.crate_id @@ -260,7 +278,17 @@ pub fn badge_handler(req: &mut Request) -> IronResult { } } } - None => { + MatchVersion::Semver(version) => { + let url = ctry!(Url::parse(&format!("{}://{}:{}/{}/badge.svg?version={}", + req.url.scheme(), + req.url.host(), + req.url.port(), + name, + version)[..])); + + return Ok(super::redirect(url)); + } + MatchVersion::None => { BadgeOptions { subject: "docs".to_owned(), status: "no builds".to_owned(), From 8d3c61b0a53573cc5e549242e5776f429a51df47 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Fri, 3 May 2019 16:22:37 -0500 Subject: [PATCH 2/2] add some explanatory comments --- src/web/releases.rs | 4 +++- src/web/rustdoc.rs | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/web/releases.rs b/src/web/releases.rs index 0fd7a9d5a..0d63f8c98 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -483,7 +483,9 @@ pub fn search_handler(req: &mut Request) -> IronResult { return Ok(resp); } - + // 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 Some(version) = match_version(&conn, &query, None).into_option() { // FIXME: This is a super dirty way to check if crate have rustdocs generated. // match_version should handle this instead of this code block. diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index a48ddfc32..f326ddf5a 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -66,7 +66,8 @@ impl ToJson for RustdocPage { } - +/// Handler called for `/:crate` and `/:crate/:version` URLs. Automatically redirects to the docs +/// or crate details page based on whether the given crate version was successfully built. pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult { fn redirect_to_doc(req: &Request, @@ -120,6 +121,8 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult { let conn = extension!(req, Pool); + // it doesn't matter if the version that was given was exact or not, since we're redirecting + // anyway let version = match match_version(&conn, &crate_name, req_version).into_option() { Some(v) => v, None => return Err(IronError::new(Nope::CrateNotFound, status::NotFound)),