fix: redefine opaque pathname detection for custom protocols (#61) (#62)

Author: hirochachacha

src/canonicalize_and_process.rs

@@ -211,11 +211,24 @@ pub fn process_pathname_init(
   if kind == &ProcessType::Pattern {
     Ok(pathname_value.to_string())
   } else {
-    match protocol_value {
+    // A path is non-opaque if:
+    // 1. The protocol is empty, OR
+    // 2. The protocol is a special scheme (http, https, etc.), OR
+    // 3. The pathname has a leading '/' (indicating hierarchical path)
+    let is_non_opaque = match protocol_value {
       Some(protocol) if protocol.is_empty() || is_special_scheme(protocol) => {
-        canonicalize_pathname(pathname_value)
+        true
       }
-      _ => canonicalize_an_opaque_pathname(pathname_value),
+      _ => {
+        // For non-special schemes, treat as non-opaque if pathname starts with '/'
+        pathname_value.starts_with('/')
+      }
+    };
+
+    if is_non_opaque {
+      canonicalize_pathname(pathname_value)
+    } else {
+      canonicalize_an_opaque_pathname(pathname_value)
     }
   }
 }

src/lib.rs

@@ -371,23 +371,38 @@ impl<R: RegExp> UrlPattern<R> {
       ..Default::default()
     };
 
-    let pathname = if protocol.protocol_component_matches_special_scheme() {
-      Component::compile(
-        processed_init.pathname.as_deref(),
-        canonicalize_and_process::canonicalize_pathname,
-        parser::Options {
-          ignore_case: options.ignore_case,
-          ..parser::Options::pathname()
-        },
-      )?
-      .optionally_transpose_regex_error(report_regex_errors)?
-    } else {
-      Component::compile(
-        processed_init.pathname.as_deref(),
-        canonicalize_and_process::canonicalize_an_opaque_pathname,
-        compile_options.clone(),
-      )?
-      .optionally_transpose_regex_error(report_regex_errors)?
+    let pathname = {
+      // Determine if path is non-opaque using the same criteria as process_pathname_init
+      let protocol_is_empty = processed_init
+        .protocol
+        .as_ref()
+        .is_some_and(|p| p.is_empty());
+      let has_leading_slash = processed_init
+        .pathname
+        .as_ref()
+        .is_some_and(|p| p.starts_with('/'));
+      let is_non_opaque = protocol_is_empty
+        || protocol.protocol_component_matches_special_scheme()
+        || has_leading_slash;
+
+      if is_non_opaque {
+        Component::compile(
+          processed_init.pathname.as_deref(),
+          canonicalize_and_process::canonicalize_pathname,
+          parser::Options {
+            ignore_case: options.ignore_case,
+            ..parser::Options::pathname()
+          },
+        )?
+        .optionally_transpose_regex_error(report_regex_errors)?
+      } else {
+        Component::compile(
+          processed_init.pathname.as_deref(),
+          canonicalize_and_process::canonicalize_an_opaque_pathname,
+          compile_options.clone(),
+        )?
+        .optionally_transpose_regex_error(report_regex_errors)?
+      }
     };
 
     Ok(UrlPattern {
@@ -1047,4 +1062,46 @@ mod tests {
     .unwrap();
     assert!(pattern.has_regexp_groups());
   }
+
+  #[test]
+  fn issue61() {
+    // Test case for https://github.com/denoland/deno/issues/29935
+    // Custom protocols should not escape colons and slashes in pattern pathnames
+
+    // Test using init with pattern components
+    let pattern = <UrlPattern>::parse(
+      UrlPatternInit {
+        protocol: Some("myhttp".to_string()),
+        hostname: Some("example.com".to_string()),
+        pathname: Some("/:directory/:file".to_string()),
+        ..Default::default()
+      },
+      Default::default(),
+    )
+    .unwrap();
+
+    println!("Pattern: {pattern:?}");
+    println!("Protocol: {}", pattern.protocol());
+    println!("Hostname: {}", pattern.hostname());
+    println!("Pathname: {}", pattern.pathname());
+
+    // The pathname should be "/:directory/:file", not "%2F:directory%2F:file"
+    assert_eq!(pattern.pathname().to_string(), "/:directory/:file");
+
+    // Also test myfile:///test case - empty hostname with leading slash
+    let myfile_pattern = <UrlPattern>::parse(
+      UrlPatternInit {
+        protocol: Some("myfile".to_string()),
+        hostname: Some("".to_string()), // empty hostname
+        pathname: Some("/test".to_string()),
+        ..Default::default()
+      },
+      Default::default(),
+    )
+    .unwrap();
+
+    println!("\nMyfile pattern pathname: {}", myfile_pattern.pathname());
+    // Should use non-opaque canonicalization because of leading slash
+    assert_eq!(myfile_pattern.pathname().to_string(), "/test");
+  }
 }