Project

General

Profile

Bug #1919

mod_expires sends headers on 404 responses

Added by codemobs about 8 years ago. Updated 12 months ago.

Status:
Fixed
Priority:
Normal
Assignee:
-
Category:
mod_expire
Target version:
Start date:
2009-02-26
Due date:
% Done:

0%

Estimated time:
1.00 h
Missing in 1.5.x:

Description

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 View - Rudimentary patch to ignore responses of status codes 400-599. (336 Bytes) codemobs, 2009-02-26 01:36

Associated revisions

Revision 760baed4 (diff)
Added by gstrauss 12 months ago

[mod_expire] reset caching response headers for error docs (fixes #1919)

remove Cache-Control and Expires headers before handling error docs
(caching headers may have been set by mod_expire before http status
was determined to be an error)

x-ref:
"mod_expires sends headers on 404 responses"
https://redmine.lighttpd.net/issues/1919

History

#1 Updated by gstrauss about 1 year 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);

#2 Updated by stbuehler about 1 year 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.

#3 Updated by gstrauss about 1 year 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.)

#4 Updated by gstrauss about 1 year 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.

#5 Updated by gstrauss 12 months ago

  • Target version set to 1.4.40

#6 Updated by gstrauss 12 months 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.

#7 Updated by gstrauss 12 months ago

  • Status changed from New to Fixed

committed in 760baed4

Also available in: Atom