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

Font Library backend: can we use _wp_handle_upload() mime type validation ? #53576

Closed
matiasbenedetto opened this issue Aug 11, 2023 · 3 comments · Fixed by #53986
Closed

Font Library backend: can we use _wp_handle_upload() mime type validation ? #53576

matiasbenedetto opened this issue Aug 11, 2023 · 3 comments · Fixed by #53986
Assignees
Labels
[Feature] Typography Font and typography-related issues and PRs [Type] Enhancement A suggestion for improvement.

Comments

@matiasbenedetto
Copy link
Contributor

matiasbenedetto commented Aug 11, 2023

What ?

Could we use _wp_handle_upload() mime type validation in the Font Library backend ?

See:

$handled_file = wp_handle_upload( $file, $overrides );

$handled_file = wp_handle_upload( $file, $overrides );

_wp_handle_upload() documents a 'mimes' key for $overrides which gets passed to wp_check_filetype_and_ext(), so would something like this work?

'test_type' => true,
'mimes'     => array(
	'woff' => PHP_VERSION_ID >= 80112 ? 'font/woff' : 'application/font-woff',
	'and'  => 'so on',
),

Context

See this @costdev 's comment here:
#52704 (comment)

Tracking issue

Part of Fonts Library implementation effort:
#52698

@jordesign jordesign added [Type] Enhancement A suggestion for improvement. [Feature] Fonts API labels Aug 14, 2023
@hellofromtonya hellofromtonya changed the title Fonts Library backend: can we use _wp_handle_upload() mime type validation ? Font Library backend: can we use _wp_handle_upload() mime type validation ? Aug 17, 2023
@hellofromtonya hellofromtonya added [Feature] Typography Font and typography-related issues and PRs and removed [Feature] Fonts API labels Aug 17, 2023
@madhusudhand madhusudhand self-assigned this Aug 28, 2023
@madhusudhand
Copy link
Member

madhusudhand commented Aug 28, 2023

@matiasbenedetto @costdev

I looked into the issue and the check fails because of the following.

The function _wp_handle_upload makes a call to wp_check_filetype_and_ext to get file type and extension and then validates them against given mime types.

But the function wp_check_filetype_and_ext returns type and ext as false because the real_mime returned for the font files is application/octet-stream
following condition makes it fail as it doesn't fall under any of 'application', 'video', 'audio'

/*
* If $real_mime doesn't match the content type we're expecting from the file's extension,
* we need to do some additional vetting. Media types and those listed in $nonspecific_types are
* allowed some leeway, but anything else must exactly match the real content type.
*/
if ( in_array( $real_mime, $nonspecific_types, true ) ) {
	// File is a non-specific binary type. That's ok if it's a type that generally tends to be binary.
	if ( ! in_array( substr( $type, 0, strcspn( $type, '/' ) ), array( 'application', 'video', 'audio' ), true ) ) {
		$type = false;
		$ext  = false;
	}
}

WordPress core should add support for font in this array array( 'application', 'video', 'audio') to support mime check for fonts.

@matiasbenedetto
Copy link
Contributor Author

I think we should consider adding font to this array. Do you think it creates any issues?

Yes, that could work. I'm not aware of specific restrictions about that maybe @azaozz could add more input here.
Meanwhile, I think it could be good to submit a PR with the fix and discuss it there.

@madhusudhand
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Typography Font and typography-related issues and PRs [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants