You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by xu...@apache.org on 2023/03/26 10:23:18 UTC
[incubator-opendal] branch main updated: fix(oli): set the root of fs service to '/' (#1773)
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 9a5b7f83 fix(oli): set the root of fs service to '/' (#1773)
9a5b7f83 is described below
commit 9a5b7f839dafa08446e3644c275060b06718753b
Author: Jian Zeng <an...@gmail.com>
AuthorDate: Sun Mar 26 18:23:13 2023 +0800
fix(oli): set the root of fs service to '/' (#1773)
Signed-off-by: Jian Zeng <an...@gmail.com>
---
bin/oli/src/commands/cat.rs | 2 +-
bin/oli/src/commands/cp.rs | 4 +-
bin/oli/src/commands/ls.rs | 2 +-
bin/oli/src/config/mod.rs | 89 ++++++++++++++++++++++++++++++++++++++-------
4 files changed, 79 insertions(+), 18 deletions(-)
diff --git a/bin/oli/src/commands/cat.rs b/bin/oli/src/commands/cat.rs
index 6111a05d..bbe3d07a 100644
--- a/bin/oli/src/commands/cat.rs
+++ b/bin/oli/src/commands/cat.rs
@@ -34,7 +34,7 @@ pub async fn main(args: &ArgMatches) -> Result<()> {
.ok_or_else(|| anyhow!("missing target"))?;
let (op, path) = cfg.parse_location(target)?;
- let mut reader = op.reader(path).await?;
+ let mut reader = op.reader(&path).await?;
let mut stdout = io::stdout();
io::copy(&mut reader, &mut stdout).await?;
Ok(())
diff --git a/bin/oli/src/commands/cp.rs b/bin/oli/src/commands/cp.rs
index 996fbc59..c990a669 100644
--- a/bin/oli/src/commands/cp.rs
+++ b/bin/oli/src/commands/cp.rs
@@ -41,9 +41,9 @@ pub async fn main(args: &ArgMatches) -> Result<()> {
.ok_or_else(|| anyhow!("missing target"))?;
let (dst_op, dst_path) = cfg.parse_location(dst)?;
- let mut dst_w = dst_op.writer(dst_path).await?;
+ let mut dst_w = dst_op.writer(&dst_path).await?;
- let reader = src_op.reader(src_path).await?;
+ let reader = src_op.reader(&src_path).await?;
let buf_reader = futures::io::BufReader::with_capacity(8 * 1024 * 1024, reader);
futures::io::copy_buf(buf_reader, &mut dst_w).await?;
// flush data
diff --git a/bin/oli/src/commands/ls.rs b/bin/oli/src/commands/ls.rs
index 65bd80c6..323efaf4 100644
--- a/bin/oli/src/commands/ls.rs
+++ b/bin/oli/src/commands/ls.rs
@@ -39,7 +39,7 @@ pub async fn main(args: &ArgMatches) -> Result<()> {
let (op, path) = cfg.parse_location(target)?;
if !recursive {
- let mut ds = op.list(path).await?;
+ let mut ds = op.list(&path).await?;
while let Some(de) = ds.try_next().await? {
println!("{}", de.name());
}
diff --git a/bin/oli/src/config/mod.rs b/bin/oli/src/config/mod.rs
index 060a4b57..8cf0bcae 100644
--- a/bin/oli/src/config/mod.rs
+++ b/bin/oli/src/config/mod.rs
@@ -15,10 +15,11 @@
// specific language governing permissions and limitations
// under the License.
+use std::borrow::Cow;
use std::collections::HashMap;
use std::env;
use std::fs;
-use std::path::Path;
+use std::path::{Component, Path, PathBuf};
use std::str::FromStr;
use anyhow::anyhow;
@@ -34,6 +35,39 @@ pub struct Config {
profiles: HashMap<String, HashMap<String, String>>,
}
+/// resolve_relative_path turns a relative path to a absolute path.
+///
+/// The reason why we don't use `fs::canonicalize` here is `fs::canonicalize`
+/// will return an error if the path does not exist, which is unwanted.
+pub fn resolve_relative_path(path: &Path) -> Cow<Path> {
+ // NOTE: `path.is_absolute()` cannot handle cases like "/tmp/../a"
+ if path
+ .components()
+ .all(|e| e != Component::ParentDir && e != Component::CurDir)
+ {
+ // it is an absolute path
+ return path.into();
+ }
+
+ let root = Component::RootDir.as_os_str();
+ let mut result = env::current_dir().unwrap_or_else(|_| PathBuf::from(root));
+ for comp in path.components() {
+ match comp {
+ Component::ParentDir => {
+ if result.parent().is_none() {
+ continue;
+ }
+ result.pop();
+ }
+ Component::CurDir => (),
+ Component::RootDir | Component::Normal(_) | Component::Prefix(_) => {
+ result.push(comp.as_os_str());
+ }
+ }
+ }
+ result.into()
+}
+
impl Config {
/// Load profiles from both environment variables and local config file,
/// environment variables have higher precedence.
@@ -88,26 +122,28 @@ impl Config {
}
/// Parse `<profile>://abc/def` into `op` and `location`.
- pub fn parse_location<'a>(&self, s: &'a str) -> Result<(Operator, &'a str)> {
+ pub fn parse_location(&self, s: &str) -> Result<(Operator, String)> {
if !s.contains("://") {
- let mut fs = services::Fs::default();
+ 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();
- let filename = match s.rsplit_once(['/', '\\']) {
+ let filename = match fp_str.split_once(['/', '\\']) {
Some((base, filename)) => {
- fs.root(base);
+ fs_builder.root(if base.is_empty() { "/" } else { base });
filename
}
- None => s,
+ _ => s,
};
- return Ok((Operator::new(fs)?.finish(), filename));
+ 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];
+ let path = parts[1].into();
let profile = self
.profiles
@@ -209,7 +245,11 @@ impl Config {
Operator::from_map::<services::Webhdfs>(profile.clone())?.finish(),
path,
)),
- _ => Err(anyhow!("invalid profile")),
+ _ => Err(anyhow!(
+ "unknown type '{}' in profile '{}'",
+ scheme,
+ profile_name
+ )),
}
}
}
@@ -303,12 +343,33 @@ enable_virtual_host_style = "on"
#[test]
fn test_parse_fs_location() {
+ struct TestCase {
+ input: &'static str,
+ suffix: &'static str,
+ }
+ let test_cases = vec![
+ TestCase {
+ input: "./foo/1.txt",
+ suffix: "/foo/1.txt",
+ },
+ TestCase {
+ input: "/tmp/../1.txt",
+ suffix: "1.txt",
+ },
+ TestCase {
+ input: "/tmp/../../1.txt",
+ suffix: "1.txt",
+ },
+ ];
let cfg = Config::default();
- let (op, path) = cfg.parse_location("./foo/bar/1.txt").unwrap();
- assert_eq!("1.txt", path);
- let info = op.info();
- assert!(info.root().ends_with("foo/bar"));
- assert_eq!(Scheme::Fs, info.scheme());
+ for case in test_cases {
+ let (op, path) = cfg.parse_location(case.input).unwrap();
+ let info = op.info();
+ assert_eq!(Scheme::Fs, info.scheme());
+ assert_eq!("/", info.root());
+ assert!(!path.starts_with('.'));
+ assert!(path.ends_with(case.suffix));
+ }
}
#[test]