On 12/30/2007 07:16 AM, Chris Darroch wrote:
> Michael Clark wrote:
>
>> I'm getting a segfault here in mod_dav from trunk (after a make clean)
>> running litmus using extras/httpd-dav.conf whereas it was working for
>> me last night. Not sure if this a work-in-progress. No time to file a
>> bug right now as i'm off for the weekend.
>
>> /* Set the ETag header required by dav_meets_conditions() */
>> -> if ((err = (*resource->hooks->set_headers)(r, resource)) != NULL) {
>> return dav_push_error(r->pool, err->status, 0,
>> "Unable to set up HTTP headers.",
>> err);
>> }
>
> I think the set_headers hook for mod_dav_fs is a NULL pointer unless
> DEBUG_GET_HANDLER in dav/fs/repos.c is non-zero, which it isn't by
> default. Using the getetag hook should work to make sure the ETag
> is in place before calling ap_meets_conditions(), though, if I read the
> code correctly. The originally proposed patch in #38034 does this, IIRC.
Thanks to both of you for debugging. I dug somewhat deeper and as I already
mentioned with the current state of trunk we would sent headers in responses
to methods where we did not before the patch. Before the patch
set_headers was only called for the methods GET and SEARCH and thus e.g.
an ETag was only set in responses to these methods (and in the GET case for
mod_dav_fs this ETag would get created via the default handler and not via
set_headers).
Especially we set no ETag header in the response to PUT requests
(validated at least with mod_dav_fs, but I suppose the same is true for all
other providers (e.g. subversion)). I do not know whether it is wrong not to
set an ETag header in a PUT response, but from what I read from RFC4918 there
is no requirement to do so. So I would not recommend to change this behaviour.
So the best way for me to move on seems to back out the patch and apply
a (possibly modified) version of the originally proposed patch.
Regards
Rüdiger
