Lighttpd

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);

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.

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.)

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.

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.