Make WordPress Core

Opened 14 years ago

Last modified 5 weeks ago

#17246 reopened enhancement

Handling of HTTP 404 errors for non-existing files

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

Description

Generally if WordPress sees a request for a specific file it means the web server couldn't find that file. Currently we run all of WordPress in such cases and output the theme's 404 template.

If the missing file is an image that has been reused 10 times (perhaps a CSS background or sprite) we will run ten extra times on every page load.

What I'm proposing is to short-circuit all requests for specific files very early in the load cycle, the same way we do it for favicon.ico requests and output a generic 404 notice.

Change History (15)

#1 follow-up: @nacin
14 years ago

See #11884 and probably other tickets.

#2 in reply to: ↑ 1 @azaozz
14 years ago

Replying to nacin:

Yes, there are few tickets and also ideas that talk about optimizing htaccess. If some of these are implemented they may handle this case too.

However I'm talking specifically about 404 errors. This is something we can do now and it wouldn't break even some "non-standard" plugins as the files they send the requests to exist. WordPress is generating JS and CSS in that way too, see load-scripts.php and load-styles.php.

#3 @dd32
14 years ago

Things that come to mind here:

  • People with permastructs with .html etc in the url
  • People with custom rewrite rules to posts/etc which uses a form of /site/some/file/name.ext

#4 @nacin
14 years ago

  • Milestone changed from 3.2 to Future Release

#5 @nacin
11 years ago

  • Component changed from Performance to Bootstrap/Load
  • Focuses performance added
  • Milestone Future Release deleted
  • Resolution set to maybelater
  • Status changed from new to closed

#6 @rpayne7264
8 years ago

  • Resolution maybelater deleted
  • Status changed from closed to reopened
  • Version set to trunk

I propose that we add a wp_abort_execution filter to the core, which would allow developers to tell Wordpress whether or not to load their plugin.

Personally, I am now including a check in all of my plugins, after realizing the Wordpress process executes for all missing files referenced in client-side mark-up.

Here are the functions I am using inside a utilities class:

    static function abortExecution(){
        $rv = false;
        $wp_action = self::globalRequest('action');
        if($wp_action == 'heartbeat')$rv = true;
        if(!$rv):
            $url = (isset($_SERVER['REQUEST_URI']))? $_SERVER['REQUEST_URI'] : '';
            $rv =  self::isScriptStyleImgRequest($url);                  
        endif;
        return $rv;
    }//abortExecution
    
    static function isScriptStyleImgRequest($url){
        if(empty($url))return false;
        $arrExts = self::extensionList();
        $url_parts = parse_url($url);        
        $path = (empty($url_parts["path"]))? '' : $url_parts["path"];
        $urlExt = pathinfo($path, PATHINFO_EXTENSION);
        return key_exists($urlExt, $arrExts);
    }//isScriptStyleImgRequest 
    
    static function extensionList(){
        $ext = array();
        $mimes = wp_get_mime_types();

        foreach ($mimes as $key => $value) {
            $ak = explode('|', $key);
            $ext = array_merge($ext,$ak)  ;      
        }            
        
        return $ext;
    }//extensionList  

Either way, a "best practice" promulgation should go out to the Wordpress developer community to encourage implementing a check of the incoming request and prevent code from running unnecessarily.

#7 follow-up: @dd32
8 years ago

  • Resolution set to maybelater
  • Status changed from reopened to closed
  • Version trunk deleted

I'm re-closing this as maybelater as I don't think anything has changed here that gives any reason to look at it again.

I propose that we add a wp_abort_execution filter to the core, which would allow developers to tell Wordpress whether or not to load their plugin.

If you wish to abort a request, you can simply call die() upon plugin inclusion if certain criteria are met, or you could delay it until WordPress is fully loaded (init) to allow other plugins to handle the request if they wish (Such as a stats plugin which wants to record all 404's).

Personally, I am now including a check in all of my plugins, after realizing the Wordpress process executes for all missing files referenced in client-side mark-up.

I'd be very careful about doing that - It's expected that /blah/blah/filename.jpg be a request that's routed to WordPress and handled sometimes by plugins.

Personally I see the bug being that either the server is routing file-requests to WordPress when you don't want it to, or that markup is being used which references invalid paths.

#8 in reply to: ↑ 7 @rpayne7264
7 years ago

  • Resolution maybelater deleted
  • Status changed from closed to reopened

Aborting execution with a die() is not the answer, young Padawan, as that will kill the entire request.
I only want to abort execution of my plugin code if I know the request is for a script, image, or stylesheet, because my plugin doesn't provide any useful functionality for those types of files.

I have submitted proposed code for adding a highly-extensible wp_is_bad_request() function to the core, thus making such checks an easy matter for all plugin and theme creators. See #41934

Replying to dd32:

I'm re-closing this as maybelater as I don't think anything has changed here that gives any reason to look at it again.

I propose that we add a wp_abort_execution filter to the core, which would allow developers to tell Wordpress whether or not to load their plugin.

If you wish to abort a request, you can simply call die() upon plugin inclusion if certain criteria are met, or you could delay it until WordPress is fully loaded (init) to allow other plugins to handle the request if they wish (Such as a stats plugin which wants to record all 404's).

Personally, I am now including a check in all of my plugins, after realizing the Wordpress process executes for all missing files referenced in client-side mark-up.

I'd be very careful about doing that - It's expected that /blah/blah/filename.jpg be a request that's routed to WordPress and handled sometimes by plugins.

Personally I see the bug being that either the server is routing file-requests to WordPress when you don't want it to, or that markup is being used which references invalid paths.

#9 @SergeyBiryukov
6 years ago

  • Milestone set to Awaiting Review

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


5 years ago

#11 @SergeyBiryukov
5 years ago

#40777 was marked as a duplicate.

#12 @Hristo Sg
5 years ago

Just a suggestion, can we simply update the default .htaccess file generated by WordPress in order not to process certain file types through permalinks.

# BEGIN WordPress
# The directives (lines) between `BEGIN WordPress` and `END WordPress` are
# dynamically generated, and should only be modified via WordPress filters.
# Any changes to the directives between these markers will be overwritten.
<IfModule mod_rewrite.c>
RewriteEngine On
RewriteBase /

RewriteRule ([^.]+\.(jpe?g|gif|bmp|png))$ - [END,NC]

RewriteRule ^index\.php$ - [L]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule . /index.php [L]
</IfModule>
# END WordPress
Last edited 5 years ago by Hristo Sg (previous) (diff)

#13 @pbearne
14 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

I am closing this as the .htaccess is not supported in nginx

#14 @SergeyBiryukov
5 weeks ago

  • Milestone set to Awaiting Review
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Related: #63111.

Reopening, comment:12 seems worth consideration. Nginx requires a separate configuration anyway, so should not be a blocker here.

This ticket was mentioned in PR #8519 on WordPress/wordpress-develop by @sukhendu2002.


5 weeks ago
#15

  • Keywords has-patch has-unit-tests added; needs-patch removed
Note: See TracTickets for help on using tickets.