httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From William A Rowe Jr <wr...@rowe-clan.net>
Subject Re: svn commit: r1750953 - /httpd/httpd/trunk/server/util_script.c
Date Sat, 02 Jul 2016 14:39:20 GMT
Relevant data points...

https://tools.ietf.org/html/rfc7231#section-7.1.1.1

There is no other supported time zone except GMT representing GMT.  That is
the only value we may send.

"Recipients of timestamp values are encouraged to be robust in parsing
timestamps unless otherwise restricted by the field definition. For
example, messages are occasionally forwarded over HTTP from a non-HTTP
source that might generate any of the date and time specifications defined
by the Internet Message Format. "

We may accept an unusual timezone but should reformat as GMT in the
response.
On Jul 2, 2016 6:52 AM, "Yann Ylavic" <ylavic.dev@gmail.com> wrote:

> 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