Skip to content

Commit

Permalink
fix: S3 copy error on non-ascii file path (apache#2909)
Browse files Browse the repository at this point in the history
* fix: S3 copy error on non-ascii file path (apache#2908)

* fix: S3 copy error on non-ascii file path, update copy test

* fix: S3 copy error on non-ascii file path, add non ascii test case

* fix: S3 copy error on non-ascii file path, add independent non ascii test case
  • Loading branch information
BoWuGit authored Aug 26, 2023
1 parent 60127ad commit 9179323
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 3 deletions.
1 change: 1 addition & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ OPENDAL_COS_SECRET_KEY=<secret_key>
OPENDAL_S3_TEST=false
OPENDAL_S3_BUCKET=<bucket>
OPENDAL_S3_ENDPOINT=<endpoint>
OPENDAL_S3_REGION=<region>
OPENDAL_S3_ACCESS_KEY_ID=<access_key_id>
OPENDAL_S3_SECRET_ACCESS_KEY=<secret_access_key>
# azblob
Expand Down
2 changes: 1 addition & 1 deletion core/src/services/s3/compatible_services.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ builder.enable_virtual_host_style();
To connect to minio, we need to set:

- `endpoint`: The endpoint of minio, for example: `http://127.0.0.1:9000`
- `region`: The region of minio. If not specified, it could be ignored.
- `region`: The region of minio. If you don't care about it, just set it to "auto", it will be ignored.
- `bucket`: The bucket name of minio.

```rust,ignore
Expand Down
2 changes: 1 addition & 1 deletion core/src/services/s3/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl S3Core {
}

let mut req = req
.header(constants::X_AMZ_COPY_SOURCE, percent_encode_path(&source))
.header(constants::X_AMZ_COPY_SOURCE, &source)
.body(AsyncBody::Empty)
.map_err(new_request_build_error)?;

Expand Down
2 changes: 1 addition & 1 deletion core/src/services/s3/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ async fn main() -> Result<()> {
builder.root("/path/to/dir");
// Set the bucket name. This is required.
builder.bucket("test");
// Set the region. This is only required for aws s3.
// Set the region. This is required for some services, if you don't care about it, for example Minio service, just set it to "auto", it will be ignored.
builder.region("us-east-1");
// Set the endpoint.
//
Expand Down
18 changes: 18 additions & 0 deletions core/tests/behavior/copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub fn behavior_copy_tests(op: &Operator) -> Vec<Trial> {
async_trials!(
op,
test_copy_file,
test_copy_file_with_non_ascii_name,
test_copy_non_existing_source,
test_copy_source_dir,
test_copy_target_dir,
Expand Down Expand Up @@ -57,6 +58,23 @@ pub async fn test_copy_file(op: Operator) -> Result<()> {
Ok(())
}

/// Copy a file and test with stat.
pub async fn test_copy_file_with_non_ascii_name(op: Operator) -> Result<()> {
let source_path = "🐂🍺中文.docx";
let target_path = "😈🐅Français.docx";
let (source_content, _) = gen_bytes();

op.write(source_path, source_content.clone()).await?;
op.copy(source_path, target_path).await?;

let target_content = op.read(target_path).await.expect("read must succeed");
assert_eq!(target_content, source_content);

op.delete(source_path).await.expect("delete must succeed");
op.delete(target_path).await.expect("delete must succeed");
Ok(())
}

/// Copy a nonexistent source should return an error.
pub async fn test_copy_non_existing_source(op: Operator) -> Result<()> {
let source_path = uuid::Uuid::new_v4().to_string();
Expand Down

0 comments on commit 9179323

Please sign in to comment.