WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 2 years ago

#19388 new enhancement

plugin_basename returns full directory in URL

Reported by: damianzaremba Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.3
Component: Plugins Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

If a trailing slash is specified on either then WP_PLUGIN_DIR or WPMU_PLUGIN_DIR settings, the plugin_basename function returns the whole directory path in the URL.

The documentation does state not to use a trailing slash, however I think this function could be improved.

By removing the explicit / in the preg_replace the correct plugin name will be returned even if a trailing slash is specified on the WP_PLUGIN_DIR or WPMU_PLUGIN_DIR constants.

This should not affect the relative path as the trim before the return takes care of stripping off the first slash if it exists (effectively duplicating the remove in this case).

While this is a very trivial change that won't affect most people (who read the docs properly) it was quite annoying to track down.

Please see the attached diff.

Attachments (1)

plugin_basename.diff (828 bytes) - added by damianzaremba 6 years ago.

Download all attachments as: .zip

Change History (7)

#1 @nacin
6 years ago

I'm fine with this, but what else might be affected with WP_PLUGIN_DIR and WPMU_PLUGIN_DIR being mis-defined with a trailing slash? We'll have to check other pieces of usage elsewhere.

#2 @damianzaremba
6 years ago

From a quick grep though the code, it seems that it would mostly cause a double slash in some file paths. I'm not familiar enough with the core to tell if that would cause issues with some of the wp-filesystem functions, however I've not seen any on my 3.3 install.

#3 @chriscct7
3 years ago

  • Severity changed from minor to normal

Wouldn't this regress part of #9561?

#4 follow-up: @DrewAPicture
2 years ago

@jeremyfelt: I wonder if you've ever run into this in terms of the multisite perspective? See also the question in comment:3.

#5 @netweb
2 years ago

  • Keywords needs-unit-tests added

#6 in reply to: ↑ 4 @jeremyfelt
2 years ago

Replying to DrewAPicture:

@jeremyfelt: I wonder if you've ever run into this in terms of the multisite perspective? See also the question in comment:3.

I know I've accidentally added a / to the constant before and had to correct it. :) But nothing more than that from what I recall.

Very much agree with the needs-unit-tests tag here. Probably worth a check on how much plugins use the constant directly. My initial gut in general says to leave things as is.

Note: See TracTickets for help on using tickets.