Attachment URL filenames are not urlencoded

[mdawaffe]

In looking at #16191, I discovered a related bug.

Upload a file called a%22b.jpg. The file will be stored on the filesystem as a%22b.jpg, but the resulting attachment URL will be ​http://example.com/wp-content/uploads/2011/01/a%22b.jpg, which will point to a file named a"b.jpg.

Possibly related: #15955

Don't allow % in filenames as it will cause false urlencoding

% should be added to the list of special characters. Browsers will treat it (and what follows) as a urlencoded entity, which means you will never be able to access the saved file.

shouldn't % be encoded to %25 instead? ie. the filename should be urlencoded.

Replying to dd32:

shouldn't % be encoded to %25 instead? ie. the filename should be urlencoded.

The difficulty is urlencoding only the filename portion... we can't urlencode the whole url. And we can't do the encoding in sanitize_file_name() because that is used in other non-URL contexts. Is there some other place where it can be done?

See the attached patch, it's a rough patch, but seems to work in the few test cases i've got here. I've not tested it against international characters, nor have i tested it against "store uploads in year/month folders" being disabled (which will probably break this patch)

Similar to this:

Upload a file called a+b.jpg. The file will be stored on the filesystem as a+b.jpg, but the resulting attachment URL will be "​http://example.com/wp-content/uploads/2011/04/a+b.jpg", which will point to a file named "a b.jpg" wich does not exist.

The plus sign translates to a space in an urlencoded string. 16226.2.diff will probably solve this as well, but not patch 16226.diff.

It seems to me that the simplest thing to do for these edge cases (albeit that I've just had a real user complaining about a file with a "+" in the filename not uploading) is to strip "+" and "%" in the sanitise_file_name function. I'm attaching a patch which does just this.

Strip + AND % chars from uploaded filenames

Note that this simplistic approach would also work for #16191. :)

The Version field is used to track the first version in which a issue is identified in, You can assume that any version between Version and Milestone will be affected by the ticket, so no need to update it to the current branch.

Related: #16330

#16330