WordPress.org

Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 9 years ago

#6938 closed enhancement (fixed)

Allow wp-content directory to exist in a custom location (not relative to ABSPATH)

Reported by: sambauers Owned by:
Milestone: 2.6 Priority: normal
Severity: normal Version: 2.5.1
Component: General Keywords: has-patch
Focuses: Cc:
PR Number:

Description (last modified by ryan)

This enhancement is related to ticket #6933, with similar reasoning.

  1. It's a little more secure if wp-content happens to also be outside the webroot
  2. It allows us to add WordPress as an external repository in other projects with plugins/themes/cache engines/database classes that sit outside that external repository (we do some of this with bbPress)

WordPress requires a few changes as there are a few hard-coded references to wp-content.

If we don't like the constant name I chose ( WP_CONTENT_DIR ), then that can be easily changed in the patch.

This will almost certainly break a few plugins where wp-config is hard-coded, but only when the blog owner chooses to use this feature. Naughty plugins should use API calls anyway :)

Attachments (7)

allow-wp-content-directory-outside-of-webroot.patch (11.5 KB) - added by sambauers 11 years ago.
allow-wp-content-directory-outside-of-webroot-2.patch (35.7 KB) - added by sambauers 11 years ago.
load_plugin_textdomain-fix.patch (516 bytes) - added by sambauers 11 years ago.
theme_screenshots_fix.patch (1.9 KB) - added by sambauers 11 years ago.
Fixes theme screenshots in admin
load_plugin_textdomain-backward-compat.diff (1.7 KB) - added by nbachiyski 11 years ago.
wp-load.php (166 bytes) - added by ryan 11 years ago.
Sample wp-content/wp-load.php
no_base_url_for_content.diff (515 bytes) - added by ryan 11 years ago.

Download all attachments as: .zip

Change History (41)

#1 @mdawaffe
11 years ago

+1

There were some false starts to allow the plugins dir, themes dir, languages dir to be defined individually: #4039 (and maybe others).

Note that you'll also need to define WP_CONTENT_URL or something similar and have the various plugin and theme functions reference that instead of get_option( 'wp_url' ) (or whatever it is).

#2 @sambauers
11 years ago

mdawaffe raised a point on IRC that some file path sanitisation function may not like the idea of relative paths i.e. the ".." in the paths. This would need checking.

Also, directory traversal may not be allowed on some hosts in PHP safe mode, but I think it's OK to have a feature that isn't available to PHP safe mode users. IN any case, this change doesn't alter or break the default behaviour.

#3 @sambauers
11 years ago

Missed some hard-coded references in the previous patch.

#4 follow-up: @ryan
11 years ago

Can we make WP_CONTENT_DIR be an absolute path so that we don't have to worry with ".."? Things like PLUGINDIR that expect it to be relative to root will need to be changed around. Something like:

define( 'WP_CONTENT_DIR', ABSPATH . 'wp-content' );
define( 'WP_CONTENT_URL', 'wp-content'); // Relative to get_option('home'), not ABSPATH
define( 'WP_PLUGIN_DIR', WP_CONTENT_DIR . '/plugins');
define( 'WP_PLUGIN_URL', WP_CONTENT_URL . '/plugins'); // Used for plugin_basename()
define( 'PLUGINDIR', 'wp-content/plugins'); // Relative to ABSPATH.  For back compat.

Calculating WP_CONTENT_URL will require stripping get_home_path() from ABSPATH to get a path relative to home rather than siteurl.

#5 in reply to: ↑ 4 @sambauers
11 years ago

Replying to ryan:

Can we make WP_CONTENT_DIR be an absolute path so that we don't have to worry with ".."?

Sounds good and I was already looking at doing this based on some conversations with mdawaffe.

I was also uncomfortable with the dotted relative paths. I'll produce another patch which does this in a more complete and clean fashion.

#6 @sambauers
11 years ago

A more thorough patch now attached mostly based on comments by ryan.

WP_CONTENT_DIR - full path
WP_CONTENT_URL - full url
WP_PLUGIN_DIR - full path
WP_PLUGIN_URL - full url
WP_LANG_DIR - full path

Deprecated constants declared with limited backwards compatibility.

PLUGINDIR
LANGDIR

#7 @sambauers
11 years ago

Trac formatting fails me again... along with not previewing...

A more thorough patch now attached mostly based on comments by ryan.

  • WP_CONTENT_DIR - full path
  • WP_CONTENT_URL - full url
  • WP_PLUGIN_DIR - full path
  • WP_PLUGIN_URL - full url
  • WP_LANG_DIR - full path

Deprecated constants declared with limited backwards compatibility.

  • PLUGINDIR
  • LANGDIR

#8 @ryan
11 years ago

(In [7999]) Allow wp-content to exist outside of webroot. Props sambauers. see #6938

#9 @ryan
11 years ago

  • Description modified (diff)

#10 follow-up: @DD32
11 years ago

FYI: By the look of it, This has broken the upgrader pretty badly, And the changes that were made to the abstraction doesnt seem to have done it any favours.

I'll try and get it working again in the next few days, I have a feeling for anyone who uses this functionality on a shared hoster is going to have huge headaches.

#11 @DD32
11 years ago

(By that, I mean the plugin upgrader)

#12 @ryan
11 years ago

Yeah, those looked suspect. We can revert that part if you can't get to it soonish.

#13 @DD32
11 years ago

Reverting those changes would be good, Its going to take a bit of time to work around it, and i dont have my usual ftp testing sites running right now.

I know i'll definately be taking advantage of these changes though once its working though (Relative paths for PLUGINDIR are messy :))

#14 in reply to: ↑ 10 @sambauers
11 years ago

  • Summary changed from Allow wp-content directory to exist in a custom location (relative to ABSPATH) to Allow wp-content directory to exist in a custom location (not relative to ABSPATH)

Replying to DD32:

FYI: By the look of it, This has broken the upgrader pretty badly, And the changes that were made to the abstraction doesnt seem to have done it any favours.

Yeah, I wasn't 100% sure about the changes I made to the WP File System stuff. I meant to contact you about it, but the patch was committed before I did that. Either way, now you know. :)

Note, these custom locations aren't relative to ABSPATH at all anymore. They are full paths and URLs. So I'm changing the title of this ticket.

Also, there is a bug in the changes to the l10n function. Patch for that part is attached.

#15 @DD32
11 years ago

Yeah, I wasn't 100% sure about the changes I made to the WP File System stuff. I meant to contact you about it, but the patch was committed before I did that. Either way, now you know. :)

I saw your patch the other day, Just hadnt had a chance to look at what it meant yet :)

Note, these custom locations aren't relative to ABSPATH at all anymore. They are full paths and URLs. So I'm changing the title of this ticket.

Yep, That makes it easier from the point of developers, I'm not sure how it'll affect the plugin upgrader code though.

At present, It just locates the base of the wordpress installer (looking for wp-config.php i think - or was it wp-settings, i cant remember) so it needs to be updated anyway to handle the new locations, I doubt it'd have worked well with a relative (../../../) PLUGINDIR constant before too now i think of it (It would've worked in some situations)

#16 @sambauers
11 years ago

Also see new related ticket at #7050

#17 @DD32
11 years ago

See new ticket at #7059 for WP_Filesystem abstraction changes to take advantage of this.

@sambauers
11 years ago

Fixes theme screenshots in admin

#18 @ryan
11 years ago

(In [8040]) Use WP_CONTENT_DIR and URL when linking to theme screenshots. Props sambauers. see #6938

#19 @ryan
11 years ago

(In [8041]) Make load_plugin_textdomain() work with WP_CONTENT_DIR. Props sambauers. see #6938

#20 follow-up: @nbachiyski
11 years ago

The load_plugin_textdomain patch breaks backward compatibility. The $path arg used to represent a path, relative to ABSPATH, and now it is relative to WP_PLUGIN_DIR.

There are a lot of plugins using it, so I think we should at least support it for one more version. We can add another argument, which represents path, relative to WP_PLUGIN_DIR and urge developers to use it. In 2.7 we will switch top the new way only.

A patch with updated function and docs is attached.

#21 @ryan
11 years ago

(In [8065]) Back compat fixes for load_plugin_textdomain() from nbachiyski. see #6938

#22 in reply to: ↑ 20 @sambauers
11 years ago

Replying to nbachiyski:

There are a lot of plugins using it, so I think we should at least support it for one more version. We can add another argument, which represents path, relative to WP_PLUGIN_DIR and urge developers to use it. In 2.7 we will switch top the new way only.

Probably no pressing need to deprecate the old way. It is nice to give the flexibility to use ABSPATH as well as WP_PLUGIN_DIR.

See notes on #7075 which this patch fixes.

#23 @ryan
11 years ago

While inserting images, I noticed that the upload was obeying the path in Settings->Misc while image display was obeying WP_CONTENT_DIR.

#24 @lilyfan
11 years ago

This enhancement breaks compatibility with plugins that includes wp-load.php/wp-config.php.
Because the relative path to the plugin dir and wp-load.php is not uniqly defined.

AJAX plugins often uses its own API such as http://blog.example.com/wp-content/plugins/ajax/api.php,
and api.php includes wp-load.php/wp-config.php to use WordPress feature like:
require dirname(dirname(dirname(dirname(__FILE__)))) . '/wp-config.php';

If plugin directory changed, api.php cannot find wp-load.php/wp-config.php.
Please provide a way to seek wp-load.php in that case.

#25 @ryan
11 years ago

We can add a wp-load.php file to the root of the content directory that loads the real wp-load.php. Those who relocate their content directory will have to modify this file. Those using WP_CONTENT_DIR to share a content directory between blogs will probably need to parse REQUEST_URI to see which blog's wp-load.php should be loaded.

@ryan
11 years ago

Sample wp-content/wp-load.php

#26 @sambauers
11 years ago

What's the fuss? wp-load.php never moves and should be called in preference to wp-config.php

require(ABSPATH . 'wp-load.php');

#27 @sambauers
11 years ago

Oh wait, I see...

#28 @DD32
11 years ago

A Better way could just to get Plugin authors to run ajax via admin-ajax.php(I do this for admin-related ajax), Or via the main WP index.php page, That way they're assured that the user logged in checks are run, potentially decreasing the ability for a plugin to accidentally use a low level function, and allow exploitation of it..

#29 @sambauers
11 years ago

I think if plugin authors want to bypass the built-in API then they are kind of on their own. That wp-load.php file isn't going to help much as it can't be relied on to be set.

I don't now why plugins would bypass the API to eventually load it. Why not just pass the AJAX through the API in the first place like DD32 suggests.

There is also nothing stopping plugin authors requiring setting a variable in their files that points to the WordPress root in the case where it can't be determined.

#30 @ryan
11 years ago

I forgot about the action in admin-ajax.php.

do_action( 'wp_ajax_' . $_POST['action'] );

POSTing to admin-ajax.php and registering a handler for your action seems the easiest way to go.

#31 @lilyfan
11 years ago

A Better way could just to get Plugin authors to run ajax via admin-ajax.php(I do this for admin-related ajax),

This solution is good for Ajax plugins. But Ajax is an example of including wp-load.php/wp-config.php.
There is many plugins that includes wp-config.php for other purpose.

For example, I developed a mobile plugin for Japanese cell phones: this provides posting comments, admin feature, etc.
To process comments, an indivisual comments-post.php file is needed and it includes wp-config.php to use WordPress feature.
Mobile admin feature also requires wp-config.php like wp-admin/admin.php does.

Therefore, providing wp-content/wp-load.php is good solution for these target.

#32 @sambauers
11 years ago

Like I said, if you step outside of the API, I don't think you can expect the API to accommodate you. If you need to locate the config file (or wp-load) then I think you need to cater for that in your plugin settings or with installation instructions somehow.

I don't see why the example you gave couldn't exist within the API somehow.

#33 @ryan
11 years ago

(In [8301]) Introduce content_url(). Don't prepend base url to content url in script loader. see #6938 #7001

#34 @ryan
11 years ago

  • Milestone changed from 2.9 to 2.6
  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.