Make WordPress Core

Opened 3 months ago

Closed 3 months ago

#64426 closed defect (bug) (fixed)

WP_Filesystem_Direct::getchmod() does not handle fileperms() errors

Reported by: vietcgi's profile vietcgi Owned by: westonruter's profile westonruter
Milestone: 7.0 Priority: normal
Severity: normal Version:
Component: Filesystem API Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Description:

The getchmod() method in WP_Filesystem_Direct has a documented FIXME:

"FIXME does not handle errors in fileperms()"

When a file doesn't exist, fileperms() returns false, which gets converted to '0' - not a valid permission string.

This PR fixes it by returning false on failure, matching the pattern used by owner() and group() in the same class.

PR: https://github.com/WordPress/wordpress-develop/pull/10637

Change History (5)

This ticket was mentioned in PR #10637 on WordPress/wordpress-develop by vietcgi.


3 months ago
#1

  • Keywords has-patch has-unit-tests added

## Summary

The getchmod() method in WP_Filesystem_Direct had a documented FIXME noting it doesn't handle errors from fileperms().

When a file doesn't exist, fileperms() returns false, which then gets passed to decoct() and returns '0' - not a valid permission string.

This PR adds proper error handling to return false on failure, matching the pattern used by other methods in the class like owner() and group().

## Changes

  • Return false when fileperms() fails
  • Update return type to string|false
  • Update test to verify false is returned for non-existent paths

## Testing

  • Existing files: returns permissions as before (e.g. '644')
  • Non-existent files: now returns false instead of '0'
  • gethchmod() still works correctly (intval(false, 8) equals 0, same as before)

#2 @SergeyBiryukov
3 months ago

  • Component changed from General to Filesystem API
  • Milestone changed from Awaiting Review to 7.0

vietcgi commented on PR #10637:


3 months ago
#3

@westonruter it's looking good now. Thanks

#4 @westonruter
3 months ago

  • Owner set to westonruter
  • Status changed from new to reviewing

#5 @westonruter
3 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 61398:

Filesystem API: Resolve FIXME comment for WP_Filesystem_Direct::getchmod() by explicitly returning '0' in error case.

Developed in https://github.com/WordPress/wordpress-develop/pull/10637

Follow-up to [11831].

Props vietcgi, westonruter.
See #10304.
Fixes #64426.

Note: See TracTickets for help on using tickets.