httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: svn commit: r1750953 - /httpd/httpd/trunk/server/util_script.c
Date Sat, 02 Jul 2016 11:51:55 GMT
On Sat, Jul 2, 2016 at 2:05 AM, Luca Toscano <toscano.luca@gmail.com> wrote:
>
> We have discussed it briefly in another email but didn't reach a conclusion,
> so I am really happy to re-discuss it again. Maybe an example would clarify
> what a user will see in the logs.

How about (modulo quick, dirty and not even compile tested errors):

Index: server/util_script.c
===================================================================
--- server/util_script.c    (revision 1750971)
+++ server/util_script.c    (working copy)
@@ -665,29 +665,28 @@ AP_DECLARE(int) ap_scan_script_header_err_core_ex(
          * pass it on blindly because of restrictions on future or
invalid values.
          */
         else if (!ap_cstr_casecmp(w, "Last-Modified")) {
-            apr_time_t last_modified_date = apr_date_parse_http(l);
-            if (last_modified_date != APR_DATE_BAD) {
-                ap_update_mtime(r, last_modified_date);
-                ap_set_last_modified(r);
-                if (APLOGrtrace1(r)) {
-                    const char* datestr = apr_table_get(r->headers_out,
-                                                        "Last-Modified");
-                    apr_time_t timestamp = apr_date_parse_http(datestr);
-                    if (timestamp < last_modified_date) {
-                        char *last_modified_datestr = apr_palloc(r->pool,
-
APR_RFC822_DATE_LEN);
-                        apr_rfc822_date(last_modified_datestr,
last_modified_date);
-                        ap_log_rerror(SCRIPT_LOG_MARK, APLOG_TRACE1, 0, r,
-                                      "The Last-Modified header value '%s' "
-                                      "(parsed as RFC822/RFC1123 datetime, "
-                                      "that assumes the GMT timezone) "
-                                      "has been replaced with: '%s'. "
-                                      "An origin server with a clock
must not send "
-                                      "a Last-Modified date that is
later than the "
-                                      "server's time of message origination.",
-                                      last_modified_datestr, datestr);
+            apr_time_t last_modified_date_rfc = apr_date_parse_rfc(l);
+            if (last_modified_date_rfc != APR_DATE_BAD) {
+                apr_time_t now = apr_time_now(),
+                           last_modified_date_gmt = last_modified_date_rfc;
+                if (last_modified_date_rfc != apr_date_parse_http(l)) {
+                    if (be_strict) {
+                        /* log an error about non-GMT Last-Modified */
+                        return HTTP_BAD_GATEWAY;
                     }
+                    if (last_modified_date_rfc <= now) {
+                        /* be liberal, log a trace1 about forcing gmt format,
+                         * i.e. l -> apr_rfc822_date(last_modified_date_rfc),
+                         * and then fall through
+                         */
+                    }
                 }
+                if (last_modified_date_rfc > now) {
+                    /* log about Last-Modified in the future (debug/info?) */
+                    last_modified_date_gmt = now;
+                }
+                ap_update_mtime(r, last_modified_date_gmt);
+                ap_set_last_modified(r);
             }
             else {
                 if (APLOGrtrace1(r))
_

Now we use apr_date_parse_rfc(), instead of apr_date_parse_http(), and
hence parse the GMT offset such that we can detect whether the
original Last-Modified is GMT or not.

If not and we have to be strict return BAD_GATEWAY, otherwise
(lenient) rewrite the Last-Modified header to its GMT form.

We can also explicitely check if it is in the future (from httpd POV),
and force it to 'now' in this case (if that's the correct behaviour).

>
> FCGI script returning the following header (Europe/Paris timezone):
>
> Last-Modified: Sat, 02 Jul 2016 01:49:27 +0200
>
> The current proposed logging (givent a proper LogLevel setting) would be:
>
> [Fri Jul 01 23:49:29.173823 2016] [proxy_fcgi:trace4] [pid 3542:tid
> 140560966862592] util_script.c(564): [client ::1:52263]   Last-Modified:
> Sat, 02 Jul 2016 01:49:27 +0200
>
> [Fri Jul 01 23:49:29.173833 2016] [proxy_fcgi:trace1] [pid 3542:tid
> 140560966862592] util_script.c(688): [client ::1:52263] The Last-Modified
> header value 'Sat, 02 Jul 2016 01:49:27 GMT' (parsed as RFC882/RFC1123
> datetime, that assumes the GMT timezone) has been replaced with: 'Fri, 01
> Jul 2016 23:49:29 GMT'. An origin server with a clock must not send a
> Last-Modified date that is later than the server's time of message
> origination.

The issue with the original code is that it does not really detect if
the Last-Modified header is in the future since it ignores the
timezone/offset.
The RFC does not tell us either what we should do if a timezone
different than GMT is specified (I read it as: if there is no timezone
assume GMT, but here we have one).
The options IMHO are, as always, be strict (BAD_GATEWAY) or liberal in
what we get and strict in what we set...

Regards,
Yann.

Mime
View raw message