Add source uri to headers for COPY request (#1228)

This changes is to address gap in supporting buckets with dots in the name for COPY requests. First encountered in s3-torch-connector awslabs/s3-connector-for-pytorch#295 ### Does this change impact existing behavior? No ### Does this change need a changelog entry? Yes --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the [Developer Certificate of Origin (DCO)](https://developercertificate.org/). --------- Signed-off-by: Ilya Isaev <[email protected]> Signed-off-by: Isaev Ilya <[email protected]> Co-authored-by: Ilya Isaev <[email protected]> Co-authored-by: Alessandro Passaro <[email protected]>
awslabs · Jan 16, 2025 · ace3093 · ace3093
1 parent d008177
commit ace3093
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 3 deletions.
diff --git a/mountpoint-s3-client/CHANGELOG.md b/mountpoint-s3-client/CHANGELOG.md
@@ -28,6 +28,8 @@
 
 ### Other changes
 
+* Add support for source buckets with dots in the name in `copy_object`.
+  ([#1228](https://github.com/awslabs/mountpoint-s3/pull/1228))
 * `HeadObjectResult` now includes the server-side encryption settings used when storing the object.
   ([#1143](https://github.com/awslabs/mountpoint-s3/pull/1143))
 * Add parameter to request checksum information as part of a `HeadObject` request.

diff --git a/mountpoint-s3-client/src/s3_crt_client/copy_object.rs b/mountpoint-s3-client/src/s3_crt_client/copy_object.rs
@@ -2,9 +2,10 @@ use std::ops::Deref;
 use std::os::unix::prelude::OsStrExt;
 
 use mountpoint_s3_crt::{http::request_response::Header, s3::client::MetaRequestResult};
+use tracing::trace;
 
 use crate::object_client::{CopyObjectError, CopyObjectParams, CopyObjectResult, ObjectClientResult};
-use crate::s3_crt_client::{S3CrtClient, S3Operation, S3RequestError};
+use crate::s3_crt_client::{S3CrtClient, S3CrtClientInner, S3Operation, S3RequestError};
 
 impl S3CrtClient {
     /// Create and begin a new CopyObject request.
@@ -40,8 +41,24 @@ impl S3CrtClient {
                 destination_key
             );
 
-            self.inner
-                .make_simple_http_request(message, S3Operation::CopyObject, span, parse_copy_object_error)?
+            let mut options = S3CrtClientInner::new_meta_request_options(message, S3Operation::CopyObject);
+            let uri = self
+                .inner
+                .endpoint_config
+                .resolve_for_bucket(source_bucket)
+                .map_err(S3RequestError::construction_failure)?
+                .uri()
+                .map_err(S3RequestError::construction_failure)?;
+            let source_uri = format!("{}/{source_key}", uri.as_os_str().to_string_lossy());
+            trace!(source_uri, "resolved source uri");
+            options.copy_source_uri(source_uri);
+            self.inner.make_simple_http_request_from_options(
+                options,
+                span,
+                |_| {},
+                parse_copy_object_error,
+                |_, _| (),
+            )?
         };
 
         let _body = request.await?;

diff --git a/mountpoint-s3-crt/src/s3/client.rs b/mountpoint-s3-crt/src/s3/client.rs
@@ -241,6 +241,9 @@ struct MetaRequestOptionsInner<'a> {
     /// Owned checksum config, if provided.
     checksum_config: Option<ChecksumConfig>,
 
+    /// Owned source uri for copy request, if provided.
+    copy_source_uri: Option<String>,
+
     /// Telemetry callback, if provided
     on_telemetry: Option<TelemetryCallback>,
 
@@ -320,6 +323,7 @@ impl<'a> MetaRequestOptions<'a> {
             endpoint: None,
             signing_config: None,
             checksum_config: None,
+            copy_source_uri: None,
             on_telemetry: None,
             on_headers: None,
             on_body: None,
@@ -462,6 +466,17 @@ impl<'a> MetaRequestOptions<'a> {
         options.inner.send_using_async_writes = send_using_async_writes;
         self
     }
+
+    /// Set the URI of source bucket/key for COPY request only
+    pub fn copy_source_uri(&mut self, source_uri: String) -> &mut Self {
+        // SAFETY: we aren't moving out of the struct.
+        let options = unsafe { Pin::get_unchecked_mut(Pin::as_mut(&mut self.0)) };
+        options.copy_source_uri = Some(source_uri);
+        // SAFETY: We ensure that the cursor points to data that lives
+        // as long as the options struct
+        options.inner.copy_source_uri = unsafe { options.copy_source_uri.as_mut().unwrap().as_aws_byte_cursor() };
+        self
+    }
 }
 
 impl Default for MetaRequestOptions<'_> {