[wp-trac] [WordPress Trac] #26598: Broken methods in WP_Filesystem_Direct and WP_Filesystem_SSH2

[WordPress Trac]

#26598: Broken methods in WP_Filesystem_Direct and WP_Filesystem_SSH2
---------------------------+------------------------------
 Reporter:  DavidAnderson  |       Owner:
     Type:  defect (bug)   |      Status:  new
 Priority:  normal         |   Milestone:  Awaiting Review
Component:  Filesystem     |     Version:  trunk
 Severity:  normal         |  Resolution:
 Keywords:  has-patch      |
---------------------------+------------------------------
Description changed by ocean90:

Old description:

> Hi,
>
> https://core.trac.wordpress.org/ticket/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.

New description:

 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.

--

--
Ticket URL: <http://core.trac.wordpress.org/ticket/26598#comment:2>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software