WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#9936 closed defect (bug) (fixed)

Improve Filesystem method choice for 'direct'; introduce FS_METHOD constant.

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

Description

Just talking with azaozz on IRC, It seems that the core upgrades are failing on some hosts due to filesystem permissions errors, Which can be traced back to the systems temporary directory being writable, whilst the actual ABSPATH directory is unwritable.

The attached patch changes get_filesystem_method() to attempt to create a file in ABSPATH called .<current unix time>.

The checks for file ownership are still present.

Patch also introduces the constant 'FS_METHOD' which can be used along side the filesystem_method filter & other filesystem constants to work around any incorrect-choices which come up.

Attachments (4)

9936.diff (1.5 KB) - added by dd32 6 years ago.
9936.2.diff (1.5 KB) - added by dd32 6 years ago.
9936.3.diff (875 bytes) - added by ryan 6 years ago.
9936.4.patch (4.0 KB) - added by azaozz 6 years ago.
Possible patch

Download all attachments as: .zip

Change History (16)

@dd326 years ago

@dd326 years ago

comment:1 @dd326 years ago

2nd patch removes the uneeded slash after ABSPATH..

comment:2 @azaozz6 years ago

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

(In [11454]) Improve Filesystem method choice for 'direct'; introduce FS_METHOD constant, props DD32, fixes #9936

comment:3 @ryan6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened
Warning: unlink(/.../public_html/blog/.1243373695) [function.unlink]: No such file or directory in /.../public_html/blog/wp-admin/includes/file.php on line 637

@ryan6 years ago

comment:4 @ryan6 years ago

Warning fix, not tested yet.

comment:5 follow-up: @azaozz6 years ago

Perhaps the proper behaviour is to check ABSPATH when upgrading the core and WP_PLUGIN_DIR when updating plugins as there may be different permissions or owners. WP_PLUGIN_DIR can be outside ABSPATH.

comment:6 @ryan6 years ago

(In [11465]) Fix unlink warning. see #9936

comment:7 in reply to: ↑ 5 @azaozz6 years ago

Continuing comment 5 above:
and also WP_CONTENT_DIR when installing/updating themes as there's no WP_THEME_DIR, or perhaps introduce WP_THEME_DIR/WP_THEME_URL and make get_theme_root() and get_theme_root_uri() obsolete so themes can be outside of WP_CONTENT_DIR like plugins.

comment:8 @westi6 years ago

Maybe get_filesystem_method() should take as an argument the root folder the user of the filesystem abstraction wants to work in.

Then it can do it's tests with the correct context.

comment:9 follow-up: @dd326 years ago

+1 for a WP_THEME_DIR and friends.

I agree that a better format would be checking the exact directory needed, However, Testing wp-content for plugins/themes would be better IMO, The temporary upgrade folder is needed afterall..

I do think its going to be a very slim set of users who have the wp-content directory writable whilst ABSPATH is not.. I guess however, Setups like VirtualMultiBlog or those with symlinks would be affected.

comment:10 in reply to: ↑ 9 @azaozz6 years ago

Replying to dd32:

I agree that a better format would be checking the exact directory needed, However, Testing wp-content for plugins/themes would be better IMO, The temporary upgrade folder is needed afterall..

Yes, checking WP_CONTENT_DIR for both plugins and themes seems good as even if the plugins have been moved elsewhere and have different permissions, the user should be able to pre-define the correct method with FS_METHOD.

@azaozz6 years ago

Possible patch

comment:11 @azaozz6 years ago

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

(In [11499]) Test writability of ABSPATH when upgrading core or WP_PLUGIN_DIR when installing/updating themes and plugins, fixes #9936

comment:12 @azaozz6 years ago

The message above is wrong, it tests WP_CONTENT_DIR.

Note: See TracTickets for help on using tickets.