WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 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 8 years ago.
9936.2.diff (1.5 KB) - added by dd32 8 years ago.
9936.3.diff (875 bytes) - added by ryan 8 years ago.
9936.4.patch (4.0 KB) - added by azaozz 8 years ago.
Possible patch

Download all attachments as: .zip

Change History (16)

@dd32
8 years ago

@dd32
8 years ago

#1 @dd32
8 years ago

2nd patch removes the uneeded slash after ABSPATH..

#2 @azaozz
8 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

#3 @ryan
8 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

@ryan
8 years ago

#4 @ryan
8 years ago

Warning fix, not tested yet.

#5 follow-up: @azaozz
8 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.

#6 @ryan
8 years ago

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

#7 in reply to: ↑ 5 @azaozz
8 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.

#8 @westi
8 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.

#9 follow-up: @dd32
8 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.

#10 in reply to: ↑ 9 @azaozz
8 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.

@azaozz
8 years ago

Possible patch

#11 @azaozz
8 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

#12 @azaozz
8 years ago

The message above is wrong, it tests WP_CONTENT_DIR.

Note: See TracTickets for help on using tickets.