Skip to content

Keep the caller's parameters and the open-time pin in S3File #1334

Closed

Description

@laughingman7743

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

  1. A read-mode open() replaces the caller's IfMatch. 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.
  2. A reopened file reads another object with the old size. S3File.__init__ calls AbstractBufferedFile.__init__ without the request parameters (s3.py:2934-2943), so self.kwargs is {} and __reduce__/fsspec.spec.reopen drop RequestPayer and SSECustomer*. 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.
  3. S3File.metadata(), getxattr(), url() and setxattr() drop the file's lookup parameters (s3.py:3225-3266): for a file opened with SSE-C, the open-time HeadObject carries SSECustomer*, the one of metadata() only Bucket and Key.
  4. An append's part copies are not pinned to the object looked up at open. _initiate_upload (s3.py:3083-3097) copies from S3Path(bucket, key) without the version or CopySourceIfMatch; above 5 GiB it re-runs an unpinned info() for the size, while AioS3StreamedFile uses the open-time size (s3_async.py:1526-1529). The small-object path reads the old content with a cat_file that 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.
  5. ab and xb on a key that is only a prefix. With no object at dir and objects under dir/, wb writes, ab raises FileNotFoundError (it runs cat_file on the directory entry), and xb raises FileExistsError although no object exists (s3.py:2795-2809). Read mode already treats a directory entry as no object (s3.py:2775-2777).
  6. commit() on a read-mode file raises AttributeError: '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

  1. laughingman7743 commented on Nov 6, 2026

    @laughingman7743
    MemberAuthor

    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 no CopySourceSSECustomer* and fails with 400. It is folded into item 4.

    Decisions

    1. Caller's IfMatch in 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.
    2. Reopen / pickle: the file's kwargs (passed by fsspec.spec.reopen) carry the caller's per-open request parameters and the open-time pin: IfMatch with 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.
    3. File metadata methods: S3File.metadata(), getxattr() and url() send the file's lookup parameters (RequestPayer, ExpectedBucketOwner, SSECustomer*); explicit keyword arguments take precedence. setxattr() maps them to the self-copy (CopySourceSSECustomer* and the destination SSECustomer*, ExpectedSourceBucketOwner and ExpectedBucketOwner), with copy_kwargs taking precedence.
    4. Append pin: the small-object read sends IfMatch with the ETag looked up at open; the part copies send CopySourceIfMatch with that ETag, the source's SSE-C parameters under the copy-source names, and use the open-time size (no second unpinned info()), in both S3File and AioS3StreamedFile. 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.
    5. ab/xb on a prefix-only key: a directory entry is treated as no object, as read mode does: ab writes a new object and xb creates it.
    6. commit() in read mode is a no-op, as discard() is.

    appended.to_api_repr() overriding the caller's metadata in append mode stays with #1342 (item 2).

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions