Date: Thu, 21 Aug 1997 08:14:01 -0400
From: coar@decus.org (Rodent of Unusual Size)
Subject: Re: [PATCH] (PR 894,895) ETags, Last-Modified, and scripts
To: New-HTTPd@apache.org, Coar@decus.org
Message-id: <97082108140101@decus.org>
Content-transfer-encoding: 7BIT
Sender: new-httpd-owner@apache.org
Precedence: bulk
Reply-To: new-httpd@apache.org

>From the fingers of "Roy T. Fielding" flowed the following:
>
>Sorry,

    No problem; your criticisms are constructive.  I *said* this was new
    ground for me.. <g> a learning experience.  At least you didn't call
    me a total ass-head for doing this at all, so I assume you feel
    there's *some* validity to the work.

>If a -u diff looks like gibberish, I'd prefer seeing a -c diff.
>I wish there was an easy way to test for gibberish.

    *narf*

>>+    mtime = (last_mtime != NULL) ? parseHTTPdate(last_mtime) : time(NULL);
>
>This is a bad plan.  You should be passing mtime to the function, not
>doing an expensive string parse of a string that was just created.

    You're making the assumptions that a) modification times are
    *always* a factor in *every* input condition check, and b) that
    meets_conditions() is *always* called right after set_{etag,lm).
    You're the HTTP cop; is [a] a valid assumption?  If it is, then it's
    reasonable to pass the mtime to meets_conditions(), which I intended
    as a general condition-check function.

>>-    if ((if_match = table_get(r->headers_in, "If-Match")) != NULL) {
>>-        if ((if_match[0] != '*') && !find_token(r->pool, if_match, etag))
>>-            return HTTP_PRECONDITION_FAILED;
>>+    if (etag != NULL) {
>>+	/*
>>+	 * If an If-Match request-header field was given
>>+	 * AND if our ETag does not match any of the entity tags in that field
>>+	 * AND the field value is not "*" (meaning match anything), then
>>+	 *     respond with a status of 412 (Precondition Failed).
>>+	 */
>>+
>>+	if ((if_match = table_get(r->headers_in, "If-Match")) != NULL) {
>>+	    if ((if_match[0] != '*') && !find_token(r->pool, if_match, etag)) {
>>+		return HTTP_PRECONDITION_FAILED;
>>+	    }
>>+	}
>>     }
>
>That incorrectly changes the condition.  If an If-Match was given and we
>have no etag, then the precondition failed.  The check for etag == NULL
>needs to be in the inner if before find_token, or within find_token.

    Ahhh.  Thanks!

>and the following appears to make xbithack_full mandatory.
>
>>--- mod_include.c	1997/08/18 13:12:13	1.47
>> 
>>+    set_last_modified(r, r->finfo.st_mtime);
>>+    set_etag(r, r->finfo.st_mtime);
>>     if (*state == xbithack_full
>> #if !defined(__EMX__) && !defined(WIN32)
>>     /*  OS/2 dosen't support Groups. */
>>         && (r->finfo.st_mode & S_IXGRP)
>> #endif
>>-        && (errstatus = set_last_modified (r, r->finfo.st_mtime)))
>>-        return errstatus;
>>+        && ((errstatus = meets_conditions(r)) != OK)) {
>>+	return errstatus;
>>+    }
>> 
>>     send_http_header(r);

    Mmm.  So this needs to turn into a nested if.

    Okey, a "take 2" will be forthcoming later to-day.

    #ken    :-)}