wp_get_font_dir(): Bail if the 'font_dir' filter is already running.

[matiasbenedetto]

What?

wp_get_font_dir(): Bail if the font_dir filter is already running.

Props to @costdev for proposing this solution.

Why?

This avoids an infinite loop that can occur if wp_upload_dir() is called inside a 'font_dir' callback.

Testing instructions

1. Use wp_get_upload_dir() inside a function used to filter upload_dir as in the following example:

 function alter_wp_fonts_dir( $defaults ) {
	$wp_upload_dir = wp_get_upload_dir();
	$uploads_basedir = $wp_upload_dir['basedir'];
	$uploads_baseurl = $wp_upload_dir['baseurl'];

	$fonts_dir = $uploads_basedir . '/fonts';
	// Generate the URL for the fonts directory from the font dir.
	$fonts_url = str_replace( $uploads_basedir, $uploads_baseurl, $fonts_dir );

	$defaults['path'] = $fonts_dir;
	$defaults['url']  = $fonts_url;

	return $defaults;
}
add_filter( 'font_dir', 'alter_wp_fonts_dir' );

2. Upload fonts using the font library
3. Check that the fonsts were successfully uploaded to the path set by the filter.
   (wp-content/uploads/fonts in the example provided).

---

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

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props mmaattiiaass, costdev, swissspidy.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[swissspidy]

The one thing I don't like about wp_get_upload_dir() is that it seems like a function that simply returns a directory name. But no, it is actually used for filtering the upload dir.

Things would be much easier if it would behave more like e.g. wp_privacy_exports_dir(), or if we split this up:

• 1 function to filter the uploads dir
• 1 function wp_get_upload_dir() that returns the desired location and itself has a filter that hosts/devs can use

[matiasbenedetto]

Things would be much easier if it would behave more like e.g. wp_privacy_exports_dir(), or if we split this up:
1 function to filter the uploads dir
1 function wp_get_upload_dir() that returns the desired location and itself has a filter that hosts/devs can use

@swissspidy I' don't see how wp_privacy_exports_dir() is different to the trunk version of wp_get_font_dir(). They seem to be doing exactly the same: setting a default and applying a filter after that. Here are the 2 implementations to compare. I removed the comments to make them shorter:

function wp_privacy_exports_dir() {
	$upload_dir  = wp_upload_dir();
	$exports_dir = trailingslashit( $upload_dir['basedir'] ) . 'wp-personal-data-exports/';
	return apply_filters( 'wp_privacy_exports_dir', $exports_dir );
}

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

	// Sets the defaults.
	$defaults['path']    = path_join( WP_CONTENT_DIR, 'fonts' ) . $site_path;
	$defaults['url']     = untrailingslashit( content_url( 'fonts' ) ) . $site_path;
	$defaults['subdir']  = '';
	$defaults['basedir'] = path_join( WP_CONTENT_DIR, 'fonts' ) . $site_path;
	$defaults['baseurl'] = untrailingslashit( content_url( 'fonts' ) ) . $site_path;
	$defaults['error']   = false;
	
	return apply_filters( 'font_dir', $defaults );
}

I think wp_privacy_exports_dir() is not entering an infinite loop because it's not used to filter upload_dir as the font library does. Otherwise, the same problem would arise.

wordpress-develop/src/wp-includes/rest-api/endpoints/class-wp-rest-font-faces-controller.php

Line 859 in a69052a

add_filter( 'upload_dir', 'wp_get_font_dir' );

@swissspidy @costdev What do you think about this alternative WordPress/gutenberg#58839 to avoid the infinite loop this PR is trying to fix?

[swissspidy]

What do you think about this alternative WordPress/gutenberg#58839 to avoid the infinite loop this PR is trying to fix?

This is exactly what I suggested in my comment :)

I' don't see how wp_privacy_exports_dir() is different to the trunk version of wp_get_font_dir().
I think wp_privacy_exports_dir() is not entering an infinite loop because it's not used to filter upload_dir as the font library does.

That's exactly what I said is confusing :)

[costdev]

Yeah, the alternative looks better to me, nice!

[matiasbenedetto]

@swissspidy @costdev, if the alternative approach is looking better, could you please add a review in that PR?

[matiasbenedetto]

Closing this PR in favor of: #6200