fix(s3): set content length for copy object by ByteBaker · Pull Request #7884 · apache/opendal

ByteBaker · 2026-07-08T07:44:36Z

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.

dentiny · 2026-07-08T09:31:38Z

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

Other changes LGTM

Xuanwo · 2026-07-08T09:47:46Z

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


Why change this?

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.

Good thought! Try build S3Core instead of building a S3Backend. An arbitrary example:

fn new_test_core() -> CompfsCore { CompfsCore { info: ServiceInfo::new("compfs", "", ""), capability: Capability::default(), root: PathBuf::from("/data/root"), dispatcher: Dispatcher::new().unwrap(), buf_pool: oio::PooledBuf::new(16), } }

I'm fine to just remove this UT.

ByteBaker · 2026-07-08T15:18:43Z

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.

dentiny · 2026-07-09T14:48:36Z

However, I did find their error listing which has one MissingContentLength that makes me think they care about this.

I'm not sure this is the best way to handle. :thinking_face:

I don't see content-length required from S3 doc: https://docs.aws.amazon.com/AmazonS3/latest/API/API_CopyObject.html#API_CopyObject_RequestSyntax
Assuming it's netapp ONTAP-9, I don't see content-length required for any operation: https://docs.netapp.com/us-en/ontap/pdfs/fullsite-sidebar/ONTAP_9_documentation.pdf
For the missing content-length, I'm not sure whether the server side is expecting 0 or a real object size
- A missing-length error is not convincing to me
It's not open source, it's not well-documented (at least I didn't find it), I don't find docker image, I'm wondering how can we verify we're doing the correct thing?

I'm actually thinking if we could allow users to inject their customized headers

I hope opendal-core to implement only S3 spec, but not un-documented behavior from all types of vendors
- For example, what if another S3-compatible vendor expecting content-length to be object size?
It's legal and natural for different S3-compatible impls could accept their own headers
- I'm working on an internal object storage for my current job, we discussed a bit on exposing some headers to better control server-side caching policy for performance, and passing checksum for validation, etc

erickguan · 2026-07-09T15:24:02Z

@dentiny Good point and I agree with you S3 is THE reference implementation. Though I wouldn't consider following S3 to bits is beneficial to us since there are lots of S3-compatible services. We test these S3-compatible services. Nevertheless, you made a valid point. We should not extend our support to all S3-compatible services. How about we track these customized behavior in an issue/discussion? We could either explore some irregular behaviors from various of vendors or set a way for the fix.

Meanwhile, we can go ahead with this PR at least.

erickguan · 2026-07-09T15:26:29Z

+            // AWS S3 doesn't care, but some other S3-compat providers
+            // require a mandatory `content-length` header to work


Suggested change

// AWS S3 doesn't care, but some other S3-compat providers

// require a mandatory `content-length` header to work

// AWS S3 doesn't define `Content-Length` in CopyObject. S3 allows `Content-Length: 0` silently.

// Some providers require a unspecific `Content-Length`. We allow e.g.,:

// - `NetApp`

dentiny · 2026-07-09T20:33:18Z

Thanks! I create a feature request here to track.

we can go ahead with this PR at least.

Sure, but I do have a few practical questions:

I'm wondering do we have any concrete conclusion that the netapp S3 is expecting a header of "Content-Length: 0" (like a behavior test?), or it's expecting users to provide actual object size? Since it's undocumented and untested, I have no clue whether this PR will address the original issue
It's completely unknown whether there's a vendor who take advantage of the Content-Length header for indicate the number of bytes to copy (I won't be surprised if any vendors does so); that way we're introducing new failures -- that's the conflict behavior across vendors I mentioned on the issue
If go with the custom header solution -- at least you seem to agree on it 😉 -- how do we plan to deal with the content-length header in this PR? If we plan to delete it, it's a behavior change... At minimum we should leave a TODO

erickguan · 2026-07-10T00:17:29Z

@ByteBaker @sudharsansrini do you have a NetApp environment to run behavior tests upon?

ByteBaker · 2026-07-10T04:34:59Z

Not me, sorry.

fix(s3): set content length for copy object

20696b9

ByteBaker requested a review from Xuanwo as a code owner July 8, 2026 07:44

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

Xuanwo reviewed Jul 8, 2026

View reviewed changes

erickguan reviewed Jul 9, 2026

View reviewed changes

dentiny mentioned this pull request Jul 9, 2026

new feature: allow users to inject customized HTTP header #7894

Open

1 task

		// AWS S3 doesn't care, but some other S3-compat providers
		// require a mandatory `content-length` header to work

-            // AWS S3 doesn't care, but some other S3-compat providers
-            // require a mandatory `content-length` header to work
+            // AWS S3 doesn't define `Content-Length` in CopyObject. S3 allows `Content-Length: 0` silently.
+            // Some providers require a unspecific `Content-Length`. We allow e.g.,:
+            // - `NetApp`

Uh oh!

Conversation

ByteBaker commented Jul 8, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

AI Usage Statement

Uh oh!

dentiny commented Jul 8, 2026

Uh oh!

Xuanwo left a comment

Choose a reason for hiding this comment

Uh oh!

Xuanwo Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

ByteBaker Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

erickguan Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Xuanwo Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

ByteBaker commented Jul 8, 2026

Uh oh!

dentiny commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erickguan commented Jul 9, 2026

Uh oh!

erickguan Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dentiny commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erickguan commented Jul 10, 2026

Uh oh!

ByteBaker commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dentiny commented Jul 9, 2026 •

edited

Loading

erickguan Jul 9, 2026 •

edited

Loading

dentiny commented Jul 9, 2026 •

edited

Loading