Broken If-Match handling and mod_dav

[Justin Erenkrantz <ju...@erenkrantz.com>]

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=16593

This bug states that a PUT with an If-Match condition to a mod_dav 
resource always fails.

The problem is that the set_headers mod_dav hook isn't necessarily 
called for dav_method_{put, post, etc, etc, etc}.  It is called for 
GET though when the special GET handling is enabled (see 
dav_method_get).  When we call ap_meets_condition in 
dav_validate_request for the PUT, we don't have an ETag header to 
satisfy the conditional request.  Therefore, PUT with If-Match always 
return a 412.

Calling the set_headers hook seems a bit bogus to me when that has 
nothing to do with the actual response.  I imagine we could fudge 
r->headers_out for the duration of ap_meets_condition and add the 
output of set_headers (which should contain ETag from the provider). 
(Hmm, the fact that mod_dav_fs doesn't implement set_headers may be 
harmful here.)

The problem here is that the output of set_headers indicates the 
original state of the resource, but by the time the request is over, 
we should probably have new resource state.  That leads me to a 
temporary r->headers_out, but it just seems wrong.

Another solution would be to set the ETag if it isn't there right 
before the ap_meets_conditions() call in dav_validate_resource using 
the getetag hook, and unset it if we set it temporarily.  But, ouch. 
(This does solve the mod_dav_fs problem though.)

Thoughts?  Is there another way?  -- justin

Re: Broken If-Match handling and mod_dav

[Greg Stein <gs...@lyra.org>]

On Sat, Feb 01, 2003 at 01:09:39AM -0800, Justin Erenkrantz wrote:
>...
> The problem is that the set_headers mod_dav hook isn't necessarily 
> called for dav_method_{put, post, etc, etc, etc}.  It is called for
>...
> The problem here is that the output of set_headers indicates the 
> original state of the resource, but by the time the request is over,
> we should probably have new resource state.  That leads me to a 
> temporary r->headers_out, but it just seems wrong.
> 
> Another solution would be to set the ETag if it isn't there right 
> before the ap_meets_conditions() call in dav_validate_resource using 
> the getetag hook, and unset it if we set it temporarily.  But, ouch. 
> (This does solve the mod_dav_fs problem though.)
> 
> Thoughts?  Is there another way?  -- justin

Problem problem problem. Um, did you stop to consider that
ap_meets_condition() is bogus? Checking an output header is the wrong part
about the situation.

The right answer is adding the ap_resource concept, then asking it for its
etag value.

Short of that? Maybe add a field into the request_rec. We can set it during
one of the lookup/walk steps or whatever. (whatever hook is selected will be
somewhat bogus since we don't have one called "determine resource")

Also note that the timing of dav_validate_request() is somewhat bogus. We
wait until we're actually handling the stupid thing. If mod_dav could
separate the lookup process, then we could do that, and put the validation
into another hook long before we ever determine that "handling" the request
is appropriate.
(ideally, you would never return something like 412 (or most other errors)
 from the handler)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/