Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Ensure pid and root path are canonicalized #851

Merged
merged 1 commit into from
Apr 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 43 additions & 23 deletions crates/libcontainer/src/container/builder.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::syscall::Syscall;
use crate::{syscall::Syscall, utils::PathBufExt};
use anyhow::{Context, Result};
use std::path::PathBuf;

Expand Down Expand Up @@ -108,8 +108,9 @@ impl<'a> ContainerBuilder<'a> {
pub fn with_root_path<P: Into<PathBuf>>(mut self, path: P) -> Result<Self> {
let path = path.into();
self.root_path = path
.canonicalize()
.with_context(|| format!("failed to canonicalize root path {:?}", path))?;
.canonicalize_safely()
.with_context(|| format!("failed to canonicalize root path {path:?}"))?;

Ok(self)
}

Expand All @@ -125,15 +126,17 @@ impl<'a> ContainerBuilder<'a> {
/// .with_pid_file(Some("/var/run/docker.pid")).expect("invalid pid file");
/// ```
pub fn with_pid_file<P: Into<PathBuf>>(mut self, path: Option<P>) -> Result<Self> {
self.pid_file = if let Some(p) = path {
let path = p.into();
Some(
path.canonicalize()
.with_context(|| format!("failed to canonicalize pid file path {:?}", path))?,
)
} else {
None
self.pid_file = match path {
Some(path) => {
let p = path.into();
Some(
p.canonicalize_safely()
.with_context(|| format!("failed to canonicalize pid file {p:?}"))?,
)
}
None => None,
};

Ok(self)
}

Expand Down Expand Up @@ -179,7 +182,7 @@ mod tests {
use std::path::PathBuf;

#[test]
fn test_builder_failable_functions() -> Result<()> {
fn test_failable_functions() -> Result<()> {
let root_path_temp_dir = TempDir::new("root_path").context("failed to create temp dir")?;
let pid_file_temp_dir = TempDir::new("pid_file").context("failed to create temp dir")?;
let syscall = create_syscall();
Expand All @@ -190,20 +193,37 @@ mod tests {
.with_console_socket(Some("/var/run/docker/sock.tty"))
.as_init("/var/run/docker/bundle");

// Accept None pid file.
// accept None pid file.
ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref())
.with_pid_file::<PathBuf>(None)?;

let invalid_path_builder =
ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref())
.with_root_path("/not/existing/path");
assert!(invalid_path_builder.is_err());

let invalid_pid_file_builder =
ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref())
.with_root_path(root_path_temp_dir.path())?
.with_pid_file(Some("/not/existing/path"));
assert!(invalid_pid_file_builder.is_err());
// accept absolute root path which does not exist
let abs_root_path = PathBuf::from("/not/existing/path");
let path_builder = ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref())
.with_root_path(&abs_root_path)
.context("build container")?;
assert_eq!(path_builder.root_path, abs_root_path);

// accept relative root path which does not exist
let cwd = std::env::current_dir().context("get current dir")?;
let path_builder = ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref())
.with_root_path("./not/existing/path")
.context("build container")?;
assert_eq!(path_builder.root_path, cwd.join("not/existing/path"));

// accept absolute pid path which does not exist
let abs_pid_path = PathBuf::from("/not/existing/path");
let path_builder = ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref())
.with_pid_file(Some(&abs_pid_path))
.context("build container")?;
assert_eq!(path_builder.pid_file, Some(abs_pid_path));

// accept relative pid path which does not exist
let cwd = std::env::current_dir().context("get current dir")?;
let path_builder = ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref())
.with_pid_file(Some("./not/existing/path"))
.context("build container")?;
assert_eq!(path_builder.pid_file, Some(cwd.join("not/existing/path")));

Ok(())
}
Expand Down
50 changes: 49 additions & 1 deletion crates/libcontainer/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ use std::ops::Deref;
use std::os::linux::fs::MetadataExt;
use std::os::unix::fs::DirBuilderExt;
use std::os::unix::prelude::{AsRawFd, OsStrExt};
use std::path::{Path, PathBuf};
use std::path::{Component, Path, PathBuf};

pub trait PathBufExt {
fn as_relative(&self) -> Result<&Path>;
fn join_safely<P: AsRef<Path>>(&self, p: P) -> Result<PathBuf>;
fn canonicalize_safely(&self) -> Result<PathBuf>;
fn normalize(&self) -> PathBuf;
}

impl PathBufExt for Path {
Expand All @@ -42,6 +44,52 @@ impl PathBufExt for Path {
.with_context(|| format!("failed to strip prefix from {}", path.display()))?;
Ok(self.join(stripped))
}

/// Canonicalizes existing and not existing paths
fn canonicalize_safely(&self) -> Result<PathBuf> {
if self.exists() {
self.canonicalize()
.with_context(|| format!("failed to canonicalize path {:?}", self))
} else {
if self.is_relative() {
let p = std::env::current_dir()
.context("could not get current directory")?
.join(self);
return Ok(p.normalize());
}

Ok(self.normalize())
}
}

/// Normalizes a path. In contrast to canonicalize the path does not need to exist.
// adapted from https://github.com/rust-lang/cargo/blob/fede83ccf973457de319ba6fa0e36ead454d2e20/src/cargo/util/paths.rs#L61
fn normalize(&self) -> PathBuf {
let mut components = self.components().peekable();
let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() {
components.next();
PathBuf::from(c.as_os_str())
} else {
PathBuf::new()
};

for component in components {
match component {
Component::Prefix(..) => unreachable!(),
Component::RootDir => {
ret.push(component.as_os_str());
}
Component::CurDir => {}
Component::ParentDir => {
ret.pop();
}
Component::Normal(c) => {
ret.push(c);
}
}
}
ret
}
}

pub fn parse_env(envs: &[String]) -> HashMap<String, String> {
Expand Down