Opened 14 years ago
Closed 13 years ago
#15575 closed defect (bug) (fixed)
ftp upgrade fails on some system due to slash in remote destination
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.3 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | Filesystem API | Keywords: | has-patch 3.2-early needs-testing |
Focuses: | Cc: |
Description
Routinely my users were unable to upgrade plugins on Mac OS X (10.5) server hosted sites.
The error occurred within upgrade.php in
class-wp-upgrader.php
The response was:
Could not create directory. <path to ftp wp-content>
/plugins/<my-plugin>
/
(At least the ftp on Max OS X (10.5) Server) The problem is that in ftp one can not do:
mkdir foo/
if foo doesn't exist then "mkdir foo/" will fail
The correct syntax is to remove the end slash:
mkdir foo
Then two solutions present themselves:
Either in class-wp-upgrader.php
//Protection against deleting files in any important base directories. if ( in_array( $destination, array(ABSPATH, WP_CONTENT_DIR, WP_PLUGIN_DIR, WP_CONTENT_DIR . '/themes') ) ) { $remote_destination = trailingslashit($remote_destination) . trailingslashit(basename($source)); $destination = trailingslashit($destination) . trailingslashit(basename($source)); }
Adding
if ( substr($remote_destination,-1,1) == '/' ) { $path = substr($remote_destination,0,-1); }
Better, we add it to the top of class-wp-filestream-ftpext.php where the problem is occurring. Since we may not be in filesystem->ftpext when running through wp-class-upgrader.php
function mkdir($path, $chmod = false, $chown = false, $chgrp = false) {
Adding
if ( substr($path,-1,1) == '/' ) { $path = substr($path,0,-1); }
This however seems to be potentially an FTP client issue as I've not tested this on other servers. So the inevitable fix may yet require a server capabilities test flag so as not to screw up other servers that might require the end slash.
If however the end slash is completely syntactically wrong for all mkdir commands:
One potential other fix is to change
$remote_destination = trailingslashit($remote_destination) . trailingslashit(basename($source)); $destination = trailingslashit($destination) . trailingslashit(basename($source));
to
$remote_destination = trailingslashit($remote_destination) . basename($source); $destination = trailingslashit($destination) . basename($source);
I'm hazarding the solution but am not sure what is the best fix for this. I will be using the mkdir hack on my system until I see how this bug pans out.
Thanks
-A
Attachments (2)
Change History (18)
#1
@
14 years ago
- Summary changed from ftp upgrade fails on some system due to slash in remote destination to ftp upgrade fails on some system due to slash in remote destination (w/solution)
#4
@
14 years ago
- Summary changed from ftp upgrade fails on some system due to slash in remote destination (w/solution) to ftp upgrade fails on some system due to slash in remote destination (w/patch)
#6
@
14 years ago
- Severity changed from major to normal
- Summary changed from ftp upgrade fails on some system due to slash in remote destination (w/patch) to ftp upgrade fails on some system due to slash in remote destination
#8
@
14 years ago
- Keywords needs-patch 3.2-early added; has-patch removed
- Milestone changed from Awaiting Review to Future Release
Easier way to fix this would be using rtrim(). Specifically, we have a function imaginatively called untrailingslashit().
#9
@
14 years ago
- Component changed from Upgrade/Install to Filesystem
- Version changed from 3.0.3 to 3.0
#10
@
14 years ago
- Keywords has-patch added; needs-patch removed
I've added untrailingslashit()
to class-wp-filesystem-ftpext.php
and class-wp-filesystem-ftpsockets.php
. In class-wp-filesystem-direct.php
and class-wp-filesystem-ssh2.php
it's already there.
Not sure about class-ftp.php
and other functions like rmdir()
. Should we use it there too?
#11
@
14 years ago
Not sure about class-ftp.php and other functions like rmdir(). Should we use it there too?
If the function will error out without having an untrailingslash'd then it should be added. class-ftp.php
shouldn't be touched as class-wp-filesystem-ftpsockets.php
is a wrapper for it.
There are some strange cases with / and rm from memory, things like folder/file detecting issues, I wouldnt be surprised if that's one reason why a trailing slash is included on some of them.
#13
@
13 years ago
- Version changed from 3.0 to 3.3
I have tested this patch extensively with the last 3 releases on several different installs.
Who do I need to pass this to so that I can stop having to manually re-add this every time I patch?
#14
@
13 years ago
- Milestone changed from Future Release to 3.3
- Owner set to dd32
- Status changed from new to accepted
- Version changed from 3.3 to 3.0
I'll throw this into 3.3 before beta2, to hopefully catch any side effects of it, whilst there shouldn't be any, if some servers don't like the trailing slash, there's a chance of others wanting it.
For what it's worth, the RFC specs for FTP don't mention anything about the expected format of the path to create, however the examples don't mention a trailing slash as part of the path. It seems like no trailing slash is the least ambiguous option.
The Version field is used to track when a issue was first raised/diagnosed, Please do not change it to a later version if it affects the earlier versions
See:
http://bugs.php.net/42739
http://bugs.php.net/43276