I can't fault the logic... +1 for the patch.
> On Jun 19, 2018, at 6:47 AM, Rainer Jung <rainer.j...@kippdata.de> wrote:
>
> I have a situation where I have a caching Apache in front of a back end. The
> backend sends a response header "Expires: -1" and mod_cache unconditionally
> refuses to cache the response with the error "Broken expires header".
>
> RFC 7234 section 5.3 [1] contains the text:
>
> =======================================================
> ...
> The Expires value is an HTTP-date timestamp, as defined in Section 7.1.1.1 of
> [RFC7231].
>
> Expires = HTTP-date
>
> For example
>
> Expires: Thu, 01 Dec 1994 16:00:00 GMT
>
> A cache recipient MUST interpret invalid date formats, especially the value
> "0", as representing a time in the past (i.e., "already expired").
> ...
> =======================================================
>
> Furthermore:
>
> =======================================================
> ...
> If a response includes a Cache-Control field with the max-age directive
> (Section 5.2.2.8), a recipient MUST ignore the Expires field. Likewise, if a
> response includes the s-maxage directive (Section 5.2.2.9), a shared cache
> recipient MUST ignore the Expires field. In both these cases, the value in
> Expires is only intended for recipients that have not yet implemented the
> Cache-Control field.
> ...
> =======================================================
>
> I would like to make the following case a) to behave like case b):
>
> a) expires header contains no valid date format
>
> b) expires header contains a valid date format, but that date is in the past
>
> This is currently not the case. Case a) never caches, case b) does not cache
> by default, but caching can be forced by CacheStoreExpired in the
> configuration. Also max-age and s-maxage take precendence over expires in
> case b).
>
> Code archeology of mod_cache shows:
>
> 1) originally content with expires being an HTTP-date but that date is in the
> past was cached (case b) not handled RFC compliant). Content with invalid
> expires wasn't cached from the beginning (that's case a)).
>
> 2) r450055 (minfrin) added a check to refuse caching in case expires was an
> HTTP-date in the past (case b))
>
> 3) r1000106 (wrowe) added a config option the allow caching stale content but
> applied this option only to case b), not to case a), although the RFC says "A
> cache recipient MUST interpret invalid date formats, ..., as representing a
> time in the past (i.e., "already expired").".
>
> 4) r1003882 (minfrin) added more options to control more caching behavior by
> configuration, for example in case b).
>
> 5) r1726675 (covener) added max-age and s-maxage checks to case b).
>
> I guess that case a) simply wasn't on the radar when 3), 4) and 5) were added.
>
> I propose the following patch for trunk and 2.4. Of course in the above case
> a), caching behavior will not be completely the same (additional caching in
> case of broken Expires header if the config uses CacheStoreExpired of either
> max-age or a-maxage was send). The condition and comment additions was copied
> from the case b) "if" directly below the changed block:
>
> Index: modules/cache/mod_cache.c
> ===================================================================
> --- modules/cache/mod_cache.c (revision 1833803)
> +++ modules/cache/mod_cache.c (working copy)
> @@ -1040,8 +1040,11 @@
> if (reason) {
> /* noop */
> }
> - else if (exps != NULL && exp == APR_DATE_BAD) {
> - /* if a broken Expires header is present, don't cache it */
> + else if (!control.s_maxage && !control.max_age && !dconf->store_expired
> + && exps != NULL && exp == APR_DATE_BAD) {
> + /* if a broken Expires header is present, don't cache it
> + * Unless CC: s-maxage or max-age is present
> + */
> reason = apr_pstrcat(p, "Broken expires header: ", exps, NULL);
> }
> else if (!control.s_maxage && !control.max_age
>
>
> Thanks for any feedback!
>
> Regards,
>
> Rainer
>
> [1] https://tools.ietf.org/html/rfc7234#section-5.3
