Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <4778FAD9.2010708@apache.org>
Date: Mon, 31 Dec 2007 15:21:13 +0100
From: Ruediger Pluem <rpluem@apache.org>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.8.1.11) Gecko/20071128 SeaMonkey/1.1.7
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: segfault in dav_validate_request
References: <4776F2D4.2090008@metaparadigm.com>
 <20071231012607.2a5d2b49@grimnir>
In-Reply-To: <20071231012607.2a5d2b49@grimnir>
Content-Type: multipart/mixed;
 boundary="------------090808000606090405050707"

This is a multi-part message in MIME format.
--------------090808000606090405050707
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8bit


On 12/31/2007 02:26 AM, Nick Kew wrote:
> On Sun, 30 Dec 2007 09:22:28 +0800
> Michael Clark <michael@metaparadigm.com> wrote:
> 
>> I'm getting a segfault here in mod_dav from trunk
> 
> Oops ... more haste less speed:-(
> 
> I attach a revised patch (against current trunk).
> This is completely untested; just posting before going to bed,
> in case anyone feels like picking it up.  If not, I'll try
> and find time to revisit it tomorrow.
> 

I think the following patch against trunk which is based on yours
does it better, because:

- It does not set the ETag header permanently as setting it may not
  be desired for all responses. If we want to do this for more methods
  as we currently do we should do this explicitly in dav_method_* in
  mod_dav.c.
- Handles the case that hooks->getetag returns an empty ETag ("") like
  dav_fs_getetag does for non existing resources.
- Does remove the conditional between calling dav_meets_conditions and
  ap_meets_conditions. To be honest I do not understand why we should not
  call dav_meets_conditions in any case. Especially in the '*' case this
  make sense IMHO as we do not need to do a comparison to an existing
  ETag of the resource in this case. It is only of interest in this case
  whether the resource exists and not what its ETag is.


Passed perl test framework and litmus.

Regards

R�diger

--------------090808000606090405050707
Content-Type: text/x-patch;
 name="dav_if_handling.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="dav_if_handling.diff"

Index: modules/dav/main/util.c
===================================================================
--- modules/dav/main/util.c	(Revision 607709)
+++ modules/dav/main/util.c	(Arbeitskopie)
@@ -1467,6 +1467,8 @@
     dav_buffer work_buf = { 0 };
     dav_response *new_response;
     int resource_state;
+    const char *etag;
+    int set_etag = 0;
 
 #if DAV_DEBUG
     if (depth && response == NULL) {
@@ -1484,17 +1486,28 @@
         *response = NULL;
 
     /* 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);
+    etag = apr_table_get(r->headers_out, "ETag");
+    if (!etag) {
+        etag = (*resource->hooks->getetag)(resource);
+        if (etag && *etag) {
+            apr_table_set(r->headers_out, "ETag", etag);
+            set_etag = 1;
+        }
     }
-
-    resource_state = dav_get_resource_state(r, resource);
     /* Do the standard checks for conditional requests using
      * If-..-Since, If-Match etc */
-    if ((result = dav_meets_conditions(r, resource_state)) != OK) {
-        /* ### fix this up... how? */
+    resource_state = dav_get_resource_state(r, resource);
+    result = dav_meets_conditions(r, resource_state);
+    if (set_etag) {
+        /*
+         * If we have set an ETag to headers out above for
+         * dav_meets_conditions() revert this here as we do not want to set
+         * the ETag in responses to requests with methods where this might not
+         * be desired.
+         */
+        apr_table_unset(r->headers_out, "ETag");
+    }
+    if (result != OK) {
         return dav_new_error(r->pool, result, 0, NULL);
     }
 

--------------090808000606090405050707--