Skip to content

S3File does not send its request parameters with the lookups made while opening #1004

Description

@laughingman7743

Problem

S3File does not send its request parameters with the requests that it makes while opening, so an object that needs them cannot be opened.

  • Read mode. S3File.__init__ looks up the object with fs.info(path, version_id=...) (pyathena/filesystem/s3.py, read branch of S3File.__init__), which sends HeadObject through _head_object(). Neither info() nor _head_object() passes request parameters to HeadObject, so the file's s3_additional_kwargs (e.g. SSECustomerAlgorithm/SSECustomerKey, RequestPayer, ExpectedBucketOwner) are not sent. The later GetObject requests do send them.
  • Append mode. The lookups of the existing object (fs.exists(path), fs.info(path), and fs.cat(path) for an object smaller than 5 MiB) send none of the file's parameters either. The upload requests do.

Consequences:

  • An object encrypted with SSE-C cannot be opened for reading or appending unless its metadata is already in the info() cache. The HeadObject API reference says that for an SSE-C object, the request must include the x-amz-server-side-encryption-customer-* headers to retrieve the metadata (otherwise 400 Bad Request).
  • With a per-file RequestPayer (a requester-pays bucket without the filesystem-level requester_pays=True), opening sends HeadObject without the payer acknowledgement.
  • ExpectedBucketOwner is not checked for the lookups.

Expected: the requests made while opening a file receive the file's parameters that their operations accept, as the file's other requests do.

The lookups go through the info() cache, which is keyed by path only. Parameters such as RequestPayer or ExpectedBucketOwner affect whether the request is authorized, so a fix has to decide whether a cached entry can serve a lookup made with different parameters.

Reproduction

Checked offline with a real botocore client, dummy credentials, and _call replaced to record the requests (no request left the process):

from pyathena.filesystem.s3 import S3FileSystem

SSE_C = {"SSECustomerAlgorithm": "AES256", "SSECustomerKey": "k" * 32}
calls = []


def call(method, **request):
    name = method if isinstance(method, str) else method.__name__
    calls.append((name, sorted(k for k in request if k not in ("Bucket", "Key"))))
    if name == "head_object":
        return {"ContentLength": 3, "ETag": '"e"'}
    if name == "get_object":
        return {"Body": type("B", (), {"read": lambda self: b"abc"})()}
    return {}


fs = S3FileSystem(key="dummy", secret="dummy", region_name="us-east-1", skip_instance_cache=True)
fs._call = call
with fs.open("s3://bucket/key", "rb", s3_additional_kwargs=SSE_C) as f:
    f.read()
print("read:  ", calls)

calls.clear()
fs.invalidate_cache()
with fs.open("s3://bucket/key", "ab", s3_additional_kwargs={**SSE_C, "RequestPayer": "requester"}) as f:
    f.write(b"d")
print("append:", calls)

On master 55af09a:

read:   [('head_object', []), ('get_object', ['IfMatch', 'Range', 'SSECustomerAlgorithm', 'SSECustomerKey'])]
append: [('head_object', []), ('get_object', []), ('put_object', ['Body', 'IfMatch', 'RequestPayer', 'SSECustomerAlgorithm', 'SSECustomerKey', 'StorageClass'])]

The HeadObject of both opens and the GetObject that reads the small existing object for the append carry no parameters. The same holds with PR #1000 (882ae2f), which routes the file's parameters to its other requests.

The 400 response from S3 for an SSE-C object was not reproduced (the CI bucket does not use SSE-C); it follows from the HeadObject API reference.

Environment

Proposed fix (optional)

  • Let info() (and exists()/cat() as used by the append path) take request parameters and pass the HeadObject/GetObject subset to their requests. With PR Route S3 request parameters to the operations that accept them #1000, S3FileSystem._get_operation_kwargs() selects the subset.
  • Decide the cache behavior for lookups with parameters, for example skip the cache (refresh=True) when request parameters are given, or include the authorization-related parameters in the cache key.
  • Unit tests with a recording _call; a real-S3 SSE-C test depends on the bucket allowing SSE-C.

Found during the independent review of PR #1000.

Activity

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