This is an automated email from the ASF dual-hosted git repository.
tustvold pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/master by this push:
new f5c2686296f fix: copy/rename return error if source is nonexistent
(#5528)
f5c2686296f is described below
commit f5c2686296f3f0af1f4072f329f72aa19ac56d08
Author: dimbtp <[email protected]>
AuthorDate: Wed Mar 20 14:00:31 2024 +0800
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
---
object_store/src/lib.rs | 27 +++++++++++++++++++++++++++
object_store/src/local.rs | 16 +++++++++++++---
2 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/object_store/src/lib.rs b/object_store/src/lib.rs
index e02675d88ab..97604a7dce6 100644
--- a/object_store/src/lib.rs
+++ b/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;
diff --git a/object_store/src/local.rs b/object_store/src/local.rs
index a7eb4661f68..6cc0c672af4 100644
--- a/object_store/src/local.rs
+++ b/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;
}
