Ticket #5953 (reopened defect)

Opened 9 months ago

Last modified 4 months ago

Absolute upload_path fails

Reported by: tellyworth Assigned to: tellyworth
Priority: normal Milestone: 2.9
Component: General Version:
Severity: normal Keywords:
Cc:

Description

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 'ABSPATH//tmp/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.

Attachments

absolute-upload-dir-r6957.patch (2.5 kB) - added by tellyworth on 02/22/08 06:38:46.

Change History

02/22/08 05:50:55 changed by tellyworth

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

02/22/08 06:38:46 changed by tellyworth

  • attachment absolute-upload-dir-r6957.patch added.

02/22/08 06:42:17 changed by 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.

02/22/08 06:44:05 changed by tellyworth

Unit tests are here:

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

test_upload_dir_absolute() covers the bug.

02/22/08 13:29:00 changed by nbachiyski

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

02/22/08 17:46:04 changed by ryan

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

02/22/08 17:46:34 changed by ryan

  • owner changed from anonymous to tellyworth.

Leaving open in case we want to use DIRECTORY_SEPARATOR.

02/23/08 00:28:49 changed by 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.

03/05/08 12:34:49 changed by nbachiyski

  • status changed from new to closed.
  • resolution set to fixed.

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

06/18/08 13:43:05 changed by Otto42

  • status changed from closed to reopened.
  • resolution deleted.

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

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

06/19/08 00:11:58 changed by 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.

07/30/08 20:39:22 changed by santosj

  • milestone changed from 2.5 to 2.9.