WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#10132 closed defect (bug) (fixed)

PHP Warning at menu-header.php line 118

Reported by: creativenadir Owned by:
Milestone: 2.8.1 Priority: high
Severity: major Version: 2.8
Component: Administration Keywords: has-patch tested commit
Focuses: Cc:

Description

After performing a new installation of 2.8-IIS release on machine running PHP under IIS' FastCGI, cannot access wp-admin pages; returns error "open_basedir restriction in effect..." and implicates line 118 of wp-admin/menu-header.php. Checking php.ini, I ensured that safe mode was off, commented-out the open_basedir parameter, and restarted IIS to no avail.

On a whim, I replaced line 118 with its 2.7.1 version, and the problem disappeared! Moreover, after it began working, I returned line 118 to its previous version and it continued to work without error from then on.

Version 2.8:

 if ( ( ('index.php' != $sub_item[2]) && file_exists(WP_PLUGIN_DIR . "/{$sub_item[2]}") ) || ! empty($menu_hook) ) {

to Version 2.7.1:

if ( ( file_exists(WP_PLUGIN_DIR . "/{$sub_item[2]}") && ('index.php' != $sub_item[2]) ) || ! empty($menu_hook) ) { 

Attachments (4)

10132.diff (2.1 KB) - added by Denis-de-Bernardy 5 years ago.
10132.patch (816 bytes) - added by peaceablewhale 5 years ago.
10132-disable-PHP-Warning.patch (710 bytes) - added by peaceablewhale 5 years ago.
10132.2.diff (3.7 KB) - added by ryan 5 years ago.

Download all attachments as: .zip

Change History (29)

comment:1 ryan5 years ago

  • Milestone Unassigned deleted
  • Resolution set to duplicate
  • Status changed from new to closed

See #10011

comment:2 peaceablewhale5 years ago

  • Component changed from General to Administration
  • Keywords reporter-feedback added
  • Resolution duplicate deleted
  • Status changed from closed to reopened

Looks like a different issue.

"(('index.php'!=$sub_item[2]) && file_exists(WP_PLUGIN_DIR."/{$sub_item[2]}"))
!empty($menu_hook))" and "(file_exists(WP_PLUGIN_DIR."/{$sub_item[2]}") && ('index.php'!=$sub_item[2])) !empty($menu_hook)" appear the same (although the order is different).

Would you verify that the change in order has fixed the problem? Also, is the error open_basedir-related?

comment:3 peaceablewhale5 years ago

In addition, what are the versions of your PHP and IIS?

comment:4 Denis-de-Bernardy5 years ago

  • Milestone set to 2.8.1
  • Severity changed from blocker to major

it might be that sub_menu contains ../ in it or something.

comment:5 ryan5 years ago

Related: [10890]

comment:6 Denis-de-Bernardy5 years ago

It's actually related to this:

http://bugs.php.net/bug.php?id=41518

"If we remove this warning for non-existent files, it could be possible
to use file_exists() to detect which files exists (since it's perfectly
legal to print this warning when the file exists)."

comment:7 Denis-de-Bernardy5 years ago

mm, maybe not, we're talking about a fatal error, rather than a warning.

comment:8 Denis-de-Bernardy5 years ago

  • Keywords has-patch added

line 118 in r10890 mostly sought to optimize the thingy by inverting the checks, to avoid an unneeded file_exists call.

@creativenadir: please give the patch I'll attach in a sec a shot.

Denis-de-Bernardy5 years ago

comment:9 Denis-de-Bernardy5 years ago

  • Keywords reporter-feedback removed

comment:10 peaceablewhale5 years ago

  • Keywords early dev-feedback added; has-patch removed
  • Summary changed from IIS fatal error at menu-header.php line 118 to PHP Warning at menu-header.php line 118

"open_basedir restriction in effect" is never a fatal error, and it is not limited to IIS I think.

To fix it, we have to either ask PHP not to report any Warning or add @ before all file_exists calls. Which method is better?

comment:11 peaceablewhale5 years ago

Actually, if error_reporting is set to E_ALL in php.ini, I can't even enter /wp-admin/... The PHP Warning blocked the access.

comment:12 peaceablewhale5 years ago

  • Priority changed from normal to high

see also #7640 and #5771, which are also related to open_basedir.

Disabling PHP Warning will also fix them.

comment:13 Denis-de-Bernardy5 years ago

  • Keywords has-patch reporter-feedback added; dev-feedback removed

@peaceablewhale: the odd thing is, the patch I uploaded is what the reporter suggested fixed it for him. have you tried it in a setup similar to his?

comment:14 peaceablewhale5 years ago

I have tried the patch but it doesn't work. The PHP Warning remains blocking the Admin interface.

peaceablewhale5 years ago

comment:15 peaceablewhale5 years ago

I have uploaded a patch that uses @file_exists to fix the problem. Tested it with PHP 5.2.9-2 NTS on IIS 7.5 via FastCGI.

By the way, the PHP Warning in full is:

PHP Warning: file_exists() [function.file-exists]: open_basedir restriction in effect. File(C:\wwwroot\wordpress/wp-content/plugins/edit-tags.php?taxonomy=post_tag) is not within the allowed path(s): (C:\wwwroot) in C:\wwwroot\wordpress\wp-admin\menu-header.php on line 118

comment:16 peaceablewhale5 years ago

  • Keywords needs-review added; early removed

This patch (10132-disable-PHP-Warning.patch) disables PHP Warning, which fixes this report and probably also #7640 and #5771. Tested with PHP 5.2.9-2 NTS on IIS 7.5 via FastCGI as well.

comment:17 ryan5 years ago

Is it the query arg causing the problem. Maybe we should trim '/\?.*$/'.

comment:18 ryan5 years ago

Patch strips query from file names.

comment:19 peaceablewhale5 years ago

Using preg_replace('/\?.*$/',,SUBJECT) seems to have better code readability.

comment:20 peaceablewhale5 years ago

  • Keywords tested added; needs-review removed

Tested 10132.2.diff on PHP 5.2.9-2, IIS 7.5 via FastCGI.

ryan5 years ago

comment:21 ryan5 years ago

I was going for fast. Usually strpos and substr are faster than pregs.

comment:22 ryan5 years ago

Updated the patch to fix submenu highlighting that was broken by a variable naming collision. menu-header.php could stand a thorough cleanup, but let's save that for later.

comment:23 peaceablewhale5 years ago

  • Keywords commit added; reporter-feedback removed

Verified fixed.

comment:24 ryan5 years ago

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

(In [11582]) Trim query strings from menu entries before seeing if a corresponding file exists. fixes #10132 for trunk

Note: See TracTickets for help on using tickets.