Skip to content

fix: s3: isdir()#380

Merged
namachan10777 merged 1 commit into
masterfrom
fix/isdir
May 15, 2026
Merged

fix: s3: isdir()#380
namachan10777 merged 1 commit into
masterfrom
fix/isdir

Conversation

@namachan10777
Copy link
Copy Markdown
Contributor

Fix a bug in S3 where it incorrectly identified simple prefix matches as directories.

When a partially matching key already exists as shown below, and MaxKeys=1 is set to 1, bus/ would be retrieved first, but since it's not bush, the operation would fail, resulting in the directory being incorrectly identified as non-existent.

  • bus/engine.txt
  • bush/kusamura.txt

The solution is to prepend a / suffix when performing the List operation and determine existence based on whether a Common Prefix or Contents exist. If at least one exists, we can safely consider this prefix as a directory presence, and the presence of the / suffix prevents simple prefix matching from failing.

Related Issue(PR)s

closes #379

Effect

This is a just bug fix.

Fix a bug in S3 where it incorrectly identified simple prefix matches as directories.
@namachan10777 namachan10777 added this to the 2.10 milestone May 15, 2026
@namachan10777 namachan10777 requested a review from Copilot May 15, 2026 01:42
@namachan10777 namachan10777 self-assigned this May 15, 2026
@namachan10777 namachan10777 added the cat:bug Bug report or fix. label May 15, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes pfio.v2.S3.isdir() incorrectly treating forward-prefix collisions (e.g., abc vs abcde) as “non-existent directory” when list_objects_v2(MaxKeys=1) returns a non-matching prefix first. The fix makes the S3 listing unambiguous by querying with a trailing slash and considers the directory present if S3 returns either Contents or CommonPrefixes.

Changes:

  • Update S3.isdir() to use Prefix=f"{key}/" and determine existence via presence of Contents/CommonPrefixes.
  • Add a regression test that exercises prefix-collision cases (abc/ vs abcde/, and nested xyz/abc*).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pfio/v2/s3.py Adjusts isdir() S3 listing logic to avoid forward-prefix collisions and correctly detect directory presence.
tests/v2_tests/test_s3.py Adds coverage for the prefix-collision regression scenario.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@namachan10777 namachan10777 merged commit 65fcae4 into master May 15, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cat:bug Bug report or fix.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Forward-matching directory names cause s3.isdir to fail

3 participants