Absolute upload_path fails

[tellyworth]

1. On Options/Misc, enter "/tmp/foo/bar" as a value for the upload path setting.
2. Write a new post and attach an image.

Expected: the image file should be stored as /tmp/foo/bar/2008/02/file.jpg or similar.

Actual: a 'tmp/foo/bar' subdirectory is created within ABSPATH (if that's possible) and the file is stored there. Both the filesystem path and URL path just append '/tmp/foo/bar', so you get 'ABSPATHtmp/foo/bar/2008/02/file.jpg' and '​http://example.com//tmp/foo/bar/2008/02/file.jpg'

An absolute upload_path should probably be rejected unless you also specify an upload_url_path, because there's no way to know what URL corresponds to an arbitrary filesystem path.

[tellyworth]

Ah. There's another precondition required to reproduce: /tmp, /tmp/foo or /tmp/foo/bar is a symbolic link.

[tellyworth]

The attached patch fixes the main issue. It doesn't do anything about the case where upload_path is absolute and upload_url_path is empty - that probably needs a second opinion.

path_is_absolute() and path_join() might be useful elsewhere.

I also snuck in a minor addition: wp_upload_dir() now returns an extra element that just contains the subdir within the upload directory (if any). That might be useful for fixing the file attachment guid situation.

[tellyworth]

Unit tests are here:

​http://svn.automattic.com/wordpress-tests/wp-testcase/test_uploads.php

test_upload_dir_absolute() covers the bug.

[nbachiyski]

In path_join() you can use DIRECTORY_SEPARATOR instead of /.

[ryan]

(In [6984]) Upload path fixes from tellyworth. see #5953

[ryan]

Leaving open in case we want to use DIRECTORY_SEPARATOR.

[tellyworth]

Initially I wrote that function with DIRECTORY_SEPARATOR, but that constant isn't used anywhere else in WP so I changed it for consistency. Also, using / instead of DIRECTORY_SEPARATOR leaves open the possibility of using path_join() on URL paths.

We could use the constant and perhaps create a second function, url_path_join(), that always uses a /. But I think for that to make sense we'd need to review and change all paths in WP to use DS instead of a hard coded slash.

[nbachiyski]

Actually I agree. / works as a separator in Windows and writing path_join or DIRECTORY_SEPERATOR all the time is very tiresome.

[Otto42]

path_join doesn't appear to work properly for some people.

Reference this support thread:
​http://wordpress.org/support/topic/182297

[DD32]

path_join doesn't appear to work properly for some people.

Looking at the code, That'd probably be caused by path_is_absolute() => realpath(), I cant see how the regular expressions could be missing it.

[Denis-de-Bernardy]

realpath() is buggy on some servers. we could try to use the shell:

echo $PWD

but that wouldn't make much of a difference -- if realpath() fails, exec() probably does as well.

[nbachiyski]

$PWD is different than realpath('.'). realpath resolves symlinks and relative references (. and ..). Historically it had caused a lot of problems in the UNIX world, because of strange/buggy implementations.

If realpath fails, we can use dirname to resolve at least . and .. and forget about the symlinks.

[Denis-de-Bernardy]

True. For what it's worth, though, I've found this quite useful in various shell scripts:

path=`cd $PWD && pwd`

I've yet to see it fail on a symb links.

[Denis-de-Bernardy]

see: #7640

[johnbillion]

The core issue was fixed in r6984 and we no longer have these upload path input fields so the issue is moot.