Bug #1919


mod_expires sends headers on 404 responses

Added by codemobs almost 15 years ago. Updated over 7 years ago.

Target version:


mod_expires sends the following headers with all responses including 400-500 status codes when it should not.

Expires: Thu, 26 Feb 2009 02:22:17 GMT
Cache-Control: max-age=3600

While it's possible for 404 pages to have expiration headers, it's generally not wanted. This seems to be a regression from 1.4.18 which is what we used before. Current version is r2349 of lighttpd 1.5.0.

I've included a rudimentary patch that ignores responses of status codes 400-599. I'm not familiar with convention of the code.


mod_expire.c.patch (336 Bytes) mod_expire.c.patch Rudimentary patch to ignore responses of status codes 400-599. codemobs, 2009-02-26 01:36
Actions #1

Updated by gstrauss almost 8 years ago

probably should decline to add caching headers unless http status 200 OK or 206 Partial Content

diff --git a/src/mod_expire.c b/src/mod_expire.c
index 1fcfec0..58481e0 100644
--- a/src/mod_expire.c
+++ b/src/mod_expire.c
@@ -291,6 +291,7 @@ URIHANDLER_FUNC(mod_expire_path_handler) {
        size_t k;

        if (buffer_is_empty(con->uri.path)) return HANDLER_GO_ON;
+       if (con->http_status != 200 && con->http_status != 206) return HANDLER_GO_ON;

        mod_expire_patch_connection(srv, con, p);

Actions #2

Updated by stbuehler over 7 years ago

I'm not quite sure how this would have changed since 1.4.18 (the original ticket description seems to refer to 1.5 anyway).

@gstrauss: Do you have some pointers why 200 + 206 would be a good choice?

It would also be nice to have some unit tests for mod_expire.

Actions #3

Updated by gstrauss over 7 years ago

As mod_expire currently applies only to static files, 200 OK and 206 Partial Content are the two response codes that make sense to be cachable. Any non-2xx response for static files should not have caching headers applied by mod_expire, since the request was for a static file. (If something else knows better, e.g. a dynamic 404 response handler, then the 404 dynamic response handler should add appropriate caching headers.)

Actions #4

Updated by gstrauss over 7 years ago

Either patch (the original patch posted, or my more explicit patch for 200 or 206) is much better than the existing behavior.

Incorrect caching headers are worse than no caching headers. Both of these patches preserve caching headers for the expected success cases (200 and 206 for static files), and cease adding caching headers for (at least) 400-599 status codes, where at least some (and probably all) of those additions of caching headers for 400-599 are incorrect, as noted in the ticket.

Actions #5

Updated by gstrauss over 7 years ago

  • Target version set to 1.4.40
Actions #6

Updated by gstrauss over 7 years ago

It turns out that mod_expire might add caching headers (based on URL) prior to a handler determining that http status is an error status (400-599)

Therefore, it would be more appropriate to reset Expires and Cache-Control headers if the connection_state_machine is going to produce the error document (or delegate to an error handler). ... It might actually be the right behavior to reset all response headers if the connection_state_machine is going to replace the response with an error document or delegate to an error handler.

Actions #7

Updated by gstrauss over 7 years ago

  • Status changed from New to Fixed

committed in 760baed4


Also available in: Atom