Ticket #5090 (closed defect (bug): fixed)

Opened 4 years ago

Last modified 4 years ago

maybe_create_table call to config.php issue

Reported by: mattyrob Owned by: ryan
Priority: high Milestone: 2.3.3
Component: Administration Version: 2.3
Severity: normal Keywords: has-patch
Cc:

Description

The maybe_create_table function has moved into a new file in this release of WordPress. In this new file there is a called on wp-config.php using a relational path - this causes issues on my plugin.

This issue appears to be resolves using a full path definition of the location of the wp-config.php file.

I'll attach a patch.diff later, in the meantime in the file /wp-admin/install-helper.php line 2 needs updating from:

require_once('../wp-config.php');

to:

require_once(ABSPATH . 'wp-config.php');

Attachments

install-helper.diff Download (291 bytes) - added by mattyrob 4 years ago.

Change History

  • Summary changed from maye_create_table call to config.php issue to maybe_create_table call to config.php issue

Sometimes, when doing Install4Free installs, when I have the index.php in the root, and WordPress in /wordpress/, modifying the first line to be this:

require_once('./wordpress/wp-blog-header.php');

...doesn't work all the time. I have to strip the ./ from the beginning:

require_once('wordpress/wp-blog-header.php');

I've only ever had the issue on IIS servers.

Jeremy,

I think your issue is slightly different but could be solved by a similar change.

Perhaps the original code should be this:

require_once(ABSPATH . 'wp-blog-header.php');

And edited to this for custom installs:

require_once(ABSPATH . 'wordpress/wp-blog-header.php');

Whoops - scratch the last - ABSPATH isn't defined at that stage, if anything you'll have to use:

require(dirname(FILE) . '/wp-blog-header.php');

Which might still fail on IIS servers because of the slash :-(

  • Keywords has-patch added

Attaching patch diff

comment:5   ryan4 years ago

I'm trying to remember what the purpose of install-helper.php is. It just defines things already defined in upgrade.php (upgrade-functions.php before that).

comment:6 follow-up: ↓ 7   mattyrob4 years ago

@Ryan,

Same problem in that file too though as there is a call to wp-config.php using a relative path.

I'm trying to use the maybe_create_table function and test for its existence first, if it's not declared I require one of the files that defines it - but the calls to wp-config are creating errors (in both upgrade.php and install-helper.php)

By using ABSPATH the errors are resoved (in both files)

comment:7 in reply to: ↑ 6   westi4 years ago

Replying to mattyrob:

@Ryan,

Same problem in that file too though as there is a call to wp-config.php using a relative path.

I'm trying to use the maybe_create_table function and test for its existence first, if it's not declared I require one of the files that defines it - but the calls to wp-config are creating errors (in both upgrade.php and install-helper.php)

By using ABSPATH the errors are resoved (in both files)

if ABSPATH is defined then wp-config.php has already been included!

comment:8 follow-up: ↓ 9   mattyrob4 years ago

@Westi,

But I'm not including wp-config.php in my plugin - I'm accessing maybe_create_table. When I include either upgrade.php or install-helper.php THESE file call on wp-config'php using a relational path. As the original file executing is coming from the plugin folder (my script) the wp-config.php file is reported as not found.

By defining the full path to wp-config.php as in the attached diff fixes this issue. If you can suggest another way for me to access maybe_create_table I'll be more than happy but I only 'require_once' one of the two files above as maybe_create_table is not defined (I check is with an if (exists()) statement).

comment:9 in reply to: ↑ 8 ; follow-up: ↓ 10   westi4 years ago

Replying to mattyrob:

@Westi,

But I'm not including wp-config.php in my plugin - I'm accessing maybe_create_table. When I include either upgrade.php or install-helper.php THESE file call on wp-config'php using a relational path. As the original file executing is coming from the plugin folder (my script) the wp-config.php file is reported as not found.

By defining the full path to wp-config.php as in the attached diff fixes this issue. If you can suggest another way for me to access maybe_create_table I'll be more than happy but I only 'require_once' one of the two files above as maybe_create_table is not defined (I check is with an if (exists()) statement).

So your plugin file is being called directly rather than in a WordPress page load?

The attached patch can't work if you haven't already got ABSPATH defined and that is normally defined in wp-config.php - ideally we should say that including install-helper.php requires you to have already pulled in wp-config.php and then we could remove the require_once at the top and resolve this issue that way.

comment:10 in reply to: ↑ 9   mattyrob4 years ago

Replying to westi:

So your plugin file is being called directly rather than in a WordPress page load?

The attached patch can't work if you haven't already got ABSPATH defined and that is normally defined in wp-config.php - ideally we should say that including install-helper.php requires you to have already pulled in wp-config.php and then we could remove the require_once at the top and resolve this issue that way.

My plugin (Subscribe2) has changed over time and I've added a colum to the table created. To make sure this doesn't cause problems when users upgrade I have an upgrade function added to the shutdown hook. This upgrade function needs to use maybe_create_table and maybe_add_column. That is how the call is made.

The patch does work - I've tested it! However, if wp-config.php is not really required by install-helper.php then removing it should also fix my issue.

  • Owner changed from anonymous to westi
  • Status changed from new to assigned
  • Milestone changed from 2.3.1 to 2.4

It is probably best to preserve the ability to just include install-helper.php and let it get you wp-config.php

Therefore a dirname(dirname(__FILE__)).'/wp.config.php' is probably best

Moving to 2.4 as 2.3.1 has gone RC

  • Status changed from assigned to closed
  • Resolution set to fixed

(In [6291]) Fix the require_once in install-helper.php so that it works when included from a plugin. Fixes #5090

  • Status changed from closed to reopened
  • Resolution fixed deleted
  • Milestone changed from 2.4 to 2.3.2

Any chance we can get this into 2.3.2?

  • Status changed from reopened to closed
  • Resolution set to fixed
  • Milestone changed from 2.3.2 to 2.4

Closing this one.

No 2.3.2 has happened yet and 2.4 is approaching soon.

  • Status changed from closed to reopened
  • Resolution fixed deleted
  • Milestone changed from 2.5 to 2.3.3

This patch missed 2.3.2 with the expectation of an imminent 2.4. Since 2.4 is being skipped in favour of 2.5 _please_ can we get this into 2.3.3 and save me from users of my plugin who won't RTFM :-)

  • Owner changed from westi to ryan
  • Status changed from reopened to new
  • Status changed from new to closed
  • Resolution set to fixed

(In [6707]) Fix the require_once in install-helper.php so that it works when included from a plugin. Fixes #5090 for 2.3

I would refer to the discussion on wp-hackers on why dirname(dirname(...)) is pointless when dirname(__FILE__).'/../wp-config.php'); could be used instead. I think there is less confusion, since in my experience it takes me a while reading dirname(dirname()) that it means get the base folder of this file and then get the directory above this one.

By using the second method, if someone knows what dirname(FILE) means, then they can realize what '../' does. The only problem with the second method is that you absolutely must have the '/' before the '../', which you need anyway. It also gets pretty optimization nightmare, because coders might look at that and decide to use it everywhere not realizing that each dirname() call causes additional overhead. However, how many actually look in wp-blog-header.php I don't know.

Err, install-helper.php, which my guess is that optimization isn't important.

Note: See TracTickets for help on using tickets.