fix(s3): send explicit Content-Length on every request - #171
fix(s3): send explicit Content-Length on every request#171ArnabChatterjee20k wants to merge 1 commit into
Conversation
Uploading large (chunked) files to S3-compatible storage such as GCS failed with HTTP 411 "POST requests require a Content-length header". call() handed a streamed body to the transport without a Content-Length header. The cURL adapter then streams a body of unknown size with Transfer-Encoding: chunked, and emits no length at all for an empty-body POST (createMultipartUpload) — both of which S3-compatible services reject with 411. hashBody() already reads the whole (seekable) body once to sign it, so count the bytes there and set an explicit, signed content-length header for every request (0 for empty bodies). The value equals the full body size, matching what the transport sends for a size-known stream. Fixes appwrite/appwrite#13548
|
Thanks for contributing! This repository is a read-only mirror; development for this library happens in |
|
| // or omits the header on an empty POST — which S3-compatible services such | ||
| // as GCS reject with HTTP 411. The value is the full body size, so it also | ||
| // matches what the transport sends for a size-known stream. | ||
| $headers['content-length'] = (string) $length; |
There was a problem hiding this comment.
Content-Length behavior lacks coverage
The new request behavior is not covered by regression tests. Existing request tests do not assert the observable Content-Length, and multipart preparation bypasses the real request-building path. Please add request-level tests showing that an unknown-size seekable stream sends its full byte count and that an empty multipart-initiation POST sends Content-Length: 0; otherwise either part of the reported failure could return without CI detecting it.
Knowledge Base Used: S3-compatible storage devices
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Storage/Device/S3.php
Line: 756
Comment:
**Content-Length behavior lacks coverage**
The new request behavior is not covered by regression tests. Existing request tests do not assert the observable `Content-Length`, and multipart preparation bypasses the real request-building path. Please add request-level tests showing that an unknown-size seekable stream sends its full byte count and that an empty multipart-initiation POST sends `Content-Length: 0`; otherwise either part of the reported failure could return without CI detecting it.
**Knowledge Base Used:** [S3-compatible storage devices](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/utopia-php/storage/-/docs/s3-compatible-storage-devices.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Problem
Uploading large (chunked) files to S3-compatible storage — reported against GCS — fails with:
Reported downstream in appwrite/appwrite#13548 (self-hosted 2.0, files 15 MB–550 MB).
Cause
S3::call()handed a streamed body to theutopia-php/clienttransport without aContent-Lengthheader. The cURL adapter then:StreamInterface::getSize()returnsnull) withTransfer-Encoding: chunked, andcreateMultipartUpload).Many S3-compatible services (GCS in particular) reject both with 411.
Fix
hashBody()already reads the whole (seekable) body once to sign it, so it now also counts the bytes and returns the length.call()sets an explicit, signedcontent-lengthheader on every request (0for empty bodies). The value is the full body size, so it matches what the transport already sends for a size-known stream — no behavioural change for those, and correct headers for the streamed / empty-body cases.No new reads of the body are introduced.
Testing
php -lclean. Existing signing path unchanged apart from the added (signed) header.Fixes appwrite/appwrite#13548