WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#11059 closed enhancement (wontfix)

Discourage plugin authors calling wp-config.php directly

Reported by: strider72 Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.9
Component: General Keywords:
Focuses: Cc:

Description

A lot of plugins call wp-config.php directly. In order to more actively discourage this practice, this patch adds a "deprecated" call within the if( !defined('ABSPATH')).

I define "wp-load.php" as the alternate, even though that's not the ideal solution. It's still better than calling wp-config.php. The $file string I pass is designed to fit within the message string from the _deprecated_file function.

Attachments (2)

wp-config_deprecate_patch.diff (605 bytes) - added by strider72 4 years ago.
Adds "deprecated file" call if wp-config.php is called directly.
11059_patch_2.diff (764 bytes) - added by strider72 4 years ago.
Error is no longer dependent on WP functions

Download all attachments as: .zip

Change History (20)

strider724 years ago

Adds "deprecated file" call if wp-config.php is called directly.

comment:1 xenlab4 years ago

I'm all for it. There are still ways around it obviously, but the more plugin authors know about not doing it this way, the more they might be inclined to research the proper way to do things.

comment:2 dd324 years ago

Correct me if i'm wrong.. But.. At the point of wp-config.php being included by a plugin, _deprecated_file() isnt actually defined yet..

comment:3 follow-up: xenlab4 years ago

With WP_DEBUG set to true, and the patch applied, I created this little plugin. I hard coded the path to wp-config to make it easy and consistent for front-end and wp-admin. Most of the offending plugins this patch seeks to discourage, torture themselves trying to figure out paths to the files they shouldn't include.

<?php
/*
	Plugin Name:	Test
	Version:		0.1
	Plugin URI:		http://xentek.net/
	Description:	test
	Author:			Eric Marden
	Author URI:		http://xentek.net/
*/
$path = '/Users/xentek/Sites/wordpress/';
include($path.'/wp-config.php');
add_action('init','myaction');

function myaction()
{
	return true;
}

?>

The only errors I get are due to constants being redefined. No deprecation warnings, and surprisingly no complaint about _deprecated_file(), though dd32 is probably correct in saying that its not defined, since it lives in wp-includes/functions.php

Either way, it doesn't seem to do the trick.

comment:4 scribu4 years ago

  • Milestone changed from Unassigned to 3.0

comment:5 in reply to: ↑ 3 strider724 years ago

Replying to xenlab:

With WP_DEBUG set to true, and the patch applied, I created this little plugin[...]. No deprecation warnings, and surprisingly no complaint about _deprecated_file(), though dd32 is probably correct in saying that its not defined, since it lives in wp-includes/functions.php

Either way, it doesn't seem to do the trick.

Generally speaking (though I'm sure there are exceptions) plugins call the file directly when they have some "stand alone" PHP file that runs outside of WordPress, but they want to access the database. So calling it from directly within a plugin's main file as your test does is not common.

However, that does point out the obvious point that _deprecated_file() will definitely not be defined. I wonder if we need to minimally recreate that function directly instead of trying to call it? Maybe something like:

if( defined( 'WP_DEBUG' ) && ( true === WP_DEBUG ) ) {

trigger_error( sprintf( ('%1$s is <strong>deprecated</strong> since version %2$s! Use %3$s instead.'), 'Calling wp-config.php directly from a plugin', '0.0', 'wp-load.php' ) );

}

We don't get the filters or the actions, but of course apply_filters etc. won't exist either in these cases. Still creates a caution to coders, which may be useful.

comment:6 westi4 years ago

  • Milestone 3.0 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

-1

The patch won't work for the reasons defined above - i.e. the file isn't there.

The correct solution is user education.

Search out plugins/code examples that do this and get them updated to do things the correct way.

comment:7 josephscott4 years ago

  • Cc joseph@… added

Although I agree that this specific approach isn't good enough, I don't think trying to comb through tens of thousands of plugins and themes looking for this problem is a good approach either.

The original concept of this ticket is still solid. The wp-config.php, wp-load.php, etc. files should never be directly included by a theme or plugin. If we can spit out a big ugly error when this is attempted then that's one of the best ways to educate a plugin/theme developer. The icing on the cake would be to have that big ugly error point to resources in the codex that describe the correct way to accomplish the most commons things that plugins/themes pull in wp-config.php for.

strider724 years ago

Error is no longer dependent on WP functions

comment:8 strider724 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

New patch no longer dependent on WP functions. It gives a PHP error if WP-CONFIG is defined true.

Westi -- I agree user education is key, but that's exactly what this is.

I do think the message could be improved with a URL for further information. What address would be best to put in there though?

comment:9 josephscott4 years ago

Plugins and themes should never, ever, ever, ever load wp-config.php, wp-load.php, wp-blog-header.php, etc. directly.

comment:10 westi4 years ago

I'm not sure I see the benefit of this as a way of educating users.

  • It only works on new installs - old installs already have a wp-config.php file
  • It requires that WP_DEBUG is enabled (and it should if it wants to error out) - but the setting of WP_DEBUG implies the plugin/theme author has half a clue as to what they are doing.

I think we do much better to put the message out explicitly that plugins/themes should never ever ever ever need to pull in wp-config.php / wp-load.php / wp-blog-header.php directly.

Pulling in those files is all about integrating WordPress into a 3rd part system.

comment:11 strider724 years ago

Westi --

Regarding new installs, that's true (to an extent), but such reasoning precludes a whole lot of improvements that have gone into WP in the past. We've added security keys, for example, that don't magically add themselves to existing installs. Personally, I from time to time look at the new sample config to see if just such things have been added.

As for "only clued-in coders will activate WP_DEBUG": It certainly won't catch all cases, but...

1) It will catch some. You could argue that clueless coders won't sign up for wp-hackers either, but how many people on that list didn't know about not loading these files?

2) Couldn't hurt, since it doesn't run unless someone does exactly what you say they'll never do. ;-)

3) Even in cases where it doesn't run, a lot of coders will *read* the message directly in the code and say "Really?" and pursue other info. I'm strongly for putting a URL in there saying where to go for more info, I just didn't know what URL to put in.

That's all I really have to say about it. I personally think it's a decent idea; but if you really hate it, close it again and I won't argue.... :-\

comment:12 follow-up: Viper007Bond4 years ago

Replying to josephscott:

Plugins and themes should never, ever, ever, ever load wp-config.php, wp-load.php, wp-blog-header.php, etc. directly.

Erm, why not? I mean stuff like dynamic .css files and stuff can instead be done by http://site.com/?outputmycss, but there's gotta be some type of situation out there where a plugin needs to load WordPress from a stand alone file and so wp-load.php has to be used.

comment:13 in reply to: ↑ 12 westi4 years ago

Replying to Viper007Bond:

Replying to josephscott:

Plugins and themes should never, ever, ever, ever load wp-config.php, wp-load.php, wp-blog-header.php, etc. directly.

Erm, why not? I mean stuff like dynamic .css files and stuff can instead be done by http://site.com/?outputmycss, but there's gotta be some type of situation out there where a plugin needs to load WordPress from a stand alone file and so wp-load.php has to be used.

I think what Joseph means here is that a plugin/theme php file which is include'd by WordPress by virtue of being used in the plugin or theme should not load in these files.

If you had a standalone php file you were calling which you then wanted to pull in the WordPress code you would then need to pull in the correct file.

I'm not sure what benefit you would gain over a query-string based url and using the available hooks though.

comment:14 josephscott4 years ago

If a plugin wants to do something like output custom CSS based on options set via wp-admin then hook into query_vars (there are other methods specifically to support custom CSS as well, just using this as the example). Given the tremendous amount of flexibility now possible in WP for moving and re-naming wp-content, theme and plugin paths, wp-config.php, etc. trying to pull in wp-config.php, wp-load.php, wp-blog-header.php is virtually guaranteed to break in some installs of WP. Using the query_vars hook will always work across all installs.

Will Norris has a good write up about this (saved me from writing one) - http://willnorris.com/2009/06/wordpress-plugin-pet-peeve-2-direct-calls-to-plugin-files

I've looked at enough submissions to the WPORG theme directory to tell you that trying to pull in wp-config.php, wp-load.php, wp-blog-header.php, etc. directly is fraught with peril. Given that solution A (query_vars hooks) works for all WP installs and solution B will break in some installs, solution B should be discouraged in any way possible.

Having worked with numerous theme authors to address this issue I have pretty strong feelings about it :-)

comment:15 xenlab4 years ago

As do the rest of the WP developer community that actually takes advantage of the power/flexibility of wp-config.php to change the layout of the WordPress files.

comment:16 sivel4 years ago

  • Keywords has-patch removed
  • Resolution set to wontfix
  • Status changed from reopened to closed

I am -1 for this. And I agree with all of the opposing reasons. While it is recommended that people not load wp-config.php directly, we should not be in the business of forcing this, it should remain a recommendation. User education is best and I think we are doing pretty well this.

It was decided in the Dev chat that this will be closed as wontfix.

comment:17 strider724 years ago

"While it is recommended that people not load wp-config.php directly, we should not be in the business of forcing this, it should remain a recommendation."

My biggest disagreement with that statement is that loading wp-config.php directly, breaks legitimate installs of WordPress as it exists right now -- not in some hypothetical "maybe" situation. (Specifically talking about installs where wp-config.php is moved up one directory, in which cases it's either not found OR sets an incorrect ABSPATH.)

Which is why the original patch pushed them toward wp-load. Not recommended, but better, because direct wp-config loading is *broken* wheras wp-load is... problematic but not broken.

Ah, well. Off to write a "Hey dummy don't load wp-config.php" article... ;-p Any recommendations for a "further information" link URL?

comment:18 josephscott4 years ago

I must have missed the discussion on this ticket during the weekly dev chat.

There's absolutely 100% a problem with themes and plugins including wp-config.php, wp-load.php, wp-blog-header.php, etc. directly. Now we may need to come up with a better mechanism (and patch) to deal with this, but the concept of actively discouraging theme and plugin developers from doing something we know will break is the right thing to do.

If we can detect a problem in code why would we choose not to do something about it? Ignoring something that will cause fatal errors and instead deferring to "user education" sounds like a missed opportunity.

Note: See TracTickets for help on using tickets.