URL-encode canonical URLs when they can include UTF8 characters

Author: syphar

Cargo.lock

@@ -99,6 +99,7 @@ tower-service = "0.3.2"
 tower-http = { version = "0.3.4", features = ["trace"] }
 mime = "0.3.16"
 httpdate = "1.0.2"
+percent-encoding = "2.2.0"
 
 # NOTE: if you change this, also double-check that the comment in `queue_builder::remove_tempdirs` is still accurate.
 tempfile = "3.1.0"

Cargo.toml

@@ -115,6 +115,7 @@ use iron::{
     Chain, Handler, Iron, IronError, IronResult, Listening, Request, Response, Url,
 };
 use page::TemplateData;
+use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS};
 use postgres::Client;
 use router::{NoRoute, TrailingSlash};
 use semver::{Version, VersionReq};
@@ -126,6 +127,15 @@ use tower::ServiceBuilder;
 use tower_http::trace::TraceLayer;
 use url::form_urlencoded;
 
+// from https://github.com/servo/rust-url/blob/master/url/src/parser.rs
+// and https://github.com/tokio-rs/axum/blob/main/axum-extra/src/lib.rs
+const FRAGMENT: &AsciiSet = &CONTROLS.add(b' ').add(b'"').add(b'<').add(b'>').add(b'`');
+const PATH: &AsciiSet = &FRAGMENT.add(b'#').add(b'?').add(b'{').add(b'}');
+
+pub(crate) fn encode_url_path(path: &str) -> String {
+    utf8_percent_encode(path, PATH).to_string()
+}
+
 /// Duration of static files for staticfile and DatabaseFileHandler (in seconds)
 const STATIC_FILE_CACHE_DURATION: u64 = 60 * 60 * 24 * 30 * 12; // 12 months
 

src/web/mod.rs

@@ -10,6 +10,7 @@ use crate::{
         cache::CachePolicy,
         crate_details::CrateDetails,
         csp::Csp,
+        encode_url_path,
         error::{AxumNope, AxumResult, Nope},
         file::File,
         match_version, match_version_axum,
@@ -571,7 +572,7 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult<Response> {
     let canonical_url = format!(
         "https://docs.rs/{}/latest/{}",
         name,
-        inner_path.replace("index.html", "")
+        encode_url_path(&inner_path.replace("index.html", ""))
     );
 
     metrics
@@ -876,7 +877,11 @@ impl Handler for LegacySharedResourceHandler {
 
 #[cfg(test)]
 mod test {
-    use crate::{test::*, web::cache::CachePolicy, Config};
+    use crate::{
+        test::*,
+        web::{cache::CachePolicy, encode_url_path},
+        Config,
+    };
     use anyhow::Context;
     use kuchiki::traits::TendrilSink;
     use reqwest::{blocking::ClientBuilder, redirect, StatusCode};
@@ -2347,11 +2352,13 @@ mod test {
                 .rustdoc_file("dummy_dash/index.html")
                 .create()?;
 
+            let utf8_filename = "序列化工具简单测试结果.html";
             env.fake_release()
                 .name("dummy-docs")
                 .version("0.1.0")
                 .documentation_url(Some("https://docs.rs/foo".to_string()))
                 .rustdoc_file("dummy_docs/index.html")
+                .rustdoc_file(&format!("dummy_docs/{utf8_filename}"))
                 .create()?;
 
             env.fake_release()
@@ -2384,6 +2391,20 @@ mod test {
                 .unwrap()
                 .contains("<https://docs.rs/dummy-docs/latest/dummy_docs/>; rel=\"canonical\""),);
 
+            assert_eq!(
+                web.get(&format!("/dummy-docs/0.1.0/dummy_docs/{utf8_filename}"))
+                    .send()?
+                    .headers()
+                    .get("link")
+                    .unwrap()
+                    .to_str()
+                    .unwrap(),
+                format!(
+                    "<https://docs.rs/dummy-docs/latest/dummy_docs/{}>; rel=\"canonical\"",
+                    encode_url_path(utf8_filename)
+                )
+            );
+
             assert!(web
                 .get("/dummy-nodocs/0.1.0/dummy_nodocs/")
                 .send()?
@@ -2407,6 +2428,7 @@ mod test {
             .unwrap()
             .contains("<https://docs.rs/dummy-nodocs/latest/dummy_nodocs/struct.Foo.html>; rel=\"canonical\""),
             );
+
             Ok(())
         })
     }

src/web/rustdoc.rs

@@ -4,8 +4,8 @@ use crate::{
     impl_axum_webpage,
     utils::{get_correct_docsrs_style_file, spawn_blocking},
     web::{
-        cache::CachePolicy, error::AxumNope, file::File as DbFile, headers::CanonicalUrl,
-        MatchSemver, MetaData,
+        cache::CachePolicy, encode_url_path, error::AxumNope, file::File as DbFile,
+        headers::CanonicalUrl, MatchSemver, MetaData,
     },
     Storage,
 };
@@ -253,7 +253,11 @@ pub(crate) async fn source_browser_handler(
     })
     .await?;
 
-    let canonical_url = format!("https://docs.rs/crate/{}/latest/source/{}", name, path);
+    let canonical_url = format!(
+        "https://docs.rs/crate/{}/latest/source/{}",
+        name,
+        encode_url_path(&path),
+    );
 
     let (file, file_content) = if let Some(blob) = blob {
         let is_text = blob.mime.starts_with("text") || blob.mime == "application/json";
@@ -321,7 +325,7 @@ pub(crate) async fn source_browser_handler(
 #[cfg(test)]
 mod tests {
     use crate::test::*;
-    use crate::web::cache::CachePolicy;
+    use crate::web::{cache::CachePolicy, encode_url_path};
     use kuchiki::traits::TendrilSink;
     use reqwest::StatusCode;
     use test_case::test_case;
@@ -338,6 +342,36 @@ mod tests {
             .collect()
     }
 
+    #[test_case(true)]
+    #[test_case(false)]
+    fn fetch_source_file_utf8_path(archive_storage: bool) {
+        wrapper(|env| {
+            let filename = "序列化工具简单测试结果.pdf";
+
+            env.fake_release()
+                .archive_storage(archive_storage)
+                .name("fake")
+                .version("0.1.0")
+                .source_file(filename, b"some_random_content")
+                .create()?;
+
+            let web = env.frontend();
+            let response = web
+                .get(&format!("/crate/fake/0.1.0/source/{filename}"))
+                .send()?;
+            assert!(response.status().is_success());
+            assert_eq!(
+                response.headers().get("link").unwrap(),
+                &format!(
+                    "<https://docs.rs/crate/fake/latest/source/{}>; rel=\"canonical\"",
+                    encode_url_path(filename),
+                )
+            );
+            assert!(response.text()?.contains("some_random_content"));
+            Ok(())
+        });
+    }
+
     #[test_case(true)]
     #[test_case(false)]
     fn fetch_source_file_content(archive_storage: bool) {