WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#5090 closed defect (bug) (fixed)

maybe_create_table call to config.php issue

Reported by: mattyrob Owned by: ryan
Milestone: 2.3.3 Priority: high
Severity: normal Version: 2.3
Component: Administration Keywords: has-patch
Focuses: 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 (1)

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

Download all attachments as: .zip

Change History (20)

#1 @JeremyVisser
10 years ago

  • 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.

#2 @mattyrob
10 years ago

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');

#3 @mattyrob
10 years ago

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 :-(

#4 @mattyrob
10 years ago

  • Keywords has-patch added

Attaching patch diff

#5 @ryan
10 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).

#6 follow-up: @mattyrob
10 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)

#7 in reply to: ↑ 6 @westi
10 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!

#8 follow-up: @mattyrob
10 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).

#9 in reply to: ↑ 8 ; follow-up: @westi
10 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.

#10 in reply to: ↑ 9 @mattyrob
10 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.

#11 @westi
10 years ago

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

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

#12 @westi
10 years ago

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

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

#13 @mattyrob
10 years ago

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

Any chance we can get this into 2.3.2?

#14 @westi
9 years ago

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

Closing this one.

No 2.3.2 has happened yet and 2.4 is approaching soon.

#15 @mattyrob
9 years ago

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

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 :-)

#16 @ryan
9 years ago

  • Owner changed from westi to ryan
  • Status changed from reopened to new

#17 @ryan
9 years ago

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

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

#18 @darkdragon
9 years ago

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.

#19 @darkdragon
9 years ago

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

Note: See TracTickets for help on using tickets.