Undefined index: extension in class-wp-image-editor.php

[JaworskiMatt]

Under some circumstances, 3rd party plugins will cause the WP_Image_Editor::generate_filename() to print a notice about a missing "extension" index in the $info variable.

It's rather hard to reproduce, but definitely happens inside PeepSo AJAX upload with some photos containing EXIF rotation information. It can also be found by simply googling it, like on this website:

​http://www.pasadenamusclecompany.com/tag/featured

Obviously it only happens with WP_DEBUG set to TRUE.

The $info variable is created using PHP's very own pathinfo() like this:

$info = pathinfo( $this->file );
$dir  = $info['dirname'];
$ext  = $info['extension'];

According to PHP documentation, the extension index can be missing (not set) if the file does not have one.

​http://php.net/manual/en/function.pathinfo.php

The code above will obviously generate a notice in such a scenario. An easy way around this is NOT to assume the "extension" index is present, for example like this:

$dir  = pathinfo( $this->file, PATHINFO_DIRNAME );
$ext  = pathinfo( $this->file, PATHINFO_EXTENSION );

As far as I can tell, there are no drawbacks to this approach.

[JaworskiMatt]

Might be somehow connected to #30052

[ocean90]

Introduced in [7041], moved to WP_Image_Editor::generate_filename() in [22094].

[JaworskiMatt]

Proposed solution

[JaworskiMatt]

I have added a .patch file containing a proposed solution

From what I can see in class-wp-image-editor.php there is at least one case of pathinfo() usage, but in that other case the index extension is being checked with isset()

[wonderboymusic]

this is a problem every time pathinfo() is called, see 37608.diff​

[wonderboymusic]

In 38294:

Media: when calling pathinfo(), also pass a PATHINFO_* constant to avoid array notices for unset keys.

Props JaworskiMatt.
Fixes #37608.

[jeremyfelt]

In [38294], PATHINFO_BASENAME was used in media_handle_upload() in a spot that had previously stripped the file extension from the basename to build an attachment title. This results in attachment titles of My Title-jpg instead of My Title for an uploaded image of My Title.jpg, which is a change in behavior.

37608.2.diff​ uses PATHINFO_FILENAME instead.

[joemcgill]

Replying to jeremyfelt:

In [38294], PATHINFO_BASENAME was used in media_handle_upload() in a spot that had previously stripped the file extension from the basename to build an attachment title. This results in attachment titles of My Title-jpg instead of My Title for an uploaded image of My Title.jpg, which is a change in behavior.

37608.2.diff​ uses PATHINFO_FILENAME instead.

Note that I've incorporated the same change into 37989.2.diff, so fixing #37989 will likely resolve this issue as well.

[JanR]

Today I noticed something relevant -I think- in the Media Library: strange behavior when a file name has a % sign in it. In a very extreme case, double extension it and name your file filename.html%00.png

On my dev site: after uploading, it has the following Attachment Details:

WordPress trunk

File name: filename.html00.png
File type: image/png
URL ​https://www.vanilla-wp.org/wp-content/uploads/2016/09/filename.html00.png
Title: filename-html%00-png

and the View attachment URL goes to about:blank in Chrome. The HTML code for the link has an additional /:

<a class="view-attachment" href="https://www.vanilla-wp.org/filename-html%00-png/">View attachment page</a>

Firefox does not have this behavior and the link is: https://www.vanilla-wp.org/filename-html%00-png/ resulting in a HTTP 400 Bad Request - Invalid URL error. (on IIS).

When I remove the double extension, the upload gives me an error message:

filename.html%00 This file type is not allowed. Please try another.

In WordPress 4.6.1 (had to remove this test URL):

File name: filename.html00.png
File type: image/png
URL ​http://www.vanilla-wp.org/test-4.6/wp-content/uploads/2016/09/filename.html00.png
Title: filename-html%00

After talking to @joemcgill on Slack, I tested WordPress trunk with https://core.trac.wordpress.org/attachment/ticket/37989/37989.3.diff applied:

File name: filename.html00.png
File type: image/png
URL ​https://www.vanilla-wp.org/wp-content/uploads/2016/09/filename.html00.png
Title: filename.html

The View attachment URL issue is fixed with the applied diff.

Now if you asked me what correct behavior is, I don't know. But I don't think it's a good idea to base the file type from it's last extension (in case of a double extension).

[SergeyBiryukov]

The issue with pathinfo() is that it depends on PHP locale and does not always work correctly with UTF-8 characters, leading to issues with truncated titles, see comment:40:ticket:37989.

We've encountered similar issues with basename() before, see #21217 and #23267.

[38294] needs a thorough test with multibyte file names. Some of the pathinfo() instances may need to be replaced with wp_basename(), which is the i18n-friendly version of basename().

[joemcgill]

In 38673:

Media: Use wp_basename() to create attachment titles from filenames.

In [38294], pathinfo() was used with the PATHINFO_BASENAME constant to
get the basename of the file to be used as an attachment title, which depends
on PHP locale and can cause issues with UTF-8 characters. This uses
wp_basename() instead, which is a more i18n-friendly version of basename().

Props SergeyBiryukov.
Fixes #37608, #37989.