Unable to upload images with URL over API when the image doesn’t have a filetype in the filename

[masteradhoc]

Problem: It’s not possible to upload images with URL per API when the image doesn’t have a filetype in the filename.

Elaboration: There are some websites which don’t put filetypes inside the URL for images. For example: ​https://img.ricardostatic.ch/t_1800x1350/pl/1187743940/3/1/

Or: ​https://media.istockphoto.com/photos/coppersmith-repair-copper-kettle-on-fire-in-kashgar-in-xinjiang-picture-id1298102169?s=612x612

The format of the image is set and recognized by the browser with the MIME-Type. In theory WordPress could handle this, but unfortunately it never gets to the point where the correct MIME type is read out of the image signature.

Reason: At the function wp_check_filetype type will always be defined as false - if there is no filetype set inside the filename.

Call of the function: ​https://github.com/WordPress/WordPress/blob/266c58518846201a7e98cd7995ce2c7429caf1db/wp-includes/functions.php#L2984

Further down there would be a function to get back the real mime type from the signature of the image. However, the function is never called since type at this point is always false in that give case:

​https://github.com/WordPress/WordPress/blob/266c58518846201a7e98cd7995ce2c7429caf1db/wp-includes/functions.php#L2999

Possible Bug: If the filename has no filetype the function wp_get_image_mime should always be called to get the type from the signature of the image.

Following Bug: After changing this, the filename at the upload should be extended with the filetype: ​https://github.com/WordPress/WordPress/blob/266c58518846201a7e98cd7995ce2c7429caf1db/wp-admin/includes/file.php#L924

Since it’s only really showing up in the library with the correct filetype.

[mitogh]

Adding the steps to replicate the problem using functions either directly on a script or via WP shell.

$result = media_sideload_image( 'https://img.ricardostatic.ch/t_1800x1350/pl/1187743940/3/1/' );


// This would be an error because file would use a .tmp extension which is not valid to be uplodaed.
var_dump( $result );

Mainly the problem is because the header content is not considered as part of the mime type instead the extension of the file is used to indicate which mime type is.

Edit 9/4/2024: originally reproduction function calls were incorrect.

When an image or a URL is downloaded using WordPress function download_url and if the provided URL does not contain the file extension, instead of using tmp try to guess the extension of the file by using the provided mime type of the file.

An important consideration is to make sure the mime type is supported by the current WordPress installation to prevent download and upload of non supported mime types on the current installation.

Trac ticket: https://core.trac.wordpress.org/ticket/54738

[hellofromTonya]

From today's scrub, I'm updating the keywords to help it make actionable to move forward towards resolution:

• needs-refresh: ​the patch has merge conflicts, thus needs a refresh against the latest trunk.
• needs-testing: test reports are needed once the patch is refreshed.
• Code review of the patch is needed.

Given there's a patch (though it needs a refresh), moving the ticket into 6.5 for visibility, i.e. to help it move forward.

[antpb]

Leaving a note here after some discussion in the recent Media Meeting. We are moving this to 6.6 to allow some time to have review with respect to any security implications this may introduce. Nothing yet identified, just want to be extra cautious changing how we validate files and assume mime types.

[antpb]

Moving this to 6.7 as it still needs work and investigation

[joedolson]

This seems like it's in good shape. The comments from @dmsnell are mostly about clarification, and worth looking at to improve the understandability of the flow, but nothing blocking. I'll work on getting this over the finish line.

[masteradhoc]

Thanks @joedolson for working further on the patch from @mitogh. Happy to help on retesting this again if needed.