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

Fix naive filtering of uploads directory. #6211

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
c6844b3
Test naive filtering of uploads directory.
peterwilsoncc Mar 3, 2024
b64bfae
Prevent infinite loop filtering fonts directory. Props @costdev.
peterwilsoncc Mar 3, 2024
c1edfba
Restore naive filtering to rest end point.
peterwilsoncc Mar 3, 2024
a866973
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 5, 2024
19cc1c7
Thought bubble…
peterwilsoncc Mar 5, 2024
a8f9edf
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 6, 2024
d8dfbf5
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 7, 2024
a10dbe4
Improve comment.
peterwilsoncc Mar 7, 2024
2b21c6d
Move the font directory filter registration.
peterwilsoncc Mar 7, 2024
ba51db2
Remove cache option: N/A for fonts.
peterwilsoncc Mar 7, 2024
d0ef78e
Docblocks.
peterwilsoncc Mar 7, 2024
3277002
Improve comment.
peterwilsoncc Mar 7, 2024
db876de
More docblocks.
peterwilsoncc Mar 7, 2024
cec16c6
Use static for closure.
peterwilsoncc Mar 7, 2024
94c576b
Revert "Use static for closure."
peterwilsoncc Mar 7, 2024
eff1763
Font uploads.
peterwilsoncc Mar 8, 2024
48aca49
Dockblock improvements
peterwilsoncc Mar 10, 2024
606aeae
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 10, 2024
5117eef
Avoid duplicate assertions testing for loop.
peterwilsoncc Mar 10, 2024
4b76a9c
Doc fix.: this not the.
peterwilsoncc Mar 10, 2024
addadc3
Font upload, not upload dir.
peterwilsoncc Mar 10, 2024
c1ac4e3
Trolling?
peterwilsoncc Mar 10, 2024
3221610
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 11, 2024
1c870cc
Rename function.
peterwilsoncc Mar 11, 2024
1c3b7ac
Filter the font directory late.
peterwilsoncc Mar 11, 2024
fb2cfc3
Document late running hook.
peterwilsoncc Mar 11, 2024
607a73a
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 11, 2024
41bbeb7
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 12, 2024
52103f6
Rename function to use plural filters.
peterwilsoncc Mar 12, 2024
21673e5
Remove a a typo.
peterwilsoncc Mar 13, 2024
bf2c2fd
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 13, 2024
1183132
Run filter at default priority.
peterwilsoncc Mar 13, 2024
f2d5a77
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 15, 2024
0204b99
Remove out of date comment.
peterwilsoncc Mar 15, 2024
9071275
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 18, 2024
022941e
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 21, 2024
eccdbac
Merge default getter and filtering function. Rename.
peterwilsoncc Mar 21, 2024
f9e41b2
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 21, 2024
22764e3
Rename function. Props @swissspidy.
peterwilsoncc Mar 21, 2024
cf211ae
Document use of filter.
peterwilsoncc Mar 21, 2024
35f8e1b
Add ticket reference
swissspidy Mar 22, 2024
ed46c1d
Merge branch 'trunk' into fix/font-filter-infinite-loop
peterwilsoncc Mar 22, 2024
7721163
Merge branch 'fix/font-filter-infinite-loop' of github.com:peterwilso…
peterwilsoncc Mar 22, 2024
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
80 changes: 74 additions & 6 deletions src/wp-includes/fonts.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,31 @@ function wp_unregister_font_collection( string $slug ) {
return WP_Font_Library::get_instance()->unregister_font_collection( $slug );
}

/**
* Retrieves font uploads directory information.
*
* Same as wp_font_dir() but "light weight" as it doesn't attempt to create the font uploads directory.
* Intended for use in themes, when only 'basedir' and 'baseurl' are needed, generally in all cases
* when not uploading files.
*
* @since 6.5.0
*
* @see wp_font_dir()
*
* @return array See wp_font_dir() for description.
*/
function wp_get_font_dir() {
return wp_font_dir( false );
}

/**
* Returns an array containing the current fonts upload directory's path and URL.
*
* @since 6.5.0
*
* @return array $defaults {
* Array of information about the upload directory.
* @param bool $create_dir Optional. Whether to check and create the font uploads directory. Default true.
* @return array {
* Array of information about the font upload directory.
*
* @type string $path Base directory and subdirectory or full path to the fonts upload directory.
* @type string $url Base URL and subdirectory or absolute URL to the fonts upload directory.
Expand All @@ -107,13 +125,54 @@ function wp_unregister_font_collection( string $slug ) {
* @type string|false $error False or error message.
* }
*/
function wp_get_font_dir() {
function wp_font_dir( $create_dir = true ) {
/*
* Allow extenders to manipulate the font directory consistently.
*
* Ensures the upload_dir filter is fired both when calling this function
* directly and when the upload directory is filtered in the Font Face
* REST API endpoint.
*/
add_filter( 'upload_dir', '_wp_filter_font_directory' );
$font_dir = wp_upload_dir( null, $create_dir, false );
remove_filter( 'upload_dir', '_wp_filter_font_directory' );
return $font_dir;
}

/**
* Returns the font directory for use by the font library.
*
* This function is a callback for the {@see 'upload_dir'} filter. It is not
* intended to be called directly. Use wp_get_font_dir() instead.
*
* The function can be used when extending the font library to modify the upload
* destination for font files via the upload_dir filter. The recommended way to
* do this is:
*
* ```php
* add_filter( 'upload_dir', '_wp_filter_font_directory' );
* // Your code to upload or sideload a font file.
* remove_filter( 'upload_dir', '_wp_filter_font_directory' );
* ```
*
* @since 6.5.0
* @access private
*
* @param string $font_dir The font directory.
* @return string The modified font directory.
*/
function _wp_filter_font_directory( $font_dir ) {
if ( doing_filter( 'font_dir' ) ) {
// Avoid an infinite loop.
return $font_dir;
}

$site_path = '';
if ( is_multisite() && ! ( is_main_network() && is_main_site() ) ) {
$site_path = '/sites/' . get_current_blog_id();
}

$defaults = array(
$font_dir = array(
'path' => path_join( WP_CONTENT_DIR, 'fonts' ) . $site_path,
'url' => untrailingslashit( content_url( 'fonts' ) ) . $site_path,
'subdir' => '',
Expand All @@ -129,9 +188,18 @@ function wp_get_font_dir() {
*
* @since 6.5.0
*
* @param array $defaults The original fonts directory data.
* @param array $font_dir {
* Array of information about the font upload directory.
*
* @type string $path Base directory and subdirectory or full path to the fonts upload directory.
* @type string $url Base URL and subdirectory or absolute URL to the fonts upload directory.
* @type string $subdir Subdirectory
* @type string $basedir Path without subdir.
* @type string $baseurl URL path without subdir.
* @type string|false $error False or error message.
* }
*/
return apply_filters( 'font_dir', $defaults );
return apply_filters( 'font_dir', $font_dir );
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -856,21 +856,8 @@ protected function sanitize_src( $value ) {
*/
protected function handle_font_file_upload( $file ) {
add_filter( 'upload_mimes', array( 'WP_Font_Utils', 'get_allowed_font_mime_types' ) );

/*
* Set the upload directory to the fonts directory.
*
* wp_get_font_dir() contains the 'font_dir' hook, whose callbacks are
* likely to call wp_get_upload_dir().
*
* To avoid an infinite loop, don't hook wp_get_font_dir() to 'upload_dir'.
* Instead, just pass its return value to the 'upload_dir' callback.
*/
$font_dir = wp_get_font_dir();
$set_upload_dir = function () use ( $font_dir ) {
return $font_dir;
};
add_filter( 'upload_dir', $set_upload_dir );
// Filter the upload directory to return the fonts directory.
add_filter( 'upload_dir', '_wp_filter_font_directory' );

$overrides = array(
'upload_error_handler' => array( $this, 'handle_font_file_upload_error' ),
Expand All @@ -887,7 +874,7 @@ protected function handle_font_file_upload( $file ) {

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

remove_filter( 'upload_dir', $set_upload_dir );
remove_filter( 'upload_dir', '_wp_filter_font_directory' );
remove_filter( 'upload_mimes', array( 'WP_Font_Utils', 'get_allowed_font_mime_types' ) );

return $uploaded_file;
Expand Down
4 changes: 2 additions & 2 deletions tests/phpunit/tests/fonts/font-library/fontLibraryHooks.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ protected function upload_font_file( $font_filename ) {
$font_file_path = DIR_TESTDATA . '/fonts/' . $font_filename;

add_filter( 'upload_mimes', array( 'WP_Font_Utils', 'get_allowed_font_mime_types' ) );
add_filter( 'upload_dir', 'wp_get_font_dir' );
add_filter( 'upload_dir', '_wp_filter_font_directory' );
$font_file = wp_upload_bits(
$font_filename,
null,
file_get_contents( $font_file_path )
);
remove_filter( 'upload_dir', 'wp_get_font_dir' );
remove_filter( 'upload_dir', '_wp_filter_font_directory' );
remove_filter( 'upload_mimes', array( 'WP_Font_Utils', 'get_allowed_font_mime_types' ) );

return $font_file;
Expand Down
40 changes: 40 additions & 0 deletions tests/phpunit/tests/fonts/font-library/wpFontsDir.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,44 @@ function set_new_values( $defaults ) {

$this->assertSame( static::$dir_defaults, $font_dir, 'The wp_get_font_dir() method should return the default values.' );
}

/**
* @ticket 60652
*/
public function test_fonts_dir_filters_do_not_trigger_infinite_loop() {
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
/*
* Naive filtering of uploads directory to return font directory.
*
* This emulates the approach a plugin developer may take to
* add the filter when extending the font library functionality.
*/
add_filter( 'upload_dir', '_wp_filter_font_directory' );

add_filter(
'upload_dir',
function ( $upload_dir ) {
peterwilsoncc marked this conversation as resolved.
Show resolved Hide resolved
static $count = 0;
++$count;
// The filter may be applied a couple of times, at five iterations assume an infinite loop.
if ( $count >= 5 ) {
$this->fail( 'Filtering the uploads directory triggered an infinite loop.' );
}
return $upload_dir;
},
5
);

/*
* Filter the font directory to return the uploads directory.
*
* This emulates moving font files back to the uploads directory due
* to file system structure.
*/
add_filter( 'font_dir', 'wp_get_upload_dir' );

wp_get_upload_dir();

// This will never be hit if an infinite loop is triggered.
$this->assertTrue( true );
}
}
Loading