Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#10774 closed defect (bug) (fixed)

Include 'hidden' directories in filesystem dirlist by default

Reported by: dd32 Owned by: dd32
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.9
Component: Filesystem API Keywords: has-patch needs-testing
Focuses: Cc:

Description (last modified by dd32)

Ok, Seems theres a bit of a inconsistency between the return values of dirlist() when comparing FTP vs Direct/SSH2.

Direct and SSH2(which was based on the Direct class) include a check to see if '.' is the first character, If it does, And the $incdot param is false (as is default) it skips including the file in the return list

In short, The behavior is unexpected, And i think its whats causing some issues with upgrading plugins which were previously svn checkouts (including Akismet on SVN installs).

.htaccess files would also cause oddities like that while attempting to ugprade the plugin (It'd hit a Cannot remove plugin error)

Attachments (3)

10774.diff (1.0 KB) - added by dd32 6 years ago.
10774.2.diff (7.8 KB) - added by dd32 6 years ago.
10774.3.diff (7.8 KB) - added by dd32 6 years ago.
forgot to lint check ftpext, add missing {

Download all attachments as: .zip

Change History (9)

@dd326 years ago

comment:1 @azaozz6 years ago

What is the purpose of $incdot in dirlist($path, $incdot = false, $recursive = false)? It seems obsolete after the patch but only for SSH2 and Direct, still used in fsockets and ftpext.

comment:2 @dd326 years ago

What is the purpose of $incdot in dirlist

It was originally to include '.' and '..' in the listing, Which is absolutely useless in reality. And as you can see, Its not even used by the ftp transports (Which i hadnt noticed)

It could be renamed to $hidden, default to true, and be used to include .directory in the output..

Either that, Or remove it all together..

That late night patch does make them all act the same, but makes that param redundant.

comment:3 @dd326 years ago

  • Description modified (diff)
  • Keywords needs-patch added; has-patch removed

Ok, I was completely wrong.

It's entire purpose was to include hidden files.. I'm going to re-work the patch & whatnot to handle hidden dir's properly..

comment:4 @dd326 years ago

  • Keywords has-patch needs-testing added; needs-patch removed
  • Summary changed from Include 'hidden' directories in filesystem dirlist for Direct and SSH2 to Include 'hidden' directories in filesystem dirlist by default

Ok, A 2nd attempt:

attachment 10774.2.diff added

  • renames $incdot to $include_hidden
    • Defaults to true instead of false now, I feel its safe to assume that is the intention of most users.
  • renames the variable $limitFile to $limit_file for Coding standards
    • Previously ftpext ignored this..
    • Note: Intention of this is to if dirlst(/dir/dir/dir/file) to return only 'file' and not 'file2', 'file3' etc.
  • Cleaning of some logic related to incdot + recursive..
  • Some spacing here and there

@dd326 years ago

comment:5 @dd326 years ago

Oh, Also, Not all files have been tested. other than a quick syntax check..

@dd326 years ago

forgot to lint check ftpext, add missing {

comment:6 @automattor6 years ago

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

(In [11934]) Include 'hidden' directories in filesystem dirlist by default, props dd32, fixes #10774

Note: See TracTickets for help on using tickets.