Repository navigation
Closed
Labels
Description
Part of #1327 (defects 2 to 7 of the audit report).
Summary
S3File loses or overrides what the caller gave open() and what the open-time lookup resolved, in six places. Each was reproduced offline with a fake client at 766d79d.
Defects
- A read-mode
open()replaces the caller'sIfMatch.kwargs.update({"IfMatch": etag})(s3.py:2787-2788) overwrites it with the current ETag:fs.open(p, "rb", IfMatch='"stale"').read()returns the current object instead of failing with 412. - A reopened file reads another object with the old size.
S3File.__init__callsAbstractBufferedFile.__init__without the request parameters (s3.py:2934-2943), soself.kwargsis{}and__reduce__/fsspec.spec.reopendropRequestPayerandSSECustomer*. The reopened file pins the ETag current at reopen but keeps the size of the original (s3.py:2790-2791): after a 4-byte object was replaced by an 8-byte one, the unpickled file returned the first 4 bytes of the new object without an error. S3File.metadata(),getxattr(),url()andsetxattr()drop the file's lookup parameters (s3.py:3225-3266): for a file opened with SSE-C, the open-time HeadObject carriesSSECustomer*, the one ofmetadata()onlyBucketandKey.- An append's part copies are not pinned to the object looked up at open.
_initiate_upload(s3.py:3083-3097) copies fromS3Path(bucket, key)without the version orCopySourceIfMatch; above 5 GiB it re-runs an unpinnedinfo()for the size, whileAioS3StreamedFileuses the open-time size (s3_async.py:1526-1529). The small-object path reads the old content with acat_filethat is not conditioned on the open-time ETag (s3.py:2795-2804). An overwrite between open and flush makes the result combine the new object's bytes with the old object's metadata, or mix two objects across part copies. abandxbon a key that is only a prefix. With no object atdirand objects underdir/,wbwrites,abraisesFileNotFoundError(it runscat_fileon the directory entry), andxbraisesFileExistsErroralthough no object exists (s3.py:2795-2809). Read mode already treats a directory entry as no object (s3.py:2775-2777).commit()on a read-mode file raisesAttributeError: 'NoneType' object has no attribute 'multipart'(s3.py:3180-3181);discard()is a documented no-op for reading.
Fix direction
Keep the resolved target (ETag, version, size, lookup parameters) on the file as the single source of the pin: respect or verify a caller's IfMatch, carry the per-open parameters and the pin through __reduce__ (deciding how SSE-C keys are pickled), route the file's metadata methods through the target's lookup parameters, plan append copies from the pinned version or ETag and the open-time size in both file classes, treat a prefix-only key as no object in ab/xb, and make commit() a no-op in read mode.
Activity
laughingman7743 commentedon Nov 6, 2026
Re-checked at 2a60ecc with an offline recording client: all six defects reproduce. One more in the same path: the part copies of an append send the file's
SSECustomer*only under the destination names, so an append to an SSE-C object larger than 5 MiB has noCopySourceSSECustomer*and fails with 400. It is folded into item 4.Decisions
IfMatchin read mode: kept as given, not replaced by the ETag of the lookup, and not compared locally. A mismatch fails the first read with S3's 412 (OSError(EINVAL)), as an object replaced after open does.kwargs(passed byfsspec.spec.reopen) carry the caller's per-open request parameters and the open-time pin:IfMatchwith the open-time ETag, the size, and the version in the path. A reopened file whose object was replaced fails on read with 412 instead of reading the new object with the old size. SSE-C keys are included in the pickle, as a pickled filesystem includes its credentials; the docstring states it.S3File.metadata(),getxattr()andurl()send the file's lookup parameters (RequestPayer,ExpectedBucketOwner,SSECustomer*); explicit keyword arguments take precedence.setxattr()maps them to the self-copy (CopySourceSSECustomer*and the destinationSSECustomer*,ExpectedSourceBucketOwnerandExpectedBucketOwner), withcopy_kwargstaking precedence.IfMatchwith the ETag looked up at open; the part copies sendCopySourceIfMatchwith that ETag, the source's SSE-C parameters under the copy-source names, and use the open-time size (no second unpinnedinfo()), in bothS3FileandAioS3StreamedFile. The copies are pinned by ETag, not by version, so an append to an object overwritten since open fails (and aborts the upload) instead of silently replacing the newer object. The final commit is not made conditional here.ab/xbon a prefix-only key: a directory entry is treated as no object, as read mode does:abwrites a new object andxbcreates it.commit()in read mode is a no-op, asdiscard()is.appended.to_api_repr()overriding the caller's metadata in append mode stays with #1342 (item 2).Reflow the filesystem docs paragraph on S3File parameters (GH-1334)
Address the independent review of the S3File parameters (GH-1334)