Add runtime check in ArtifactPath::new

This adds a runtime check whether the artifact path is indeed relative.
An absolute artifact path is a bug.

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>

3 files changed, 22 insertions, 16 deletions

diff --git a/src/filestore/artifact.rs b/src/filestore/artifact.rs
index d97aceb..3b9f2e0 100644
--- a/src/filestore/artifact.rs
+++ b/src/filestore/artifact.rs
@@ -85,7 +85,7 @@ mod tests {
 
     #[test]
     fn test_parser_one_letter_name() {
-        let p = ArtifactPath::new(PathBuf::from("a-1.ext"));
+        let p = ArtifactPath::new(PathBuf::from("a-1.ext")).unwrap();
         let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
@@ -98,7 +98,7 @@ mod tests {
 
     #[test]
     fn test_parser_multi_letter_name() {
-        let p = ArtifactPath::new(PathBuf::from("foo-1.ext"));
+        let p = ArtifactPath::new(PathBuf::from("foo-1.ext")).unwrap();
         let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
@@ -111,7 +111,7 @@ mod tests {
 
     #[test]
     fn test_parser_multi_char_version() {
-        let p = ArtifactPath::new(PathBuf::from("foo-1123.ext"));
+        let p = ArtifactPath::new(PathBuf::from("foo-1123.ext")).unwrap();
         let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
@@ -124,7 +124,7 @@ mod tests {
 
     #[test]
     fn test_parser_multi_char_version_dashed() {
-        let p = ArtifactPath::new(PathBuf::from("foo-1-1-2-3.ext"));
+        let p = ArtifactPath::new(PathBuf::from("foo-1-1-2-3.ext")).unwrap();
         let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
@@ -137,7 +137,7 @@ mod tests {
 
     #[test]
     fn test_parser_multi_char_version_dashed_and_dotted() {
-        let p = ArtifactPath::new(PathBuf::from("foo-1-1.2-3.ext"));
+        let p = ArtifactPath::new(PathBuf::from("foo-1-1.2-3.ext")).unwrap();
         let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
@@ -150,7 +150,7 @@ mod tests {
 
     #[test]
     fn test_parser_alnum_version() {
-        let p = ArtifactPath::new(PathBuf::from("foo-1-1.2a3.ext"));
+        let p = ArtifactPath::new(PathBuf::from("foo-1-1.2a3.ext")).unwrap();
         let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
@@ -163,7 +163,7 @@ mod tests {
 
     #[test]
     fn test_parser_package_name_with_number() {
-        let p = ArtifactPath::new(PathBuf::from("foo2-1-1.2a3.ext"));
+        let p = ArtifactPath::new(PathBuf::from("foo2-1-1.2a3.ext")).unwrap();
         let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
diff --git a/src/filestore/path.rs b/src/filestore/path.rs
index fd79e1b..9a3e193 100644
--- a/src/filestore/path.rs
+++ b/src/filestore/path.rs
@@ -17,8 +17,8 @@ impl StoreRoot {
 
     pub (in crate::filestore) fn stripped_from(&self, pb: &Path) -> Result<ArtifactPath> {
         pb.strip_prefix(&self.0)
-            .map(|p| ArtifactPath::new(p.to_path_buf()))
             .map_err(Error::from)
+            .and_then(|p| ArtifactPath::new(p.to_path_buf()))
     }
 
     pub fn join<'a>(&'a self, ap: &'a ArtifactPath) -> FullArtifactPath<'a> {
@@ -49,8 +49,12 @@ impl AsRef<Path> for StoreRoot {
 pub struct ArtifactPath(PathBuf);
 
 impl ArtifactPath {
-    pub (in crate::filestore) fn new(p: PathBuf) -> Self {
-        ArtifactPath(p)
+    pub (in crate::filestore) fn new(p: PathBuf) -> Result<Self> {
+        if p.is_relative() {
+            Ok(ArtifactPath(p))
+        } else {
+            Err(anyhow!("Path is not relative: {}", p.display()))
+        }
     }
 
     pub fn display(&self) -> std::path::Display {
diff --git a/src/filestore/staging.rs b/src/filestore/staging.rs
index df7e006..93c4047 100644
--- a/src/filestore/staging.rs
+++ b/src/filestore/staging.rs
@@ -70,12 +70,14 @@ impl StagingStore {
                     None
                 } else {
                     Some({
-                        let path = ArtifactPath::new(path);
-                        self.0.load_from_path(&path)
-                            .inspect(|r| trace!("Loaded from path {} = {:?}", path.display(), r))
-                            .with_context(|| anyhow!("Loading from path: {}", path.display()))
-                            .map_err(Error::from)
-                            .map(|art| art.path().clone())
+                        ArtifactPath::new(path)
+                            .and_then(|ap| {
+                                self.0.load_from_path(&ap)
+                                    .inspect(|r| trace!("Loaded from path {} = {:?}", ap.display(), r))
+                                    .with_context(|| anyhow!("Loading from path: {}", ap.display()))
+                                    .map_err(Error::from)
+                                    .map(|art| art.path().clone())
+                            })
                     })
                 }
             })