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.

Sign up for GitHub

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: add mime type validation for font uploads #53986

Merged
merged 9 commits into from
Sep 19, 2023
Merged

Font Library: add mime type validation for font uploads #53986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ec84551
add mime type validation for font uploads
madhusudhand Aug 28, 2023
fd420aa
fix unit tests
madhusudhand Sep 13, 2023
8c8e8c6
adding ttf mime type for PHP 7.3
matiasbenedetto Sep 14, 2023
3777f23
adding ttf mime types for PHP 7
matiasbenedetto Sep 14, 2023
3835728
fix constant reference
matiasbenedetto Sep 14, 2023
4813ea3
testing version for php8
matiasbenedetto Sep 14, 2023
2298ce3
format
matiasbenedetto Sep 14, 2023
2a0cbc9
add unit tests for woff, woff2 and otf font types
madhusudhand Sep 19, 2023
67ab0b2
fix unit tests
madhusudhand Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/experimental/fonts/font-library/class-wp-font-family.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ private function get_upload_overrides( $filename ) {
'test_form' => false,
// Seems mime type for files that are not images cannot be tested.
// See wp_check_filetype_and_ext().
'test_type' => false,
'test_type' => true,
'mimes' => WP_Font_Library::ALLOWED_FONT_MIME_TYPES,
'unique_filename_callback' => static function () use ( $filename ) {
// Keep the original filename.
return $filename;
Expand Down Expand Up @@ -541,6 +542,7 @@ private function create_or_update_font_post() {
* @return array|WP_Error An array of font family data on success, WP_Error otherwise.
*/
public function install( $files = null ) {
add_filter( 'upload_mimes', array( 'WP_Font_Library', 'set_allowed_mime_types' ) );
add_filter( 'upload_dir', array( 'WP_Font_Library', 'set_upload_dir' ) );
$were_assets_written = $this->download_or_move_font_faces( $files );
remove_filter( 'upload_dir', array( 'WP_Font_Library', 'set_upload_dir' ) );
Comment on lines +545 to 548
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove the filter as we do with the upload_dir? I would believe that yes, because otherwise, we would be enabling that globally and that's not the intention I guess.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried removing that and the mime type function is no longer working. I'm not sure that we can.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that we should bring this in as it stands. Testing showed we may need the global filter, but the local filter (get_upload_overrides()) maybe not.

Either way I believe this should be brought in and we can work out if it makes sense to pare it down.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, seems good to go. If the mimes in get_upload_overrides() are not necessary, we could remove them, right? Could you add a follow-up issue to remind us to do that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Removed it here #54647

Expand Down
20 changes: 17 additions & 3 deletions lib/experimental/fonts/font-library/class-wp-font-library.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@
*/
class WP_Font_Library {

const PHP_7_TTF_MIME_TYPE = PHP_VERSION_ID >= 70300 ? 'application/font-sfnt' : 'application/x-font-ttf';

const ALLOWED_FONT_MIME_TYPES = array(
'otf' => 'font/otf',
'ttf' => 'font/ttf',
'woff' => 'font/woff',
'woff2' => 'font/woff2',
'ttf' => PHP_VERSION_ID >= 70400 ? 'font/sfnt' : self::PHP_7_TTF_MIME_TYPE,
'woff' => PHP_VERSION_ID >= 80100 ? 'font/woff' : 'application/font-woff',
'woff2' => PHP_VERSION_ID >= 80100 ? 'font/woff2' : 'application/font-woff2',
);

/**
Expand Down Expand Up @@ -118,4 +120,16 @@ public static function set_upload_dir( $defaults ) {

return $defaults;
}

/**
* Sets the allowed mime types for fonts.
*
* @since 6.4.0
*
* @param array $mime_types List of allowed mime types.
* @return array Modified upload directory.
*/
public static function set_allowed_mime_types( $mime_types ) {
return array_merge( $mime_types, self::ALLOWED_FONT_MIME_TYPES );
}
}
Binary file added phpunit/tests/data/fonts/DMSans.woff2
View file
Open in desktop
Binary file not shown.
Binary file added phpunit/tests/data/fonts/Merriweather.ttf
View file
Open in desktop
Binary file not shown.
Binary file added phpunit/tests/data/fonts/cooper-hewitt.woff
View file
Open in desktop
Binary file not shown.
2 changes: 1 addition & 1 deletion phpunit/tests/fonts/font-library/wpFontFamily/base.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function set_up() {
parent::set_up();

$merriweather_tmp_name = wp_tempnam( 'Merriweather-' );
file_put_contents( $merriweather_tmp_name, 'Mocking file content' );
copy( __DIR__ . '/../../../data/fonts/Merriweather.ttf', $merriweather_tmp_name );
$this->merriweather = array(
'font_data' => array(
'name' => 'Merriweather',
Expand Down
65 changes: 62 additions & 3 deletions phpunit/tests/fonts/font-library/wpFontFamily/install.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,13 @@ public function data_should_download_fontfaces() {
public function test_should_move_local_fontfaces( $font_data, array $files_data, array $expected ) {
// Set up the temporary files.
foreach ( $files_data as $file ) {
file_put_contents( $file['tmp_name'], 'Mocking file content' );
if ( 'font/ttf' === $file['type'] ) {
copy( __DIR__ . '/../../../data/fonts/Merriweather.ttf', $file['tmp_name'] );
} elseif ( 'font/woff' === $file['type'] ) {
copy( __DIR__ . '/../../../data/fonts/cooper-hewitt.woff', $file['tmp_name'] );
} elseif ( 'font/woff2' === $file['type'] ) {
copy( __DIR__ . '/../../../data/fonts/DMSans.woff2', $file['tmp_name'] );
}
}

$font = new WP_Font_Family( $font_data );
Expand All @@ -180,7 +186,8 @@ public function test_should_move_local_fontfaces( $font_data, array $files_data,
*/
public function data_should_move_local_fontfaces() {
return array(
'1 local font' => array(
// ttf font type.
'1 local font' => array(
'font_data' => array(
'name' => 'Inter',
'slug' => 'inter',
Expand All @@ -205,7 +212,7 @@ public function data_should_move_local_fontfaces() {
),
'expected' => array( 'inter_italic_900.ttf' ),
),
'2 local fonts' => array(
'2 local fonts' => array(
'font_data' => array(
'name' => 'Lato',
'slug' => 'lato',
Expand Down Expand Up @@ -243,6 +250,58 @@ public function data_should_move_local_fontfaces() {
),
'expected' => array( 'lato_normal_400.ttf', 'lato_normal_500.ttf' ),
),
// woff font type.
'woff local font' => array(
'font_data' => array(
'name' => 'Cooper Hewitt',
'slug' => 'cooper-hewitt',
'fontFamily' => 'Cooper Hewitt',
'fontFace' => array(
array(
'fontFamily' => 'Cooper Hewitt',
'fontStyle' => 'italic',
'fontWeight' => '900',
'uploadedFile' => 'files0',
),
),
),
'files_data' => array(
'files0' => array(
'name' => 'cooper-hewitt.woff',
'type' => 'font/woff',
'tmp_name' => wp_tempnam( 'Cooper-' ),
'error' => 0,
'size' => 123,
),
),
'expected' => array( 'cooper-hewitt_italic_900.woff' ),
),
// woff2 font type.
'woff2 local font' => array(
'font_data' => array(
'name' => 'DM Sans',
'slug' => 'dm-sans',
'fontFamily' => 'DM Sans',
'fontFace' => array(
array(
'fontFamily' => 'DM Sans',
'fontStyle' => 'regular',
'fontWeight' => '500',
'uploadedFile' => 'files0',
),
),
),
'files_data' => array(
'files0' => array(
'name' => 'DMSans.woff2',
'type' => 'font/woff2',
'tmp_name' => wp_tempnam( 'DMSans-' ),
'error' => 0,
'size' => 123,
),
),
'expected' => array( 'dm-sans_regular_500.woff2' ),
),
);
}
}
4 changes: 2 additions & 2 deletions phpunit/tests/fonts/font-library/wpFontFamily/uninstall.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public function data_should_return_error_when_not_able_to_uninstall() {
public function test_should_uninstall( $font_data, array $files_data ) {
// Set up.
foreach ( $files_data as $file ) {
file_put_contents( $file['tmp_name'], 'Mocking file content' );
copy( __DIR__ . '/../../../data/fonts/Merriweather.ttf', $file['tmp_name'] );
}
$font = new WP_Font_Family( $font_data );
$font->install( $files_data );
Expand Down Expand Up @@ -103,7 +103,7 @@ public function test_should_uninstall_only_its_font_family( $font_data, array $f

// Set up the font family to be uninstalled.
foreach ( $files_data as $file ) {
file_put_contents( $file['tmp_name'], 'Mocking file content' );
copy( __DIR__ . '/../../../data/fonts/Merriweather.ttf', $file['tmp_name'] );
}
$font = new WP_Font_Family( $font_data );
$font->install( $files_data );
Expand Down
5 changes: 4 additions & 1 deletion phpunit/tests/fonts/font-library/wpRestFontLibraryController/installFonts.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,13 @@ public function test_install_fonts( $font_families, $files, $expected_response )
if ( isset( $installed_font['fontFace'] ) || isset( $expected_font['fontFace'] ) ) {
for ( $face_index = 0; $face_index < count( $installed_font['fontFace'] ); $face_index++ ) {
// Checks that the font asset were created correctly.
$this->assertStringEndsWith( $expected_font['fontFace'][ $face_index ]['src'], $installed_font['fontFace'][ $face_index ]['src'], 'The src of the fonts were not updated as expected.' );
if ( isset( $installed_font['fontFace'][ $face_index ]['src'] ) ) {
$this->assertStringEndsWith( $expected_font['fontFace'][ $face_index ]['src'], $installed_font['fontFace'][ $face_index ]['src'], 'The src of the fonts were not updated as expected.' );
}
// Removes the src from the response to compare the rest of the data.
unset( $installed_font['fontFace'][ $face_index ]['src'] );
unset( $expected_font['fontFace'][ $face_index ]['src'] );
unset( $installed_font['fontFace'][ $face_index ]['uploadedFile'] );
}
}

Expand Down
Loading