fix: copy/rename return error if source is nonexistent (#5528)

* copy() return error if `from` is nonexistent

* Check `from` in loop to avoid TOCTOU race

Author: dimbtp

object_store/src/lib.rs

@@ -2127,6 +2127,33 @@ mod tests {
         storage.delete(&path2).await.unwrap();
     }
 
+    pub(crate) async fn copy_rename_nonexistent_object(storage: &DynObjectStore) {
+        // Create empty source object
+        let path1 = Path::from("test1");
+
+        // Create destination object
+        let path2 = Path::from("test2");
+        storage.put(&path2, Bytes::from("hello")).await.unwrap();
+
+        // copy() errors if source does not exist
+        let result = storage.copy(&path1, &path2).await;
+        assert!(result.is_err());
+        assert!(matches!(result.unwrap_err(), crate::Error::NotFound { .. }));
+
+        // rename() errors if source does not exist
+        let result = storage.rename(&path1, &path2).await;
+        assert!(result.is_err());
+        assert!(matches!(result.unwrap_err(), crate::Error::NotFound { .. }));
+
+        // copy_if_not_exists() errors if source does not exist
+        let result = storage.copy_if_not_exists(&path1, &path2).await;
+        assert!(result.is_err());
+        assert!(matches!(result.unwrap_err(), crate::Error::NotFound { .. }));
+
+        // Clean up
+        storage.delete(&path2).await.unwrap();
+    }
+
     pub(crate) async fn multipart(storage: &dyn ObjectStore, multipart: &dyn MultipartStore) {
         let path = Path::from("test_multipart");
         let chunk_size = 5 * 1024 * 1024;

object_store/src/local.rs

@@ -598,7 +598,10 @@ impl ObjectStore for LocalFileSystem {
                 }
                 Err(source) => match source.kind() {
                     ErrorKind::AlreadyExists => id += 1,
-                    ErrorKind::NotFound => create_parent_dirs(&to, source)?,
+                    ErrorKind::NotFound => match from.exists() {
+                        true => create_parent_dirs(&to, source)?,
+                        false => return Err(Error::NotFound { path: from, source }.into()),
+                    },
                     _ => return Err(Error::UnableToCopyFile { from, to, source }.into()),
                 },
             }
@@ -613,7 +616,10 @@ impl ObjectStore for LocalFileSystem {
             match std::fs::rename(&from, &to) {
                 Ok(_) => return Ok(()),
                 Err(source) => match source.kind() {
-                    ErrorKind::NotFound => create_parent_dirs(&to, source)?,
+                    ErrorKind::NotFound => match from.exists() {
+                        true => create_parent_dirs(&to, source)?,
+                        false => return Err(Error::NotFound { path: from, source }.into()),
+                    },
                     _ => return Err(Error::UnableToCopyFile { from, to, source }.into()),
                 },
             }
@@ -636,7 +642,10 @@ impl ObjectStore for LocalFileSystem {
                         }
                         .into())
                     }
-                    ErrorKind::NotFound => create_parent_dirs(&to, source)?,
+                    ErrorKind::NotFound => match from.exists() {
+                        true => create_parent_dirs(&to, source)?,
+                        false => return Err(Error::NotFound { path: from, source }.into()),
+                    },
                     _ => return Err(Error::UnableToCopyFile { from, to, source }.into()),
                 },
             }
@@ -990,6 +999,7 @@ mod tests {
         list_with_delimiter(&integration).await;
         rename_and_copy(&integration).await;
         copy_if_not_exists(&integration).await;
+        copy_rename_nonexistent_object(&integration).await;
         stream_get(&integration).await;
         put_opts(&integration, false).await;
     }