always redirect version-catchall URLs to their latest version

QuietMisdreavus · QuietMisdreavus · commit fe2ee9161fdb · 2019-05-03T15:34:41.000-05:00
diff --git 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<Response> {
 
     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
@@ -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<String> {
+        match self {
+            MatchVersion::Exact(v) | MatchVersion::Semver(v) => Some(v),
+            MatchVersion::None => None,
+        }
+    }
+}
 
-fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> Option<String> {
+/// 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<Response> {
     let mut response = Response::with((status::Ok, STYLE_CSS));
diff --git a/src/web/releases.rs b/src/web/releases.rs
@@ -484,7 +484,7 @@ pub fn search_handler(req: &mut Request) -> IronResult<Response> {
             }
 
 
-            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
@@ -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<Response> {
 
     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,17 +153,35 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult<Response> {
 
     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
     for _ in 0..2 {
         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<Response> {
     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<Response> {
                 }
             }
         }
-        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(),

Original file line number	Diff line number	Diff line change
`@@ -484,7 +484,7 @@ pub fn search_handler(req: &mut Request) -> IronResult<Response> {`
`484`	`484`	`}`
`485`	`485`
`486`	`486`
`487`		`- if let Some(version) = match_version(&conn, &query, None) {`
	`487`	`+ if let Some(version) = match_version(&conn, &query, None).into_option() {`
`488`	`488`	`// FIXME: This is a super dirty way to check if crate have rustdocs generated.`
`489`	`489`	`// match_version should handle this instead of this code block.`
`490`	`490`	`// This block is introduced to fix #163`