Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#50695 closed enhancement (fixed)

Make media_sideload_image extension list filterable

Reported by: paulschreiber's profile paulschreiber Owned by: johnbillion's profile johnbillion
Milestone: 5.6 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch commit has-dev-note
Focuses: Cc:


While upload_mimes is filterable and allows you to add additional items to the media library (i.e. SVGs), media_sideload_image() relies on an extension type check, and is not filterable.

Current behaviour:

preg_match( '/[^\?]+\.(jpe?g|jpe|gif|png)\b/i', $file, $matches );

Desired behaviour:

 * Filters the file extensions used for media sideload.
 * @since 5.5.0
 * @param array $extensions The original extension list.
$allowed_extensions =  apply_filters( 'media_sideload_extensions',  array( 'jpg', 'jpeg', 'jpe', 'png', 'gif' ) );
preg_match( '/[^\?]+\.(' . join( '|', allowed_extensions ) . ')\b/i', $file, $matches );

Change History (15)

This ticket was mentioned in PR #416 on WordPress/wordpress-develop by paulschreiber.

4 years ago

  • Keywords has-patch added

Makes media_sideload_image() extension list filterable.

Trac ticket:

#2 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.6

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.

4 years ago

#4 @johnbillion
4 years ago

This could benefit from some preg_quote() in case someone tries to get too clever with an asterisk.

#5 @hellofromTonya
4 years ago

  • Keywords needs-dev-note needs-unit-tests added

New filter added. This one needs-dev-note on the Misc Dev Note.

The extensions are passed through a filter with an expected array of extensions returned from it. Adding needs-unit-tests to add test data to valid the expected and unexpected filtered extensions.

A guard clause might be handy here too to handle the unexpected filtered content, such as a string of extensions or nothing being returned.

#6 @paulschreiber
4 years ago

How about this to clean up the list?

$allowed_extensions = array_filter(
        function( $item ) {
            return preg_replace( '/[^A-Z0-9]/i', '', $item );

if ( ! $allowed_extensions ) {
    return new WP_Error( 'image_sideload_failed', __( 'Invalid extension list.' ) );

This ticket was mentioned in Slack in #core-media by antpb. View the logs.

4 years ago

#8 @hellofromTonya
4 years ago

@johnbillion What do you think of what @paulschreiber is proposing here?

This ticket was mentioned in Slack in #core-media by hellofromtonya. View the logs.

4 years ago

#10 @johnbillion
4 years ago

  • Keywords commit added; needs-unit-tests removed

I've been trying to restructure this code so it's nicely testable, but there is so much code that's tightly coupled to actually downloading the file that it's turning into a rabbit hole.

I don't think there's a need to verify that the extension list is valid, this is not something that core typically does with other filters and if an invalid value is returned then it'll trigger a PHP warning, which is an appropriate means of showing an error for incorrect code.

I've just pushed a tweak to Paul's PR to improve the filter name, docs, and add the $file parameter for extra context, as well as preg_quote-ing the list.

#11 @hellofromTonya
4 years ago

Made a code suggestion to change join to implode to comply the coding standard fixes happening in ticket 50676.

#12 @johnbillion
4 years ago

  • Owner set to johnbillion
  • Resolution set to fixed
  • Status changed from new to closed

In 49198:

Media: Add an image_sideload_extensions filter to the list of allowed file extensions when sideloading an image from a URL.

Props paulschreiber, hellofromTonya

Fixes #50695

This ticket was mentioned in Slack in #core by daisyo. View the logs.

4 years ago

Note: See TracTickets for help on using tickets.