You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Tom Tromey <tr...@creche.cygnus.com> on 1996/09/23 19:12:15 UTC

another http/1.0 bug in set_last_modified

I've found another HTTP/1.0 bug in set_last_modified.  This bug is in
Apache 1.1.1; the current CVS version has a slightly different
incarnation of the bug (which is apparently a bug wrt the HTTP/1.1
spec as well).

Here is the text from section 10.10 ("Last-Modified") of the HTTP/1.0
spec:

  An origin server must not send a Last-Modified date which is later
  than the server's time of message origination. In such cases, where
  the resource's last modification would indicate some time in the
  future, the server must replace that date with the message
  origination date.

My test suite checks this by making a file with a date in the future,
and then comparing the returned Last-Modified and Date fields.

Appended is a patch (relative to all my other patches to this
function) which fixes this bug.

In the current CVS Apache, set_last_modified just returns if the
file's mtime is in the future.  By my reading of the above section,
this is incorrect.  Draft 07 says the same thing as the 1.0 spec
(except that "must" and "must not" are in all caps).  A patch for the
newer Apache should remove the "just return" code, and do something
like the appended patch.

Tom
-- 
tromey@cygnus.com                 Member, League for Programming Freedom

Index: http_protocol.c
===================================================================
RCS file: /rel/cvsfiles/devo/apache/src/http_protocol.c,v
retrieving revision 1.3
diff -c -5 -r1.3 http_protocol.c
*** http_protocol.c	1996/09/19 20:26:29	1.3
--- http_protocol.c	1996/09/23 17:00:54
***************
*** 196,206 ****
       * to have it cached.
       */
      
      if (r->no_cache) return OK;
      
!     ts = gm_timestr_822(r->pool, mtime);
      table_set (r->headers_out, "Last-modified", ts);
  
      /* Check for conditional GETs --- note that we only want this check
       * to succeed if the GET was successful; ErrorDocuments *always* get sent.
       */
--- 196,206 ----
       * to have it cached.
       */
      
      if (r->no_cache) return OK;
      
!     ts = gm_timestr_822(r->pool, (mtime > server_time) ? server_time : mtime);
      table_set (r->headers_out, "Last-modified", ts);
  
      /* Check for conditional GETs --- note that we only want this check
       * to succeed if the GET was successful; ErrorDocuments *always* get sent.
       */

Re: another http/1.0 bug in set_last_modified

Posted by Brian Behlendorf <br...@organic.com>.
I think this patch to 1.2-dev fixes the problem.  Main issue - server_time no
longer available, now it's r->request_time (I think - can anyone confirm?)

	Brian

taz [265] cvs diff -C3 http_protocol.c
Index: http_protocol.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_protocol.c,v
retrieving revision 1.47
diff -C3 -r1.47 http_protocol.c
*** http_protocol.c     1996/09/24 12:44:57     1.47
--- http_protocol.c     1996/09/28 01:28:17
***************
*** 331,337 ****
      /* Invalid, future time... just ignore it */
      if (mtime > r->request_time) return OK;
  
!     ts = gm_timestr_822(r->pool, mtime);
      table_set (r->headers_out, "Last-Modified", ts);
  
      /* Make an ETag header out of various peices of information. We use
--- 331,337 ----
      /* Invalid, future time... just ignore it */
      if (mtime > r->request_time) return OK;
  
!     ts = gm_timestr_822(r->pool, (mtime > r->request_time) ? r->request_time : mtime);
      table_set (r->headers_out, "Last-Modified", ts);
  
      /* Make an ETag header out of various peices of information. We use





On 23 Sep 1996, Tom Tromey wrote:
> I've found another HTTP/1.0 bug in set_last_modified.  This bug is in
> Apache 1.1.1; the current CVS version has a slightly different
> incarnation of the bug (which is apparently a bug wrt the HTTP/1.1
> spec as well).
> 
> Here is the text from section 10.10 ("Last-Modified") of the HTTP/1.0
> spec:
> 
>   An origin server must not send a Last-Modified date which is later
>   than the server's time of message origination. In such cases, where
>   the resource's last modification would indicate some time in the
>   future, the server must replace that date with the message
>   origination date.
> 
> My test suite checks this by making a file with a date in the future,
> and then comparing the returned Last-Modified and Date fields.
> 
> Appended is a patch (relative to all my other patches to this
> function) which fixes this bug.
> 
> In the current CVS Apache, set_last_modified just returns if the
> file's mtime is in the future.  By my reading of the above section,
> this is incorrect.  Draft 07 says the same thing as the 1.0 spec
> (except that "must" and "must not" are in all caps).  A patch for the
> newer Apache should remove the "just return" code, and do something
> like the appended patch.
> 
> Tom
> -- 
> tromey@cygnus.com                 Member, League for Programming Freedom
> 
> Index: http_protocol.c
> ===================================================================
> RCS file: /rel/cvsfiles/devo/apache/src/http_protocol.c,v
> retrieving revision 1.3
> diff -c -5 -r1.3 http_protocol.c
> *** http_protocol.c	1996/09/19 20:26:29	1.3
> --- http_protocol.c	1996/09/23 17:00:54
> ***************
> *** 196,206 ****
>        * to have it cached.
>        */
>       
>       if (r->no_cache) return OK;
>       
> !     ts = gm_timestr_822(r->pool, mtime);
>       table_set (r->headers_out, "Last-modified", ts);
>   
>       /* Check for conditional GETs --- note that we only want this check
>        * to succeed if the GET was successful; ErrorDocuments *always* get sent.
>        */
> --- 196,206 ----
>        * to have it cached.
>        */
>       
>       if (r->no_cache) return OK;
>       
> !     ts = gm_timestr_822(r->pool, (mtime > server_time) ? server_time : mtime);
>       table_set (r->headers_out, "Last-modified", ts);
>   
>       /* Check for conditional GETs --- note that we only want this check
>        * to succeed if the GET was successful; ErrorDocuments *always* get sent.
>        */
> 
> 

--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
brian@organic.com  www.apache.org  hyperreal.com  http://www.organic.com/JOBS