Use of add_theme_page() causes Wordpress on IIS to crash.

[bforchhammer]

The P2 Theme uses the following line to add a theme settings page:

add_theme_page('Prologue Options', 'Prologue Options', 8, __FILE__, 'prologue_options_page');

On IIS this causes a 500 Error and the admin section can not be loaded anymore.

When we substitute __FILE__ by a string, e.g. "p2" everything starts behaving again.

The following might be the cause of the problem:

__FILE__ is passed through the function plugin_basename() in add_submenu_page()... usually that function strips out the path and returns only the plugin filename; for themes that does not work and the function returns the full path to the theme's functions.php.

I don't know why it only crashes on IIS but the value of plugin_basename() is used for a few things including for registering an action hook (which means that it's probably used as a key of an array at some point).

[Denis-de-Bernardy]

what does plugin_basename() return?

[peaceablewhale]

I have installed the P2 theme (​http://wordpress.org/extend/themes/p2) to the latest trunk which runs on IIS 7.5 and PHP 5.3.0 RC2 through FastCGI. No error, warning or notice is displayed. The 'Prologue Options' page works successfully. Which version of IIS and PHP are you using?

[bforchhammer]

Replying to Denis-de-Bernardy:

what does plugin_basename() return?

The full path to the theme's functions.php...
E:/xampp/wordpress/wp-content/themes/mytheme/functions.php

[bforchhammer]

Replying to peaceablewhale:

I have installed the P2 theme (​http://wordpress.org/extend/themes/p2) to the latest trunk which runs on IIS 7.5 and PHP 5.3.0 RC2 through FastCGI. No error, warning or notice is displayed. The 'Prologue Options' page works successfully. Which version of IIS and PHP are you using?

Microsoft-IIS/7.0 with PHP Version 5.2.5 (x64) though FastCGI as well.

[peaceablewhale]

Thanks for the info. Would you add <httpErrors existingResponse="PassThrough" /> to the "system.webServer" section of your web.config file and post the 500 error message here? Please remove the the PassThrough statement after that.

[bforchhammer]

Replying to peaceablewhale:

Thanks for the info. Would you add <httpErrors existingResponse="PassThrough" /> to the "system.webServer" section of your web.config file and post the 500 error message here? Please remove the the PassThrough statement after that.

Adding <httpErrors existingResponse="PassThrough" /> does not change anything.

All it says is "The page cannot be displayed because an internal server error has occurred.".

Hm, I should add that he spins for 30s and then times out. 30s is what's set as max_execution_time.

[peaceablewhale]

Still can't reproduce the issue with PHP 5.2.9-2 NTS...

The Prologue Options page is loaded successfully and the URL of the page is "WORDPRESS-URL/wp-admin/themes.php?page=C:/wwwroot/wordpress/wp-content/themes/p2/functions.php".

So, "E:/xampp/wordpress/wp-content/themes/mytheme/functions.php" seems to be the intended path.

[dd32]

have you looked in t he logs yet?

It'd not surprise me if you've got a security function installed which detects ?xxxx=c:/... as remote file inclusion exploitation attempt..

[Denis-de-Bernardy]

could also be an infinite loop. or the seems_utf8 issue that was highlighted in a ticket from a day or two ago.

[bforchhammer]

Okay, I've tracked it down into ​menu-header.php and a file_exists() call.

file_exists("E:\Inetpub_Prod\wwwroot/wp-content/plugins/E:/Inetpub_Prod/wwwroot/wp-content/themes/p2/functions.php");

The path is a combination of WP_PLUGIN_DIR and that __FILE__ variable passed to add_theme_page; it obviously is quite wrong and file_exists() should simply return false.

In our environment however it spins and times out... very strange.

I've copied the code above into an empty php file and played around with it a bit; it seems to timeout as soon as there's a second e:/ in the path; wrong slashes, wrong path, etc. doesn't cause a timeout.

Could you guys try this on your IIS boxes and check how your server reacts?

It might just be a wrong server setting or php.ini configuration, or maybe a PHP/IIS Bug, but maybe WordPress should also do some cleaning up on that filename?

What do you guys think?

[ryan]

WP_PLUGIN_DIR should not contain a URL. Do you hard code it to something, or let wp-settings.php set it up?

[ryan]

Oops, duh, that's a windows path, not a url. Silly me, can't read.

[peaceablewhale]

Seem to be caused by this PHP bug: ​http://bugs.php.net/bug.php?id=44412

I cannot reproduce the error in both PHP 5.2.9-2 NTS and PHP 5.3.0RC2 NTS. Would you update your PHP?

[Denis-de-Bernardy]

haven't checked or tested, but this:

file_exists("E:\Inetpub_Prod\wwwroot/wp-content/plugins/E:/Inetpub_Prod/wwwroot/wp-content/themes/p2/functions.php");

seems like an an issue in the plugin_folder() function et al. or their mis-use in p2.

[bforchhammer]

Replying to ryan:

WP_PLUGIN_DIR should not contain a URL. Do you hard code it to something, or let wp-settings.php set it up?

I let wp-settings.php do the work...

[bforchhammer]

Replying to peaceablewhale:

Seem to be caused by this PHP bug: ​http://bugs.php.net/bug.php?id=44412

I cannot reproduce the error in both PHP 5.2.9-2 NTS and PHP 5.3.0RC2 NTS. Would you update your PHP?

5.2.5 seems to be the latest available 64bit version; I can ask our server guy but I don't think he's going to download and compile a newer version himself...

[bforchhammer]

add_submenu_page( $parent, $page_title, $menu_title, $access_level, $file, $function = '' )

If $file is set to __FILE__ in a plugin then the $file parameter gets cleaned up, and the absolute path to the file converted to a relative one (relative to WP_PLUGIN_DIR).

If the same thing is done in a theme's functions.php then the absolute path is not converted to a relative path!

I think the example in Codex does not recommend the use of __FILE__ anymore but there are probably still many themes out there using it.

[bforchhammer]

That patch should take care of the error... (needs testing; we're still on 2.7.1 but theme_basename() was added in 2.8).

[dd32]

My only negitive point to the patch: Not only themes add items under there.

Perhaps a helper function could be used:

function the_base($in) {
$plugin = plugin_basename($in);
if ( $plugin != $in )
return $in;
else
return theme_basename($in); //No need to check this, a non-theme dir passes right through.
}

and use that for all menu's.

[dd32]

of course, that first return should return $plugin..

[Denis-de-Bernardy]

-1 on the patch too. This ought be handled further downstream, and for all menu types if it's needed.

[peaceablewhale]

Replying to bforchhammer:

5.2.5 seems to be the latest available 64bit version; I can ask our server guy but I don't think he's going to download and compile a newer version himself...

x64 versions are not officially supported and they usually run slower than x86 versions.

[bforchhammer]

Replying to peaceablewhale:

x64 versions are not officially supported and they usually run slower than x86 versions.

Hm, good to know... I'll forward that note to our server admin ;-)

[bforchhammer]

Hm, theme_basename() doesn't look right to me.. $theme_dir is set up but not actually used in the preg_replace() call...

[bforchhammer]

Replying to dd32:

My only negitive point to the patch: Not only themes add items under there.

Perhaps a helper function could be used:

function the_base($in) {
$plugin = plugin_basename($in);
if ( $plugin != $in )
return $in;
else
return theme_basename($in); //No need to check this, a non-theme dir passes right through.
}

and use that for all menu's.

I am not sure we need a separate function to check because as far as I can see non-plugin dirs pass right through plugin_basename() as well don't they? It should be possible to just use them after each other.

$file = plugin_basename($file);
$file = theme_basename($file);

Of course if someone would put a completely different path in it would fail again and maybe a more general approach would be better?

Hm, other functions (menu-header.php) still assume that $file is relative to the plugins directory so that probably would need to be changed as well.

[dd32]

You're right... that would work just as well.

i'm not too sure it matters about other parts of WordPress expecting those parts to be relative to the plugins directory, The chances of a plugin of the same name with a file called 'functions.php' is incredibly unlikely. Furthurmore, I think that the hookname would still be reconised correctly.

So the solution here, seems to be to replace plugin_basename($file) with theme_basename(plugin_basename($file)) in all of the add-menu calls IMO.

Mind you, Thats going to require some testing(And given 2.8's nearing release), I'd suggest that this gets delayed to early 2.9 to allow for some proper testing?

If someone thinks of a better way thats absolutely 100% backwards compatible.. then speak up..

[bforchhammer]

Replying to dd32:

So the solution here, seems to be to replace plugin_basename($file) with theme_basename(plugin_basename($file)) in all of the add-menu calls IMO.

Sounds good to me.

That would probably not change anything in the current behaviour; as far as I can see the full path is not used for anything else but the "action hook name" at the moment so it shouldn't break anything... (?)

If I read the code correctly it should probably also be possible to point the path to a different file instead of using a function callback for the menu item; the last parameter in add_submenu_page() seems optional.

Mind you, Thats going to require some testing(And given 2.8's nearing release), I'd suggest that this gets delayed to early 2.9 to allow for some proper testing?

Fine by me :)

[Denis-de-Bernardy]

see also #10067

[ryan]

See #10132

[Denis-de-Bernardy]

#10132 has a patch that needs testing

[bforchhammer]

Replying to Denis-de-Bernardy:

#10132 has a patch that needs testing

#10132 does not address the issue with the php bug mentioned earlier (​http://bugs.php.net/bug.php?id=44412). In other words, it is still possible that the file_exists call receives an invalid path (see Comment 10 or am I missing something? Correct me if I'm wrong... :)

[Denis-de-Bernardy]

we understood it the same way. the only thing is the php bug is about a warning, whereas we're getting a fatal.

[Denis-de-Bernardy]

oh, sorry, I was thinking you mentioned the bug I had referenced in #10132. :-)

[peaceablewhale]

I have confirmed that ​http://bugs.php.net/bug.php?id=44412 exists in PHP 5.2.5 with open_basedir set, the PHP script will not return anything until it times out.

The bug has been fixed since PHP 5.2.6. With open_basedir set, a PHP warning is returned: "PHP Warning: file_exists() [function.file-exists]: open_basedir restriction in effect.".

Using @file_exists() will get "false".

Is it really worth working around the PHP bug?

[bforchhammer]

Replying to peaceablewhale:

Is it really worth working around the PHP bug?

Considering that version 5.2.5 is the ​highest available 64bit version I think it is.

[peaceablewhale]

However, that build is not officially supported by PHP and contains known security issues.

[peaceablewhale]

The official, experimental x64 versions of PHP 5.3.0 has been released on windows.php.net: ​http://windows.php.net/qa/

The problem reported in this report should not affect PHP 5.3.0. As a new x64 PHP package has been released, I suggest closing this report.

[bforchhammer]

Fine by me.

[mijk]

This works for me:

  //add_theme_page('GTD Options', 'GTD Options', 8, __FILE__, 'prologue_options_page');
    add_theme_page('GTD Options', 'GTD Options', 'edit_theme_options', 'p2-options-page', 'prologue_options_page');