Bug 54610

Summary: COPY doesn't respect If/If-Modified/etc
Product: Apache httpd-2 Reporter: Timothy Wood <tjw>
Component: mod_davAssignee: Apache HTTPD Bugs Mailing List <bugs>
Status: RESOLVED FIXED    
Severity: normal Keywords: FixedInTrunk
Priority: P2    
Version: 2.4.3   
Target Milestone: ---   
Hardware: PC   
OS: Mac OS X 10.4   

Description Timothy Wood 2013-02-26 04:09:30 UTC
Sending a If or If-Match header with an invalid ETag doesn't result in a 412 Precondition Failed.

<http://www.webdav.org/specs/rfc4918.html#METHOD_COPY> has an example specifically calling out preconditions on COPY (10.4.9), for example, but I'm just doing a simple If-Match.

This patch (vs our local clone of 2.4.3) resolves the issue for me, but I'm wondering if there is some case this would break.

Thanks!

-tim

Index: modules/dav/main/mod_dav.c
===================================================================
--- modules/dav/main/mod_dav.c	(revision 180376)
+++ modules/dav/main/mod_dav.c	(revision 180377)
@@ -2761,10 +2761,10 @@
    }

    /*
-     * Check If-Headers and existing locks for each resource in the source
-     * if we are performing a MOVE. We will return a 424 response with a
-     * DAV:multistatus body. The multistatus responses will contain the
-     * information about any resource that fails the validation.
+     * Check If-Headers and existing locks for each resource in the source.
+     * We will return a 424 response with a DAV:multistatus body.
+     * The multistatus responses will contain the information about any
+     * resource that fails the validation.
     *
     * We check the parent resource, too, since this is a MOVE. Moving the
     * resource effectively removes it from the parent collection, so we
@@ -2773,17 +2773,17 @@
     * If a problem occurs with the Request-URI itself, then a plain error
     * (rather than a multistatus) will be returned.
     */
-    if (is_move
-        && (err = dav_validate_request(r, resource, depth, NULL,
+    if ((err = dav_validate_request(r, resource, depth, NULL,
                                       &multi_response,
                                       DAV_VALIDATE_PARENT
                                       | DAV_VALIDATE_USE_424,
                                       NULL)) != NULL) {
        err = dav_push_error(r->pool, err->status, 0,
                             apr_psprintf(r->pool,
-                                          "Could not MOVE %s due to a failed "
+                                          "Could not %s %s due to a failed "
                                          "precondition on the source "
                                          "(e.g. locks).",
+                                          is_move ? "MOVE" : "COPY",
                                          ap_escape_html(r->pool, r->uri)),
                             err);
        return dav_handle_err(r, err, multi_response);
Comment 1 Wim Lewis 2013-04-23 01:45:17 UTC
This bug still exists in 2.4.4, and this patch still fixes it there.

This bug still exists in trunk/2.5 r1470737, and this patch still fixes it there (although you have to apply it with -l because indentation has changed in that file).
Comment 2 Graham Leggett 2013-04-27 15:40:40 UTC
Fix on trunk in r1476604.
Comment 3 Graham Leggett 2013-05-23 13:18:49 UTC
Proposed for backport to v2.4 and v2.2.
Comment 4 Graham Leggett 2013-05-26 19:50:29 UTC
Backported to v2.4.5.
Comment 5 Ben Reser 2013-10-03 05:38:18 UTC
Marking as fixed since this has been released in 2.2.25 and 2.4.6.

Note that the change for this actually spawned two addition issues PR 55304 and PR 55306, which have not been included in a released version yet.  I'd encourage people interested in this issue to investigate the changes made in response to those issues to make sure that they didn't break something else.