Skip to content

fix(s3): set content length for copy object#7884

Open
ByteBaker wants to merge 1 commit into
apache:mainfrom
ByteBaker:main
Open

fix(s3): set content length for copy object#7884
ByteBaker wants to merge 1 commit into
apache:mainfrom
ByteBaker:main

Conversation

@ByteBaker

Copy link
Copy Markdown

Which issue does this PR close?

Closes #7821.

Rationale for this change

Some S3-compatible services require Content-Length to be present on CopyObject requests, even when the request body is empty. OpenDAL already sends an empty body for S3 copy, so this makes the request explicit by setting Content-Length: 0.

What changes are included in this PR?

  • Set Content-Length: 0 when building S3 CopyObject requests.
  • Split CopyObject request construction into s3_copy_object_request, matching existing request-builder patterns.
  • Add a unit test that verifies the CopyObject request method, headers, and empty body.

Are there any user-facing changes?

No API changes. This improves compatibility with S3-compatible services.

AI Usage Statement

I used OpenAI Codex to help implement and test this change.

@ByteBaker ByteBaker requested a review from Xuanwo as a code owner July 8, 2026 07:44
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. releases-note/fix The PR fixes a bug or has a title that begins with "fix" labels Jul 8, 2026
@dentiny

dentiny commented Jul 8, 2026

Copy link
Copy Markdown
Member

Just for my own curiosity, is there any doc mentioning netapp S3 gateway requires content-length header? I didn't find it after some seach

@Xuanwo Xuanwo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other changes LGTM


fn build(self) -> Result<impl Service> {
impl S3Builder {
fn build_backend(self) -> Result<S3Backend> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to test the constructor in a UT, but the construction was not separate, it was happening inside impl Builder, which returned Service, not S3Backend.

Doing this allowed me to test on S3Backend itself w/o duplicating much code.

@ByteBaker

Copy link
Copy Markdown
Author

Just for my own curiosity, is there any doc mentioning netapp S3 gateway requires content-length header? I didn't find it after some seach

@dentiny even I couldn't find one that explicitly makes it mandatory. However, I did find their error listing which has one MissingContentLength that makes me think they care about this. Other than this, not really.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

releases-note/fix The PR fixes a bug or has a title that begins with "fix" size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: operator.copy() not working in NetApp s3 server and ADLS with azdls

3 participants