[incubator-opendal] branch main updated: fix(oli): oli commands should not accept invalid URI format (#2845)

[xuanwo]

This is an automated email from the ASF dual-hosted git repository.

xuanwo pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-opendal.git


The following commit(s) were added to refs/heads/main by this push:
     new b76664117 fix(oli): oli commands should not accept invalid URI format 
(#2845)
b76664117 is described below

commit b7666411796a9699ffa2f6a8ff4cb2c7ac2f1b95
Author: Kousuke Saruta <saru...@apache.org>
AuthorDate: Thu Aug 10 22:28:39 2023 +0900

    fix(oli): oli commands should not accept invalid URI format (#2845)
    
    Disallow invalid uri format.
---
 bin/oli/src/config/mod.rs | 62 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 8 deletions(-)

diff --git a/bin/oli/src/config/mod.rs b/bin/oli/src/config/mod.rs
index adb39665a..40dae693d 100644
--- a/bin/oli/src/config/mod.rs
+++ b/bin/oli/src/config/mod.rs
@@ -31,6 +31,7 @@ use opendal::Operator;
 use opendal::Scheme;
 use serde::Deserialize;
 use toml;
+use url::Url;
 
 #[derive(Deserialize, Default)]
 pub struct Config {
@@ -125,7 +126,7 @@ impl Config {
 
     /// Parse `<profile>://abc/def` into `op` and `location`.
     pub fn parse_location(&self, s: &str) -> Result<(Operator, String)> {
-        if !s.contains("://") {
+        if !s.contains(":/") {
             let mut fs_builder = services::Fs::default();
             let fp = resolve_relative_path(Path::new(s));
             let fp_str = fp.as_os_str().to_string_lossy();
@@ -144,12 +145,13 @@ impl Config {
             return Ok((Operator::new(fs_builder)?.finish(), filename.into()));
         }
 
-        let parts = s.splitn(2, "://").collect::<Vec<_>>();
-        debug_assert!(parts.len() == 2);
-
-        let profile_name = parts[0];
-        let path = parts[1].into();
+        let location = Url::parse(s)?;
+        if location.has_host() {
+            Err(anyhow!("Host part in a location is not supported."))?;
+        }
 
+        let profile_name = location.scheme();
+        let path = location.path().to_string();
         let profile = self
             .profiles
             .get(profile_name)
@@ -400,10 +402,54 @@ enable_virtual_host_style = "on"
                 ]),
             )]),
         };
-        let (op, path) = cfg.parse_location("mys3://foo/1.txt").unwrap();
-        assert_eq!("foo/1.txt", path);
+        let (op, path) = cfg.parse_location("mys3:///foo/1.txt").unwrap();
+        assert_eq!("/foo/1.txt", path);
+        let info = op.info();
+        assert_eq!(Scheme::S3, info.scheme());
+        assert_eq!("mybucket", info.name());
+    }
+
+    #[test]
+    fn test_parse_s3_location2() {
+        let cfg = Config {
+            profiles: HashMap::from([(
+                "mys3".into(),
+                HashMap::from([
+                    ("type".into(), "s3".into()),
+                    ("bucket".into(), "mybucket".into()),
+                    ("region".into(), "us-east-1".into()),
+                ]),
+            )]),
+        };
+        let (op, path) = cfg.parse_location("mys3:/foo/1.txt").unwrap();
+        assert_eq!("/foo/1.txt", path);
         let info = op.info();
         assert_eq!(Scheme::S3, info.scheme());
         assert_eq!("mybucket", info.name());
     }
+
+    #[test]
+    fn test_parse_s3_location3() -> Result<()> {
+        let cfg = Config {
+            profiles: HashMap::from([(
+                "mys3".into(),
+                HashMap::from([
+                    ("type".into(), "s3".into()),
+                    ("bucket".into(), "mybucket".into()),
+                    ("region".into(), "us-east-1".into()),
+                ]),
+            )]),
+        };
+
+        let uri = "mys3://foo/1.txt";
+        let expected_msg = "Host part in a location is not supported.";
+        match cfg.parse_location(uri) {
+            Err(e) if e.to_string() == expected_msg => Ok(()),
+            _ => Err(anyhow!(
+                "Getting an message \"{}\" is expected when parsing {}.",
+                expected_msg,
+                uri
+            ))?,
+        }
+    }
 }