WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#10304 closed defect (bug) (fixed)

WP_Filesystem fixes

Reported by: azaozz Owned by: dd32
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.8
Component: Filesystem API Keywords:
Focuses: Cc:

Description

Fixes and improvements to the WP_Filesystem classes as discussed in #10170:

  • Remove settings for default permission and use FS_CHMOD_FILE and FS_CHMOD_DIR
  • Do chmod after creating directories to ensure proper permissions

Attachments (1)

10304.patch (17.5 KB) - added by azaozz 5 years ago.

Download all attachments as: .zip

Change History (8)

azaozz5 years ago

comment:1 azaozz5 years ago

Merged 10170.2.patch and 10170.2.diff but kept the $mode optional in $this->chmod. As we have the constants with the proper permissions may as well set them there.

mkdir in WP_Filesystem_SSH2 will need more testing, mainly for ssh2_sftp_mkdir() with different permissions.

comment:2 follow-up: dd325 years ago

Do chmod after creating directories to ensure proper permissions

Note: This was fixed for the Direct Class in #10170, It shouldnt be an issue in the other classes due to the original problem being php's mkdir() and umasks.

comment:3 Denis-de-Bernardy5 years ago

Do we get this in 2.8.1 too, or is it going to be 2.9-only material?

comment:4 in reply to: ↑ 2 azaozz5 years ago

Replying to dd32:

Note: This was fixed for the Direct Class in #10170, It shouldn't be an issue in the other classes due to the original problem being php's mkdir() and umasks.

Yes, that exists in the other classes except in WP_Filesystem_SSH2 where it would need some testing.

Are we removing WP_Filesystem_Base::gethchmod() or are we going to fix it. It seems it needs the permissions as decimal int to work properly.

Replying to Denis-de-Bernardy:

Do we get this in 2.8.1 too, or is it going to be 2.9-only material?

It would need more testing and may introduce some plugins incompatibilities as the setDefaultPermissions() method is gone. Don't think there will be enough time to test it properly for 2.8.1.

comment:5 dd325 years ago

Functions worthwhile removing:

  • setDefaultPermissions()
  • find_base_dir() - Deprecated in 2.7
  • get_base_dir() - Deprecated in 2.7
  • gethchmod() - Not used within core, If a plugin needs it, It might as well add it itself so it knows it works.
    • Remove some vals from dirlist() that reference gethchmod() (or a variant of it if returned from 3rd-party items already)
  • Should make getchmod() return 0xxx for all classes under all OS's.
  • Also should consider removing any methods from Direct which are not supported by the FTP's, Theres a heap of those.. Just so people dont rely on it and find it doesnt work.
  • rmdir() should be redirected to delete() is almost all cases.
  • SSH2 needs investigating with regard to using the ssh file wrapper, and maybe a fallback case in there for systems where that cant be used - #10195

comment:6 azaozz5 years ago

(In [11831]) WP_Filesystem fixes and phpdoc, props hakre dd32, see #10304

comment:7 azaozz5 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.