Keys containing '?' are rejected, and pinned or listed versions are not addressable

[laughingman7743]

Problem

Paths are parsed with one regular expression that treats every ? as the start of a version query, and the version that a file or listing refers to is not carried into the paths it produces.

1. Keys containing ? cannot be used. PATTERN_PATH only accepts a key without ? and only accepts a ?versionId=-style query after it (pyathena/filesystem/s3.py:125-128, parse_path() at :254-275). S3 allows ? in keys, and ls()/find() return such keys, but every call that parses the path raises ValueError: Invalid S3 path format:

   • info(), exists() and open() on s3://bucket/dir/what?.txt;
   • rm("s3://bucket/dir", recursive=True) on a prefix that contains such a key, because _delete_objects() parses each listed path (s3.py:938). No key under the prefix is deleted.

   PR Invalidate the object path when a version is deleted #960 (invalidate_cache() for versioned paths) also splits paths at the first ?, relying on keys not containing it.

2. S3File.metadata(), getxattr() and url() ignore the pinned version. With version_aware=True, S3File pins the version it observed at open time in S3File.version_id (s3.py:2220-2225), and reads use it. metadata(), getxattr() and url() pass only self.path to the filesystem (s3.py:2438, :2452, :2466), so they describe and sign the latest version instead. (open(path, version_id=...) itself is S3FileSystem.open() and cat_file() raise TypeError when given version_id #936, fixed by PR Accept version_id in S3FileSystem.open() and cat_file() #958.)

3. ls(versions=True, detail=False) returns names that do not identify a version. Each version is listed by its plain key name (s3.py:295-304, :519-546), so two versions of one key appear as two identical strings, and neither can be passed back to open()/info() to reach that version. detail=True entries carry version_id, but their name is also the plain key. test_ls_versions pins the plain names (tests/pyathena/filesystem/test_s3.py:563-585), so changing them needs a maintainer decision.

Expected:

• keys containing ? can be read, listed and deleted; only a trailing ?versionId= (and its spellings) is treated as a version query;
• S3File.metadata(), getxattr() and url() use S3File.version_id;
• versions returned by ls(versions=True) can be addressed from their names.

Reproduction

from unittest.mock import MagicMock
from pyathena.filesystem.s3 import S3FileSystem

def make_fs(responses, **kwargs):
    fs = S3FileSystem(connection=MagicMock(), skip_instance_cache=True, **kwargs)
    calls = []
    def call(method, **request):
        name = method._extract_mock_name().split(".")[-1]
        calls.append((name, request))
        if isinstance(responses[name], Exception):
            raise responses[name]
        return responses[name]
    fs._call = call
    return fs, calls

fs, calls = make_fs({
    "head_object": FileNotFoundError("dir"),
    "list_objects_v2": {
        "Contents": [{"Key": "dir/a.txt", "Size": 1}, {"Key": "dir/what?.txt", "Size": 1}],
        "KeyCount": 2,
    },
})
print(fs.ls("s3://bucket/dir"))
# ['bucket/dir/a.txt', 'bucket/dir/what?.txt']
for method in (fs.info, fs.exists, fs.open):
    try:
        method("s3://bucket/dir/what?.txt")
    except ValueError as e:
        print(method.__name__, e)
# info Invalid S3 path format bucket/dir/what?.txt.
# exists Invalid S3 path format bucket/dir/what?.txt.
# open Invalid S3 path format bucket/dir/what?.txt.
try:
    fs.rm("s3://bucket/dir", recursive=True)
except ValueError as e:
    print("rm", e, [name for name, _ in calls if name == "delete_objects"])
# rm Invalid S3 path format bucket/dir/what?.txt. []

fs, calls = make_fs({
    "head_object": {"ContentLength": 1, "ETag": '"e"', "VersionId": "v1"},
    "generate_presigned_url": "https://signed",
}, version_aware=True)
f = fs.open("s3://bucket/key", "rb")
f.metadata()
f.url()
print(f.version_id, calls[-2:])
# v1 [('head_object', {'Bucket': 'bucket', 'Key': 'key'}), ('generate_presigned_url', {'ClientMethod': 'get_object', 'Params': {'Bucket': 'bucket', 'Key': 'key'}, 'ExpiresIn': 3600})]

fs, calls = make_fs({"list_object_versions": {
    "Versions": [
        {"Key": "p/key", "VersionId": "v2", "IsLatest": True, "Size": 4},
        {"Key": "p/key", "VersionId": "v1", "IsLatest": False, "Size": 2},
    ],
    "IsTruncated": False,
}}, version_aware=True)
print(fs.ls("s3://bucket/p", versions=True))
# ['bucket/p/key', 'bucket/p/key']

Other observed results:

• With version_aware=True, after the object was overwritten between open() and read(), read() returned the first version's bytes and f.metadata() sent HeadObject without VersionId.

Environment

• PyAthena master e0e85da, fsspec 2026.9.0, botocore from uv.lock, Python 3.13.1.
• Found in a full audit of pyathena/filesystem/. Unless noted, checked offline with mocked S3 responses (fs._call replaced) or botocore's Stubber with dummy credentials.

Proposed fix (optional)

• Parse the version query only when the path ends with ?versionId=/?versionID=/?versionid=/?version_id=, and take everything before it as the key, so [^?] is no longer required. Update PR Invalidate the object path when a version is deleted #960's path splitting the same way.
• Make S3File.metadata(), getxattr() and url() pass version_id=self.version_id (adding version_id to S3FileSystem.metadata()/getxattr()/sign(), as info() already has).
• If the maintainers agree, name versioned listing entries bucket/key?versionId=<id> and update test_ls_versions.

[laughingman7743]

Another case for item 1, found in the independent review of PR #993 (#963). Please handle it together with the ? parsing here.

A version-qualified path passed as a single string is expanded as a glob. fsspec's expand_path() treats any path containing ? as a pattern (has_magic()), so ?versionId=... becomes a one-character wildcard and the version is never addressed. copy() and mv() (since #1013, S3FileSystem._move_paths() calls expand_path()) go through it, and fsspec's get() does too. Lists of paths skip the expansion.

Measured offline with mocked ListObjectsV2/HeadObject/CopyObject/DeleteObjects responses on master a73c6db, with the keys b and c:

Call	Result
expand_path("s3://bucket/b?versionId=v1")	FileNotFoundError(['bucket/b?versionId=v1']) after one ListObjectsV2
copy("s3://bucket/b?versionId=v1", "s3://bucket/out")	same FileNotFoundError, nothing copied
mv("s3://bucket/b?versionId=v1", "s3://bucket/out")	same FileNotFoundError, nothing moved
cp_file("s3://bucket/b?versionId=v1", "s3://bucket/out")	copies v1 (HeadObject, CopyObject)
rm("s3://bucket/b?versionId=v1")	sends DeleteObjects for the path

Expected: a trailing ?versionId= (and its spellings) is not a glob character, so the calls above address that version, as cp_file() and rm() already do. This needs the same rule as the parsing in item 1 (only a trailing version query is special) so that keys containing ? still work as literal keys and as patterns.

Related, in PR #993: _move_paths() now compares the paths of a move by bucket/key, keeping the version ID except for null, which a write to the key replaces. It does this with _move_target(), which uses parse_path(), so the parsing change here also applies to it.