Make artifact path matching match the official runner

gitlab-runner/Cargo.toml

@@ -36,6 +36,7 @@ hmac = "0.13.0"
 rand = "0.10.1"
 tokio-util = { version = "0.7.18", features = [ "io" ] }
 tokio-retry2 = { version = "0.9.1", features = ["jitter"] }
+normalize-path = "0.2.1"
 
 [dev-dependencies]
 tokio = { version = "1.50.0", features = [ "full", "test-util" ] }

gitlab-runner/src/run.rs

@@ -1,4 +1,5 @@
 use bytes::Bytes;
+use normalize_path::NormalizePath;
 use std::future::Future;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
@@ -85,6 +86,48 @@ where
     overall_result
 }
 
+fn artifact_path_matches<I: IntoIterator>(file_path: &Path, patterns: I) -> bool
+where
+    I::Item: AsRef<str>,
+{
+    patterns.into_iter().any(|test_path| {
+        let test_path = Path::new(test_path.as_ref());
+        let Some(test_path) = test_path.try_normalize() else {
+            return false;
+        };
+        match glob::Pattern::new(&test_path.to_string_lossy()) {
+            Ok(pattern) => file_path.ancestors().any(|p| {
+                pattern.matches_path_with(
+                    p,
+                    glob::MatchOptions {
+                        require_literal_separator: true,
+                        ..glob::MatchOptions::new()
+                    },
+                )
+            }),
+            Err(_) => file_path.ancestors().any(|p| p == test_path),
+        }
+    })
+}
+
+#[test]
+fn test_artifact_path_matches() {
+    assert!(artifact_path_matches(Path::new("abc"), &["abc"]));
+    assert!(artifact_path_matches(Path::new("abc"), &["a*c"]));
+    assert!(artifact_path_matches(Path::new("abc/d"), &["abc"]));
+    assert!(artifact_path_matches(Path::new("abc/d"), &["a*c"]));
+    assert!(!artifact_path_matches(Path::new("ab/c"), &["a*c"]));
+    assert!(artifact_path_matches(Path::new("abc"), &["."]));
+    assert!(artifact_path_matches(Path::new("abc"), &["./abc//"]));
+    assert!(artifact_path_matches(Path::new("a/b/c"), &["a/**/c"]));
+    assert!(artifact_path_matches(Path::new("a/b/d/e/f/c"), &["a/**/c"]));
+    assert!(artifact_path_matches(
+        Path::new("a/b/c.yaml"),
+        &["**/*.yaml"]
+    ));
+    assert!(artifact_path_matches(Path::new("["), &["["]));
+}
+
 async fn process_artifact<J, U>(
     artifact: &JobArtifact,
     script_result: JobResult,
@@ -124,16 +167,13 @@ where
     let mut uploaded = 0;
 
     for file in handler.get_uploadable_files().await? {
-        if artifact
-            .paths
-            .iter()
-            .any(|path| match glob::Pattern::new(path) {
-                Ok(pattern) => pattern.matches(&file.get_path()),
-                Err(_) => path == &file.get_path(),
-            })
-        {
-            let path = file.get_path();
-            match uploader.file(path.to_string()).await {
+        let file_path = Path::new(&file.get_path().as_ref()).normalize();
+
+        if artifact_path_matches(&file_path, &artifact.paths) {
+            match uploader
+                .file(file_path.to_string_lossy().into_owned())
+                .await
+            {
                 Ok(mut upload) => {
                     let mut data = file.get_data().await?;
                     match futures::io::copy(&mut data, &mut upload).await {

gitlab-runner/tests/artifacts.rs

@@ -17,6 +17,7 @@ use gitlab_runner_mock::{
 enum TestFile {
     Test,
     Test2,
+    TestDir,
 }
 
 #[async_trait::async_trait]
@@ -26,13 +27,15 @@ impl UploadableFile for TestFile {
     fn get_path(&self) -> Cow<'_, str> {
         match self {
             TestFile::Test => "test".into(),
-            TestFile::Test2 => "test2".into(),
+            TestFile::Test2 => "./test2".into(),
+            TestFile::TestDir => "dir/test".into(),
         }
     }
     async fn get_data(&self) -> Result<&'_ [u8], ()> {
         Ok(match self {
             TestFile::Test => b"testdata".as_slice(),
             TestFile::Test2 => b"testdata2".as_slice(),
+            TestFile::TestDir => b"testdata-dir".as_slice(),
         })
     }
 }
@@ -48,7 +51,9 @@ impl JobHandler<TestFile> for Upload {
     async fn get_uploadable_files(
         &mut self,
     ) -> Result<Box<dyn Iterator<Item = TestFile> + Send>, ()> {
-        Ok(Box::new([TestFile::Test, TestFile::Test2].into_iter()))
+        Ok(Box::new(
+            [TestFile::Test, TestFile::Test2, TestFile::TestDir].into_iter(),
+        ))
     }
 }
 
@@ -65,22 +70,20 @@ impl Download {
                 .expect("No artifacts to download");
             let mut names: Vec<_> = artifact.file_names().collect();
             names.sort_unstable();
-            assert_eq!(2, names.len());
-            assert_eq!(&["test", "test2"], names.as_slice());
-
-            let mut f = artifact.file("test").expect("Testfile not available");
-            assert_eq!(f.name(), "test");
-            let mut data = Vec::new();
-            f.read_to_end(&mut data).expect("Failed to read content");
-            assert_eq!("testdata".as_bytes(), &*data);
-            drop(f);
-
-            let mut f = artifact.file("test2").expect("Testfile not available");
-            assert_eq!(f.name(), "test2");
-            let mut data = Vec::new();
-            f.read_to_end(&mut data).expect("Failed to read content");
-            assert_eq!("testdata2".as_bytes(), &*data);
-            drop(f);
+            assert_eq!(3, names.len());
+            assert_eq!(&["dir/test", "test", "test2"], names.as_slice());
+
+            for (name, expected) in [
+                ("test", "testdata"),
+                ("test2", "testdata2"),
+                ("dir/test", "testdata-dir"),
+            ] {
+                let mut f = artifact.file(name).expect("Testfile not available");
+                assert_eq!(f.name(), name);
+                let mut data = Vec::new();
+                f.read_to_end(&mut data).expect("Failed to read content");
+                assert_eq!(expected.as_bytes(), &*data);
+            }
 
             /* Try reading first data file a second time */
             let mut f = artifact.file("test").expect("Testfile not available");
@@ -113,7 +116,11 @@ async fn upload_download() {
         MockJobStepWhen::OnSuccess,
         false,
     );
-    upload.add_artifact_paths(vec!["*".to_string()]);
+    upload.add_artifact_paths(vec![
+        "dir".to_string(),
+        "./*2".to_string(),
+        "test/".to_string(),
+    ]);
     let upload = upload.build();
     mock.enqueue_job(upload.clone());
 
@@ -236,7 +243,7 @@ async fn multiple_upload() {
                     let mut z = ZipArchive::new(std::io::Cursor::new(artifact.data.as_slice()))
                         .expect("failed to open zip archive");
                     saw_zip = true;
-                    assert_eq!(z.len(), 2);
+                    assert_eq!(z.len(), 3);
                     for ix in 0..z.len() {
                         let mut f = z.by_index(ix).expect("failed to get zip file");
                         match f.name() {
@@ -250,6 +257,11 @@ async fn multiple_upload() {
                                 f.read_to_end(&mut data).expect("failed to read zip file");
                                 assert_eq!(b"testdata2", data.as_slice());
                             }
+                            "dir/test" => {
+                                let mut data = Vec::new();
+                                f.read_to_end(&mut data).expect("failed to read zip file");
+                                assert_eq!(b"testdata-dir", data.as_slice());
+                            }
                             _ => {
                                 unreachable!("unknown file in archive");
                             }