use clean URLs as hashmap keys

.travis.yml

@@ -16,9 +16,9 @@ before_script:
     - rustup component add rustfmt
 
 script:
-   - cargo build --all --locked --verbose
-   - cargo test --all --locked --verbose
-   - cargo fmt --all -- --check
+    - cargo build --all --locked --verbose
+    - cargo test --all --locked --verbose
+    - cargo fmt --all -- --check
 
 jobs:
   allow_failures:

src/http.rs

@@ -1,7 +1,7 @@
 use reqwest::header::CONTENT_TYPE;
 use reqwest::Client;
 use std::collections::HashMap;
-use utils::{data_to_dataurl, is_data_url};
+use utils::{clean_url, data_to_dataurl, is_data_url};
 
 pub fn retrieve_asset(
     cache: &mut HashMap<String, String>,
@@ -11,15 +11,17 @@ pub fn retrieve_asset(
     mime: &str,
     opt_silent: bool,
 ) -> Result<(String, String), reqwest::Error> {
+    let cache_key = clean_url(&url);
+
     if is_data_url(&url).unwrap() {
         Ok((url.to_string(), url.to_string()))
     } else {
-        if cache.contains_key(&url.to_string()) {
+        if cache.contains_key(&cache_key) {
             // url is in cache
             if !opt_silent {
                 eprintln!("{} (from cache)", &url);
             }
-            let data = cache.get(&url.to_string()).unwrap();
+            let data = cache.get(&cache_key).unwrap();
             Ok((data.to_string(), url.to_string()))
         } else {
             // url not in cache, we request it
@@ -33,6 +35,8 @@ pub fn retrieve_asset(
                 }
             }
 
+            let new_cache_key = clean_url(response.url().to_string());
+
             if as_dataurl {
                 // Convert response into a byte array
                 let mut data: Vec<u8> = vec![];
@@ -50,12 +54,12 @@ pub fn retrieve_asset(
                 };
                 let dataurl = data_to_dataurl(&mimetype, &data);
                 // insert in cache
-                cache.insert(response.url().to_string(), dataurl.to_string());
+                cache.insert(new_cache_key, dataurl.to_string());
                 Ok((dataurl, response.url().to_string()))
             } else {
                 let content = response.text().unwrap();
                 // insert in cache
-                cache.insert(response.url().to_string(), content.clone());
+                cache.insert(new_cache_key, content.clone());
                 Ok((content, response.url().to_string()))
             }
         }

src/tests/utils.rs

@@ -1,5 +1,6 @@
 use crate::utils::{
-    data_to_dataurl, detect_mimetype, is_data_url, is_valid_url, resolve_url, url_has_protocol,
+    clean_url, data_to_dataurl, detect_mimetype, is_data_url, is_valid_url, resolve_url,
+    url_has_protocol,
 };
 use url::ParseError;
 
@@ -158,3 +159,19 @@ fn test_is_data_url() {
     assert!(!is_data_url("//kernel.org").unwrap_or(false));
     assert!(!is_data_url("").unwrap_or(false));
 }
+
+#[test]
+fn test_clean_url() {
+    assert_eq!(
+        clean_url("https://somewhere.com/font.eot#iefix"),
+        "https://somewhere.com/font.eot"
+    );
+    assert_eq!(
+        clean_url("https://somewhere.com/font.eot#"),
+        "https://somewhere.com/font.eot"
+    );
+    assert_eq!(
+        clean_url("https://somewhere.com/font.eot?#"),
+        "https://somewhere.com/font.eot"
+    );
+}

src/utils.rs

@@ -196,3 +196,14 @@ pub fn resolve_css_imports(
         resolved_css
     }
 }
+
+pub fn clean_url<T: AsRef<str>>(url: T) -> String {
+    let mut result = Url::parse(url.as_ref()).unwrap();
+    // Clear fragment
+    result.set_fragment(None);
+    // Get rid of stray question mark
+    if result.query() == Some("") {
+        result.set_query(None);
+    }
+    result.to_string()
+}