Skip to content

Fix recursive chmod, empty key segments, and version paths in listings #1335

Closed

Description

@laughingman7743

Part of #1327 (defects 8 to 11 of the audit report).

Summary

Listing and recursive operations mishandle four path shapes. Each was reproduced offline at 766d79d.

Defects

  1. chmod(path, acl, recursive=True) skips the object at the key when the key is also a prefix. With objects a and a/x, chmod("bucket/a", acl) updates a, and recursive=True updates only a/x: find(path, withdirs=False) lists a/... (s3.py:2206), and the method returns early on the assumption that a key prefix is not an object (s3.py:2232-2235). rm(recursive=True) does delete a.
  2. Keys with an empty segment (dir//x). A common prefix is named with c.prefix[:-1].rstrip("/") (s3.py:516-518, :585), so dir// becomes bucket/dir, the directory being listed, and _find_levels recurses into the same name (s3.py:857-860). ls("bucket/dir") returns ['bucket/dir']; find("bucket/dir", maxdepth=4, withdirs=True) returns bucket/dir five times; find("bucket/dir", maxdepth=4) returns [] and misses dir//x, which find without maxdepth finds. rm, copy and glob with a maxdepth miss such objects.
  3. ls/find of a version-qualified path. For bucket/dir?versionId=v1, _ls_dirs lists the prefix dir/, puts the version ID on the directory entries, and caches the listing under ("bucket/dir?versionId=v1", "/") (s3.py:497-527), which no invalidation reaches: after pipe_file("bucket/dir/b"), ls of the version path still returned the old listing without a request. A version path names one object, as info, expand_path and rm treat it.
  4. _split_version_paths is quadratic: [p for p in paths if p not in versions] is a list membership test (s3.py:963-964); 20,000 paths take 1.1 s. It is reached by cat, copy and get with lists of paths.

Fix direction

Include the object at the key in recursive chmod; name common prefixes by dropping exactly one delimiter and recurse on the raw prefix; resolve ls/find of a version path to that version without listing or caching; use a set in _split_version_paths.

Activity

  1. laughingman7743 commented on Nov 6, 2026

    @laughingman7743
    MemberAuthor

    Defects 2 to 4 are fixed in #1361; defect 1 (recursive chmod) stays open for a PR after step 3 of #1332.

    Two pre-existing edge cases of the same paths came up in the review of #1361 and are not changed there:

    1. A bucket path with a version query is listed and cached under the query. ls("s3://bucket?versionId=v") and find(...) list the whole bucket, and ls caches it under ("bucket?versionId=v", "/"), which writes under the bucket do not invalidate.
    2. find(path, prefix=..., withdirs=True) without maxdepth, with a prefix that starts with an empty segment, includes the directory above the prefix: for the key dir//sub/x, find("s3://bucket/dir", prefix="/sub", withdirs=True) includes bucket/dir/, because the base key joins the key and the prefix's head with if k and drops the empty head. With maxdepth it is left out, as documented.
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