Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check mime type exists before performing regex #1709

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

KTS915
Copy link
Member

@KTS915 KTS915 commented Jan 3, 2025

The function wp_match_mime_types( $wildcard_mime_types, $real_mime_types ) in ~wp-includes/post.php includes a foreach loop that runs a regex on each uploaded media's mime type. The problem is that sometimes CP does not recognize the mime type, which results in the array, $real_mime_types, including some entries of false. When preg_match is run in line 3019, every false value for $real generates a warning.

This PR addresses this issue by checking that every instance of $real has a truthy value.

@KTS915 KTS915 added the type: bug Something isn't working label Jan 3, 2025
@KTS915 KTS915 added this to the 2.4 milestone Jan 3, 2025
@mattyrob
Copy link
Collaborator

mattyrob commented Jan 4, 2025

Could a potential cause of this bug be from this line:

$matches = wp_match_mime_types( array_keys( $post_mime_types ), array_keys( $_num_posts ) );

This call to wp_match_mime_types() is not passing a mime type as the second function parameter.

@KTS915
Copy link
Member Author

KTS915 commented Jan 4, 2025

No, I don't think that's cause. I don't think the code in and around that function is loaded on wp-admin/upload.php, which is where I'm seeing this happen.

@mattyrob
Copy link
Collaborator

mattyrob commented Jan 5, 2025

I've done a little more checking in the code and that line I linked to matches upstream so probably not the cause of a problem.

I'm sure I've seen comments in the code that PHP is not 100% reliable at extracting mime type. Do you have an example file that creates this issue? Your proposed fix looks reasonable but I'd like to see if we can identify the cause earlier in the chain of events if possible.

@KTS915
Copy link
Member Author

KTS915 commented Jan 5, 2025

A very recent thread on the inaccuracy of mime type detection is at https://forums.phpfreaks.com/topic/318512-problems-getting-mime-type-of-files/

I have quite a few SVGs that I have created from rasterized images using Inkscape. CP never recognizes their mime types. Here's one:

cheesecake

@mattyrob
Copy link
Collaborator

mattyrob commented Jan 5, 2025

How are you enabling SVG upload to the Media Library? This is working locally for me, recognised as image/svg+xml, with the following code snippet to allow SVG uploads:

function allow_svg_upload( $mimes ) {
	$mimes['svg']  = 'image/svg+xml';
	$mimes['svgz'] = 'image/svg+xml';
	return $mimes;
}
add_filter( 'upload_mimes', 'allow_svg_upload' );

@KTS915
Copy link
Member Author

KTS915 commented Jan 5, 2025

I'm using a plugin. It was Safe SVG before, now SVG Support.

@mattyrob
Copy link
Collaborator

mattyrob commented Jan 5, 2025

I'm using a plugin. It was Safe SVG before, now SVG Support.

Strange one this, I've use my snippet and the SVG Support plugin and both allow upload of the SVG image you linked without anything logged in debug.log or shown on screen as an error. I'll try again next week.

@xxsimoxx
Copy link
Member

xxsimoxx commented Jan 8, 2025

I can't reproduce it either.

@mattyrob mattyrob removed this from the 2.4 milestone Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants