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.

[SergeyBiryukov]

Possibly related: #15955

[solarissmoke]

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

[solarissmoke]

% 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.

[dd32]

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

[solarissmoke]

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?

[dd32]

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)

[phogberg]

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.

[simonwheatley]

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.

[simonwheatley]

Strip + AND % chars from uploaded filenames

[simonwheatley]

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

[dd32]

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.

[kawauso]

Related: #16330

[krembo99]

#16330

[jblifestyles]

Why hasn't this been implemented?

I'm running into issues importing images that had a % in their URL.. Simply adding it to the $special_chars fixed the problem, but I'm wondering why this hasn't made it into core... does this cause other conflicts?

[mordauk]

16226.patch is a refreshed patch that applies cleanly.

16226-tests.patch refreshes unit tests so they pass properly.

The only issue I see with this is that encoded spaces and other characters (such as %20) get their % removed but leave the rest. This means that multi %20 +space.png becomes multi-20-space.png.

[jblifestyles]

If someone is looking to address this issue temporarily while waiting on the patch to get integrated, and happen to sit on a server, like Flywheel, that doesn't let you edit wp-includes.... You can simply add this to your theme's functions.php file

add_filter ('sanitize_file_name', 'remove_percents_filter', 10 );
function remove_percents_filter( $filename ) {
    $filename = str_replace( '%', '', $filename );
    return $filename;
}

[ocean90]

#32673 was marked as a duplicate.

[ocean90]

#19447 was marked as a duplicate.

[wonderboymusic]

In 35122:

Fotmatting: in sanitize_file_name(), escape % when uploads contain them, otherwise attachment URLs will unescape the char and break.

Adds unit tests.

Props mordauk, simonwheatley, dd32, solarissmoke.
Fixes #16226.

[wonderboymusic]

In 35124:

After [35122], update test_wp_unique_filename() to reflect the change.

See #16226.