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

Assets URIs computed wrong if plugin is loaded from /vendor #489

Open
kadamwhite opened this issue Nov 1, 2022 · 3 comments
Open

Assets URIs computed wrong if plugin is loaded from /vendor #489

kadamwhite opened this issue Nov 1, 2022 · 3 comments

Comments

@kadamwhite
Copy link

kadamwhite commented Nov 1, 2022

If you have an application structure where this plugin gets installed into vendor due to a composer install override, the asset URLs do not resolve properly in subdomain multisite installs.

For example, if you have a WP site at mydomain.com/site-name/, then because the asset URIs are computed using plugins_url( '/path', __FILE ), you'll get a final asset URI like this:

mydomain.com/site-name/vendor/wordpress/two-factor/providers/js/fido-u2f-admin.js

instead of

mydomain.com/vendor/wordpress/two-factor/providers/js/fido-u2f-admin.js

with /vendor in the site root. This causes the 2FA form to fail to render, and you cannot set a provider.

This small plugin resolves the issue:

/**
 * Fix URL resolution for plugins_url when used on a file within the vendor directory.
 *
 * @param string $url    URL being returned for this plugin.
 * @param string $path   Path of resource within plugin.
 * @param string $plugin Plugin path.
 * @return string The filtered URL.
 */
function prefix_fix_two_factor_asset_uris( string $url, string $path, string $plugin ) {
	if ( strpos( $plugin, 'vendor/' ) === false ) {
		return $url;
	}
	$site_url = parse_url( get_site_url() );
	return str_replace( trailingslashit( $site_url['path'] ) . 'vendor', '/vendor', $url );
}
add_filter( 'plugins_url', 'prefix_fix_two_factor_asset_uris', 10, 3 );

I understand if installation to vendor/ is an edge case this plugin isn't interested in handling, but I wanted to report this here in case it's something y'all felt worth fixing.

@jeffpaul
Copy link
Member

jeffpaul commented Nov 1, 2022

Ensuring the plugin supports this case seems worthy to me, +1. I'm dropping into the Future Release milestone as there are some larger items targeted for a 0.8.0 and 0.9.0 releases, but if someone wants to work on a PR for this then we could look to pull into one of those two releases as feasible via code review/approvals.

@jeffpaul jeffpaul added this to the Future Release milestone Nov 1, 2022
@iandunn
Copy link
Member

iandunn commented Nov 2, 2022

plugins_url() is the conventional/official approach though, isn't it? Do other plugins do something different that doesn't conflict with vendor installs, or would you need the prefix_fix_two_factor_asset_uris() workaround for other plugins too?

IIRC what I've seen done before is configuring Composer to install plugins into wp-content/plugins/ instead.

@dd32
Copy link
Member

dd32 commented Nov 4, 2022

I agree with @iandunn here - It sounds like this is simply Core's behaviour when __FILE__ exists outside of ABSPATH and WP_CONTENT_DIR, which necessitates such hosting environments to have a filter on plugins_url (amongst others) to set the correct public URL for those files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants