Make WordPress Core

Opened 9 years ago

Last modified 6 years ago

#34634 new defect (bug)

Empty PHP_SELF causes 404 pages to load front page with 200 status code

Reported by: l3rady's profile l3rady Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 2.0
Component: Bootstrap/Load Keywords: has-patch has-unit-tests
Focuses: multisite Cc:

Description

I've been having this issue where visiting a non-existent page in wordpress on our live server would result in the homepage being loaded with a 200 OK response, however on local and staging going to an non-existent page would result in the correct 404 page and template being loaded.

I have a standard 4.3.1 WP install subdomain base multisite with pretty permalinks enabled running with Nginx/PHP-FPM.

I've looked to see what was different between local and live and it turns out on live $_SERVER['PHP_SELF'] is being set blank and on staging it is being set to /index.php. So to test I set at the top of my index.php file the following line $_SERVER['PHP_SELF'] = '';. Then when running on staging I was now getting the homepage load on a non-existent page.

So I've fired up my debugger to see what is going on in core and here are my findings.

In wp-load.php we set PHP_SELF to something based off REQUEST_URI

// Fix empty PHP_SELF
    $PHP_SELF = $_SERVER['PHP_SELF'];
    if ( empty( $PHP_SELF ) )
        $_SERVER['PHP_SELF'] = $PHP_SELF = preg_replace( '/(\?.*)?$/', '', $_SERVER["REQUEST_URI"] );

So the request http://www.example.com/non-existent results in PHP_SELF equal to /non-existent instead of /index.php

Then in class-wp.php WP->parese_request() we have:

            $self = $_SERVER['PHP_SELF'];
            $home_path = trim( parse_url( home_url(), PHP_URL_PATH ), '/' );
            $home_path_regex = sprintf( '|^%s|i', preg_quote( $home_path, '|' ) );

            // Trim path info from the end and the leading home path from the
            // front. For path info requests, this leaves us with the requesting
            // filename, if any. For 404 requests, this leaves us with the
            // requested permalink.
            $req_uri = str_replace($pathinfo, '', $req_uri);
            $req_uri = trim($req_uri, '/');
            $req_uri = preg_replace( $home_path_regex, '', $req_uri );
            $req_uri = trim($req_uri, '/');
            $pathinfo = trim($pathinfo, '/');
            $pathinfo = preg_replace( $home_path_regex, '', $pathinfo );
            $pathinfo = trim($pathinfo, '/');
            $self = trim($self, '/');
            $self = preg_replace( $home_path_regex, '', $self );
            $self = trim($self, '/');

This ends up with: $req_uri = $self = non-existent

Later in the same function we have:

// If req_uri is empty or if it is a request for ourself, unset error.
            if ( empty($request) || $req_uri == $self || strpos($_SERVER['PHP_SELF'], 'wp-admin/') !== false ) {
                unset( $error, $_GET['error'] );

                if ( isset($perma_query_vars) && strpos($_SERVER['PHP_SELF'], 'wp-admin/') !== false )
                    unset( $perma_query_vars );

                $this->did_permalink = false;
            }

Here because $req_uri == $self this code unsets the 404 which subsequently results in the front page being loaded rather than the 404 page

I did some further investigation and this appears to only happen on multisite installs. If I take a normal wordpress install and set $_SERVER['PHP_SELF'] = '' the 404's work as normal.

Attachments (2)

34634.diff (679 bytes) - added by Chouby 8 years ago.
34634-unit-tests.diff (1.8 KB) - added by andizer 7 years ago.
Unit tests

Download all attachments as: .zip

Change History (10)

@Chouby
8 years ago

#1 @Chouby
8 years ago

  • Component changed from Query to Bootstrap/Load
  • Keywords has-patch added
  • Version changed from 4.3.1 to 2.0

I had a different issue which however has the same cause, i.e the fact that $_SERVER['PHP_SELF'] does not contain a reliable value depending on the server configuration.

In most cases, it contains the relative path of the file to boot WordPress. When the server does not set it, wp_fix_server_vars() fixes it with $_SERVER['REQUEST_URI'] without the query string.

This works well for admin, login, etc... but not when WordPress boots from index.php. In that case, it is expected that the server fills the variable with '/index.php'. But if the server does not fill it, then wp_fix_server_vars() fixes it incorrectly to something like '/' when using default permalinks or something like '/postname' when using pretty permalinks.

This inconsistency can lead to various potential bugs including the one reported here.

In 34634.diff, I propose to explicitely set $_SERVER['PHP_SELF'] to '/index.php' when it is empty and $_SERVER['REQUEST_URI'] does not contain a php file. This is a first rough patch which may need a smarter test to avoid some edged cases.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


8 years ago

#3 @jorbin
8 years ago

  • Focuses multisite added

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


8 years ago

#5 @flixos90
8 years ago

  • Keywords needs-unit-tests added

@andizer
7 years ago

Unit tests

#6 @andizer
7 years ago

I've added unit tests. This is my first time writing WordPress core unit tests. Feedback is welcome.

#7 @andizer
6 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

#8 @jeremyfelt
6 years ago

Noting that I hit this issue (also logged here https://github.com/laravel/valet/issues/526) when using Valet due to its default population of PHP_SELF.

Note: See TracTickets for help on using tickets.