Broken methods in WP_Filesystem_Direct and WP_Filesystem_SSH2

[DavidAnderson]

Hi,

#25741 mentions some broken methods in WP_Filesystem classes, but here are two different broken methods, for different reasons:

getchmod() in both WP_Filesystem_Direct and WP_Filesystem_SSH2 uses substr(, 3) on the results of decoct().

e.g. in WP_Filesystem_Direct:

return substr(decoct(@fileperms($file)),3);

That's wrong if $file was a directory; in the directory case, substr(, 3) results in the first character being dropped:

$ mkdir /tmp/775
$ chmod 775 /tmp/775
$ ls -ld /tmp/775
$ ls -ld /tmp/775
drwxrwxr-x. 2 david david 4096 Dec 13 10:44 /tmp/775
$ php -r 'echo decoct(@fileperms("/tmp/775"));'
40775
$ php -r 'echo substr(decoct(@fileperms("/tmp/775")),3);'
75

The correct result would be obtained for both directories and files with the help of a sprintf:

substr(sprintf("%06d", decoct(@fileperms($file))),3);

e.g.:

$ mkdir /tmp/775dir
$ touch /tmp/775file
$ chmod 775 /tmp/775dir /tmp/775file
$ ls -ld /tmp/775 /tmp/775file
drwxrwxr-x. 2 david david 4096 Dec 13 10:44 /tmp/775
-rwxrwxr-x. 1 david david    0 Dec 13 10:49 /tmp/775file
$ php -r 'echo substr(sprintf("%06d", decoct(@fileperms("/tmp/775dir"))),3);'
775
$ php -r 'substr(sprintf("%06d", decoct(@fileperms("/tmp/775file"))),3);'
775

Patch attached.

[DavidAnderson]

Patch (replacing previous version)

[DavidAnderson]

Note that gethchmod() is also broken when called on a directory as a knock-on effect of this bug, as it calls getchmod().

[SergeyBiryukov]

Introduced in [10919] and [11063].

[dd32]

Instead of using sprintf could we simply use negative positions with substr? ie, substr( $data, -3 ) to pull the last 3 characters?

[DavidAnderson]

New patch using substr(, -3) instead of sprintf

[DavidAnderson]

Hi,

I've attached an updated patch.

It should be entirely equivalent, as I think there's no way for the results of dec2oct to exceed 6 characters. But, if perhaps there is, then using substr(, -3) instead will have it covered.

David

[nacin]

In 27566:

Filesystem: Fix getchmod() for direct and ssh2 transports, for directories.

props DavidAnderson.
fixes #26598.