fix(plugins): location path decoding from Url (#2190)

* Add unit test for plugin run location parsing

* Fix file plugin parsing for relative paths

* Update test to check for path with spaces

* Add a couple more tests

Cargo.lock

@@ -4095,6 +4095,7 @@ dependencies = [
  "miette 3.3.0",
  "nix 0.23.1",
  "once_cell",
+ "percent-encoding",
  "regex",
  "rmp-serde",
  "serde",

zellij-utils/Cargo.toml

@@ -23,6 +23,7 @@ lazy_static = "1.4.0"
 libc = "0.2"
 nix = "0.23.1"
 once_cell = "1.8.0"
+percent-encoding = "2.1.0"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
 strip-ansi-escapes = "0.1.0"

zellij-utils/src/input/layout.rs

@@ -23,7 +23,6 @@ use std::str::FromStr;
 use super::plugins::{PluginTag, PluginsConfigError};
 use serde::{Deserialize, Serialize};
 use std::collections::BTreeMap;
-use std::convert::TryFrom;
 use std::vec::Vec;
 use std::{
     fmt,
@@ -221,6 +220,35 @@ pub enum RunPluginLocation {
     Zellij(PluginTag),
 }
 
+impl RunPluginLocation {
+    pub fn parse(location: &str) -> Result<Self, PluginsConfigError> {
+        let url = Url::parse(location)?;
+
+        let decoded_path = percent_encoding::percent_decode_str(url.path()).decode_utf8_lossy();
+
+        match url.scheme() {
+            "zellij" => Ok(Self::Zellij(PluginTag::new(decoded_path))),
+            "file" => {
+                let path = if location.starts_with("file:/") {
+                    // Path is absolute, its safe to use URL path.
+                    //
+                    // This is the case if the scheme and : delimiter are followed by a / slash
+                    decoded_path
+                } else {
+                    // URL dep doesn't handle relative paths with `file` schema properly,
+                    // it always makes them absolute. Use raw location string instead.
+                    //
+                    // Unwrap is safe here since location is a valid URL
+                    location.strip_prefix("file:").unwrap().into()
+                };
+
+                Ok(Self::File(PathBuf::from(path.as_ref())))
+            },
+            _ => Err(PluginsConfigError::InvalidUrlScheme(url)),
+        }
+    }
+}
+
 impl From<&RunPluginLocation> for Url {
     fn from(location: &RunPluginLocation) -> Self {
         let url = match location {
@@ -965,21 +993,6 @@ fn split_space(
     Ok(pane_positions)
 }
 
-impl TryFrom<Url> for RunPluginLocation {
-    type Error = PluginsConfigError;
-
-    fn try_from(url: Url) -> Result<Self, Self::Error> {
-        match url.scheme() {
-            "zellij" => Ok(Self::Zellij(PluginTag::new(url.path()))),
-            "file" => {
-                let path = PathBuf::from(url.path());
-                Ok(Self::File(path))
-            },
-            _ => Err(PluginsConfigError::InvalidUrl(url)),
-        }
-    }
-}
-
 impl Default for SplitDirection {
     fn default() -> Self {
         SplitDirection::Horizontal

zellij-utils/src/input/plugins.rs

@@ -225,8 +225,10 @@ impl Default for PluginType {
 pub enum PluginsConfigError {
     #[error("Duplication in plugin tag names is not allowed: '{}'", String::from(.0.clone()))]
     DuplicatePlugins(PluginTag),
+    #[error("Failed to parse url: {0:?}")]
+    InvalidUrl(#[from] url::ParseError),
     #[error("Only 'file:' and 'zellij:' url schemes are supported for plugin lookup. '{0}' does not match either.")]
-    InvalidUrl(Url),
+    InvalidUrlScheme(Url),
     #[error("Could not find plugin at the path: '{0:?}'")]
     InvalidPluginLocation(PathBuf),
 }

zellij-utils/src/input/unit/layout_test.rs

@@ -1942,3 +1942,91 @@ fn cannot_define_stacked_panes_with_grandchildren_in_pane_template() {
     let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into(), None, None);
     assert!(layout.is_err(), "error provided for tab name with space");
 }
+
+fn run_plugin_location_parsing() {
+    let kdl_layout = r#"
+        layout {
+            pane {
+                plugin location="zellij:tab-bar"
+            }
+            pane {
+                plugin location="file:/path/to/my/plugin.wasm"
+            }
+            pane {
+                plugin location="file:plugin.wasm"
+            }
+            pane {
+                plugin location="file:relative/with space/plugin.wasm"
+            }
+            pane {
+                plugin location="file:///absolute/with space/plugin.wasm"
+            }
+            pane {
+                plugin location="file:c:/absolute/windows/plugin.wasm"
+            }
+        }
+    "#;
+    let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into(), None, None).unwrap();
+    let expected_layout = Layout {
+        template: Some((
+            TiledPaneLayout {
+                children: vec![
+                    TiledPaneLayout {
+                        run: Some(Run::Plugin(RunPlugin {
+                            _allow_exec_host_cmd: false,
+                            location: RunPluginLocation::Zellij(PluginTag::new("tab-bar")),
+                        })),
+                        ..Default::default()
+                    },
+                    TiledPaneLayout {
+                        run: Some(Run::Plugin(RunPlugin {
+                            _allow_exec_host_cmd: false,
+                            location: RunPluginLocation::File(PathBuf::from(
+                                "/path/to/my/plugin.wasm",
+                            )),
+                        })),
+                        ..Default::default()
+                    },
+                    TiledPaneLayout {
+                        run: Some(Run::Plugin(RunPlugin {
+                            _allow_exec_host_cmd: false,
+                            location: RunPluginLocation::File(PathBuf::from("plugin.wasm")),
+                        })),
+                        ..Default::default()
+                    },
+                    TiledPaneLayout {
+                        run: Some(Run::Plugin(RunPlugin {
+                            _allow_exec_host_cmd: false,
+                            location: RunPluginLocation::File(PathBuf::from(
+                                "relative/with space/plugin.wasm",
+                            )),
+                        })),
+                        ..Default::default()
+                    },
+                    TiledPaneLayout {
+                        run: Some(Run::Plugin(RunPlugin {
+                            _allow_exec_host_cmd: false,
+                            location: RunPluginLocation::File(PathBuf::from(
+                                "/absolute/with space/plugin.wasm",
+                            )),
+                        })),
+                        ..Default::default()
+                    },
+                    TiledPaneLayout {
+                        run: Some(Run::Plugin(RunPlugin {
+                            _allow_exec_host_cmd: false,
+                            location: RunPluginLocation::File(PathBuf::from(
+                                "c:/absolute/windows/plugin.wasm",
+                            )),
+                        })),
+                        ..Default::default()
+                    },
+                ],
+                ..Default::default()
+            },
+            vec![],
+        )),
+        ..Default::default()
+    };
+    assert_eq!(layout, expected_layout);
+}

zellij-utils/src/kdl/kdl_layout_parser.rs

@@ -22,10 +22,8 @@ use crate::{
     kdl_string_arguments,
 };
 
-use std::convert::TryFrom;
 use std::path::PathBuf;
 use std::vec::Vec;
-use url::Url;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum PaneOrFloatingPane {
@@ -297,14 +295,13 @@ impl<'a> KdlLayoutParser<'a> {
                 plugin_block.span().len(),
             ),
         )?;
-        let url = Url::parse(string_url).map_err(|e| {
+        let location = RunPluginLocation::parse(&string_url).map_err(|e| {
             ConfigError::new_layout_kdl_error(
-                format!("Failed to parse url: {:?}", e),
+                e.to_string(),
                 url_node.span().offset(),
                 url_node.span().len(),
             )
         })?;
-        let location = RunPluginLocation::try_from(url)?;
         Ok(Some(Run::Plugin(RunPlugin {
             _allow_exec_host_cmd,
             location,

zellij-utils/src/kdl/mod.rs

@@ -13,7 +13,6 @@ use std::collections::HashMap;
 use std::fs::File;
 use std::io::Read;
 use strum::IntoEnumIterator;
-use url::Url;
 
 use miette::NamedSource;
 
@@ -1387,14 +1386,13 @@ impl RunPlugin {
                 kdl_node.span().len(),
             ),
         )?;
-        let url = Url::parse(string_url).map_err(|e| {
+        let location = RunPluginLocation::parse(string_url).map_err(|e| {
-            ConfigError::new_kdl_error(
+            ConfigError::new_layout_kdl_error(
-                format!("Failed to parse url: {:?}", e),
+                e.to_string(),
                 kdl_node.span().offset(),
                 kdl_node.span().len(),
             )
         })?;
-        let location = RunPluginLocation::try_from(url)?;
         Ok(RunPlugin {
             _allow_exec_host_cmd,
             location,