Skip to content

Fix is_readable() to test readability, not writability - #41

Merged
remyperona merged 1 commit into
developfrom
fix/is-readable-predicate-34
Aug 13, 2026
Merged

Fix is_readable() to test readability, not writability#41
remyperona merged 1 commit into
developfrom
fix/is-readable-predicate-34

Conversation

@remyperona

Copy link
Copy Markdown
Contributor

Summary

VirtualFilesystemDirect::is_readable() delegated to is_writeable(), so it returned an entry's writability instead of its readability. A read-only entry (0444) is readable but not writable, and was wrongly reported as "not readable".

This is not cosmetic: dirlist() gates on is_readable(), so listing a read-only directory returned false — a false negative for exactly the permission scenario this filesystem mock exists to reproduce.

Change

public function is_readable( $file ) {
-	return is_writeable( $this->getUrl( $file ) );
+	return is_readable( $this->getUrl( $file ) );
}

Regression coverage

  • isReadable.php: 0444 asserts true
  • isWritable.php: 0444 asserts false (guards against the inverse regression)
  • dirlist.php: a read-only directory still returns its listing, not false

The pre-existing unit tests only covered 0777 (→ true) and 000 (→ false), both of which passed under the buggy delegation, which is why this went unnoticed.

Validation

Ran the full suite in Docker via wp-env (WordPress test site, PHP 8.3.33, PHPUnit 9.6.35):

  • Unit: 93 tests, 284 assertions — OK
  • Integration: 29 tests, 70 assertions — OK
  • Integration (AdminOnly): 3 tests, 4 assertions — OK

Both new read-only cases were confirmed to execute (not silently skipped).

Fixes #34

🤖 Generated with Claude Code

VirtualFilesystemDirect::is_readable() delegated to is_writeable(),
so a read-only entry (0444) was wrongly reported as not readable.
Because dirlist() gates on is_readable(), listing a read-only
directory returned false — a false negative for the exact permission
scenario this filesystem mock exists to reproduce.

Use is_readable() for the predicate, and add regression coverage:
- isReadable: 0444 asserts true
- isWritable: 0444 asserts false (guards the inverse regression)
- dirlist: read-only directory still returns its listing

Fixes #34

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@remyperona remyperona self-assigned this Aug 13, 2026
@remyperona
remyperona merged commit 710956f into develop Aug 13, 2026
7 checks passed
@remyperona
remyperona deleted the fix/is-readable-predicate-34 branch August 13, 2026 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

is_readable() returns writability, not readability (breaks dirlist on read-only dirs)

1 participant