Ticket #5090 (closed defect: fixed)

Opened 1 year ago

Last modified 9 months ago

maybe_create_table call to config.php issue

Reported by: mattyrob Assigned to: 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 (291 bytes) - added by mattyrob on 09/27/07 18:06:25.

Change History

09/27/07 09:02:37 changed by JeremyVisser

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

09/27/07 11:19:43 changed by mattyrob

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

09/27/07 11:24:13 changed by mattyrob

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

09/27/07 18:06:25 changed by mattyrob

  • attachment install-helper.diff added.

09/27/07 18:06:41 changed by mattyrob

  • keywords set to has-patch.

Attaching patch diff

10/12/07 21:35:01 changed by ryan

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

(follow-up: ↓ 7 ) 10/22/07 19:12:53 changed by 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)

(in reply to: ↑ 6 ) 10/23/07 19:50:10 changed by westi

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!

(follow-up: ↓ 9 ) 10/24/07 18:14:42 changed by 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).

(in reply to: ↑ 8 ; follow-up: ↓ 10 ) 10/24/07 18:36:12 changed by westi

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.

(in reply to: ↑ 9 ) 10/25/07 08:47:11 changed by mattyrob

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.

10/25/07 17:34:48 changed by westi

  • 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

10/25/07 17:35:35 changed by westi

  • 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

10/27/07 20:13:05 changed by mattyrob

  • status changed from closed to reopened.
  • resolution deleted.
  • milestone changed from 2.4 to 2.3.2.

Any chance we can get this into 2.3.2?

12/20/07 21:52:21 changed by westi

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

01/09/08 19:28:23 changed by mattyrob

  • status changed from closed to reopened.
  • resolution 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 :-)

01/09/08 20:13:38 changed by ryan

  • owner changed from westi to ryan.
  • status changed from reopened to new.

02/02/08 17:45:44 changed by ryan

  • 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

02/02/08 19:40:21 changed by darkdragon

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.

02/02/08 19:42:13 changed by darkdragon

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