[wp-trac] [WordPress Trac] #22813: Media Uploader doesn't escape "+" in filenames and doesn't upload file

[WordPress Trac]

#22813: Media Uploader doesn't escape "+" in filenames and doesn't upload file
----------------------------------------+------------------------------
 Reporter:  devinreams                  |       Owner:
     Type:  defect (bug)                |      Status:  new
 Priority:  normal                      |   Milestone:  Awaiting Review
Component:  Media                       |     Version:  3.4.2
 Severity:  normal                      |  Resolution:
 Keywords:  has-patch needs-unit-tests  |
----------------------------------------+------------------------------

Comment (by jamescollins):

 Thanks nacin.

 This bug is a problem for WordPress multisite installations that use ms-
 files.php rewriting when using Apache. Nginx seems to work fine according
 to feedback above.

 By default WordPress Multisite with ms-files rewriting uses this rule in
 the .htaccess file:

 {{{RewriteRule ^files/(.+) wp-includes/ms-files.php?file=$1 [L]}}}

 But doing that, ms-files.php receives a {{{$_GET[ 'file' ]}}} value that
 uses a space instead of the {{{+}}} sign.

 There are two potential ways to fix this so that ms-files.php rewriting in
 multisite works for files with + characters in them:

 1. Use the [http://httpd.apache.org/docs/2.2/rewrite/flags.html#flag_b B
 flag] in .htaccess, which would mean .htaccess would need to use:

 {{{RewriteRule ^files/(.+) wp-includes/ms-files.php?file=$1 [L,B]}}}

 The problem with this is that the B flag is only available in Apache 2.2.7
 and newer. It would also be difficult to get existing installs to update
 their .htaccess files.

 2. In ms-files.php, use {{{urlencode( $_GET[ 'file' ] )}}} instead of
 {{{$_GET[ 'file' ])}}} on
 [http://core.trac.wordpress.org/browser/trunk/src/wp-includes/ms-
 files.php#L26 line 26].

 The third (and probably less risky alternative) is to go with the existing
 patch which makes sure future uploaded files can't contain the {{{+}}}
 character.


 If you know of a way we could write unit tests for any (or all) of this,
 then I'd love to know. As far as I can tell, the Multisite unit tests sets
 up a multisite instance that doesn't use ms-files.php rewriting, so we'd
 have to add a way to do that, and then possibly some unit tests that
 uploads a test file to the WordPress media library and then performs a
 wp_remote_get() request on it to make sure it gets a HTTP 200 (not 400)
 response.


 Interestingly, I just tried uploading a file called {{{wordpress logo test
 file which contains a + character.png}}} to wordpress.com, and it was
 renamed to {{{wordpress-logo-test-file-which-contains-a-character.png}}}
 (ie the {{{+}}} character was removed).'''

--
Ticket URL: <http://core.trac.wordpress.org/ticket/22813#comment:15>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software