[buddypress-trac] [BuddyPress] #2945: Don't hard-code the 'buddypress' plugin folder

[buddypress-trac at lists.automattic.com]

#2945: Don't hard-code the 'buddypress' plugin folder
--------------------------+-----------------------------
 Reporter:  cnorris23     |       Owner:
     Type:  defect (bug)  |      Status:  reopened
 Priority:  normal        |   Milestone:  Future Release
Component:  Core          |     Version:  1.6
 Severity:  normal        |  Resolution:
 Keywords:  needs-patch   |
--------------------------+-----------------------------

Comment (by foxly):

 However, @cnorris32, I disagree with your statement that...

 ''"The changes proposed by this ticket, and the changes already committed,
 do nothing different than what is already being done by the majority of
 actively maintained plugins in the WP plugin directory"''

 A comment you posted to this ticket two months ago reads:

 ''"New patch should fix admin/bp-core-update.php. Two things''

 ''I'd still like to see BP use''


 {{{
 define( 'BP_PLUGIN_DIR', plugin_dir_path( __FILE__ ) );
 }}}

 instead of

 {{{
 define( 'BP_PLUGIN_DIR', trailingslashit( WP_PLUGIN_DIR . '/buddypress' )
 );
 }}}


 ''This way, a folder name change can be made, and BP will adjust
 automagically ;) The global can still be defined for more fine-grained
 control/trickery."''

 In my opinion, it really sounds like your design intent is to allow the
 site admin to change BP's folder name.

 Based your comment from four days ago:

 ''"@sakthi31 try removing trailingslashit(). I believe BP_PLUGIN_DIR is
 expected without a trailing slash. You can also turn on WP_DEBUG to see
 what kind of errors you're getting, as it could be a plugin that hard
 coded the BP plugin directory, rather than using BP_PLUGIN_DIR."''

 It sounds to me like the BP_PLUGIN_DIR constant is allowing users to
 modify BP's folder name, which is clearly NOT in compliance with WP design
 practices.

 '''Here's why:'''

 Take a look at how the WP core handles plugin activation:

 http://core.svn.wordpress.org/branches/3.4/wp-includes/plugin.php

 On line 560 function plugin_basename($file) turns the realpath of a plugin
 file that looks like this:

 {{{
 "C://var/dir/dir/wp_plugins_folder/plugin_folder/loader.php"
 }}}

 into something that looks like this:

 {{{
 "plugin_folder/loader.php"
 }}}

 That function gets called by

 {{{
 function register_activation_hook($file, $function)
 }}}

 on line 617 in the same file, which includes the code

 {{{
 add_action('activate_' . $file, $function);
 }}}

 So the first thing that will happen if you move buddypress to an arbitrary
 folder is that '''any plugin that listens for Buddypress to activate
 itself is going to break'''.

 http://core.svn.wordpress.org/branches/3.4/wp-admin/plugin.php

 Further along in the activation process,  inside

 {{{
 activate_plugin( $plugin, $redirect = '', $network_wide = false, $silent =
 false )
 }}}

 on lines 547 and 551, Wordpress uses the plugin_basename value as a GUID
 and writes it to the 'active_plugins' and optionally the
 'active_sitewide_plugins' options.

 http://core.svn.wordpress.org/branches/3.4/wp-includes/update.php

 Then, in function

 {{{
 wp_update_plugins()
 }}}

 on line 136 of update.php, on line 147 the function fetches the unique
 active plugin slugs using

 {{{
 $active  = get_option( 'active_plugins', array() );
 }}}

 on line 205 it checks the slug against the WP plugin repo to see if the
 repo has been updated

 {{{
 $raw_response = wp_remote_post('http://api.wordpress.org/plugins/update-
 check/1.0/', $options);
 }}}

 http://core.svn.wordpress.org/branches/3.4/wp-admin/includes/class-wp-
 upgrader.php

 And if the repo has been updated

 {{{
 class Plugin_Upgrader extends WP_Upgrader {
 }}}

 on line 23 gets called, successively downloading and checking the updated
 zip file from the repo, then deleting the entire contents of the folder
 that maps to the plugin_basename and overwriting it with the files
 downloaded from the repo.

 The consequence of this is if the admin renames the "/buddypress" folder
 to be "/awesome-plugin" and "awesome-plugin" exists on the WP plugin repo,
 then when the author of "awesome-plugin" updates their plugin, the site
 admin will get an "upgrade alert" for BuddyPress.

 If the site admin clicks on "upgrade plugin", '''Buddypress will be
 deleted and replaced with "awesome-plugin"'''. In addition to this, '''all
 of Buddypress' database tables will probably be dropped and replaced with
 awesome-plugin's database tables'''.

 These are only '''some''' of the problems that renaming the "/buddypress"
 folder can cause. Because changing the /buddypress plugin folder name to
 something other than what Buddypress has been assigned by the WP plugin
 repo falls outside of the WP plugin specification, there is no limit to
 the potential problems it can cause in future versions of Wordpress.

 =====

 '''Nobody on this ticket should feel bad for requesting or going along
 with this modification request. On the surface it looks like a relatively
 innocent change. Only developers with very, very in-depth knowledge of the
 WP core would catch this one, and only after spending a considerable
 amount of time researching it.'''

 =====

 I recommend we make sure Buddypress follows the base folder path
 generation methods set forth in the codex at the link provided by
 @cnorris32; but '''I recommend we immediately remove any code that implies
 or assumes the user is able to set a BP_PLUGIN_DIR constant'''.

 The existence of a BP_PLUGIN_DIR constant that is *generated by
 Buddypress* for the convenience of other developers is, of course, totally
 OK.

 -F

-- 
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/2945#comment:28>
BuddyPress <http://buddypress.org/>
BuddyPress