From bd4ca721eb2aa7b772a3834add2c37f606d4568b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Date: Sat, 11 Sep 2021 02:36:57 +0200
Subject: [PATCH] fix: Query string percent-encoded in import map (#11976)

This commit fixes a problem in import map when resolving
specifiers containing "?" or "#".

Due to special handling of Windows specifiers required because
of how "url" crate works, a regression was introduced that percent-encoded
all parts of URL that were not considered "path segments".

Co-authored-by: Andreu Botella <abb@randomunok.com>
---
 cli/import_map.rs | 63 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 61 insertions(+), 2 deletions(-)

diff --git a/cli/import_map.rs b/cli/import_map.rs
index 4e9feb0bef..31384ecef8 100644
--- a/cli/import_map.rs
+++ b/cli/import_map.rs
@@ -197,13 +197,29 @@ impl ImportMap {
     // the url since it contains what looks to be a uri scheme. To work around
     // this, we append the specifier to the path segments of the base url when
     // the specifier is not relative or absolute.
+    let mut maybe_query_string_and_fragment = None;
     if !is_relative_or_absolute_specifier && base.path_segments_mut().is_ok() {
       {
         let mut segments = base.path_segments_mut().unwrap();
         segments.pop_if_empty();
-        segments.extend(specifier.split('/'));
+
+        // Handle query-string and fragment first, otherwise they would be percent-encoded
+        // by `extend()`
+        let prefix = match specifier.find(&['?', '#'][..]) {
+          Some(idx) => {
+            maybe_query_string_and_fragment = Some(&specifier[idx..]);
+            &specifier[..idx]
+          }
+          None => specifier,
+        };
+        segments.extend(prefix.split('/'));
+      }
+
+      if let Some(query_string_and_fragment) = maybe_query_string_and_fragment {
+        Ok(base.join(query_string_and_fragment)?)
+      } else {
+        Ok(base)
       }
-      Ok(base)
     } else {
       Ok(base.join(specifier)?)
     }
@@ -805,4 +821,47 @@ mod tests {
       "http://localhost/C:/folder/file.ts"
     );
   }
+
+  #[test]
+  fn querystring() {
+    let json_map = r#"{
+      "imports": {
+        "npm/": "https://esm.sh/"
+      }
+    }"#;
+    let import_map =
+      ImportMap::from_json("https://deno.land", json_map).unwrap();
+
+    let resolved = import_map
+      .resolve("npm/postcss-modules@4.2.2?no-check", "https://deno.land")
+      .unwrap();
+    assert_eq!(
+      resolved.as_str(),
+      "https://esm.sh/postcss-modules@4.2.2?no-check"
+    );
+
+    let resolved = import_map
+      .resolve("npm/key/a?b?c", "https://deno.land")
+      .unwrap();
+    assert_eq!(resolved.as_str(), "https://esm.sh/key/a?b?c");
+
+    let resolved = import_map
+      .resolve(
+        "npm/postcss-modules@4.2.2?no-check#fragment",
+        "https://deno.land",
+      )
+      .unwrap();
+    assert_eq!(
+      resolved.as_str(),
+      "https://esm.sh/postcss-modules@4.2.2?no-check#fragment"
+    );
+
+    let resolved = import_map
+      .resolve("npm/postcss-modules@4.2.2#fragment", "https://deno.land")
+      .unwrap();
+    assert_eq!(
+      resolved.as_str(),
+      "https://esm.sh/postcss-modules@4.2.2#fragment"
+    );
+  }
 }