Bug 53910

Summary: If: check spuriously succeeds with %-encoded URL and ETag qualifier
Product: Apache httpd-2 Reporter: Timothy Wood <tjw>
Component: mod_davAssignee: Apache HTTPD Bugs Mailing List <bugs>
Severity: normal Keywords: FixedInTrunk, PatchAvailable
Priority: P2    
Version: 2.4.3   
Target Milestone: ---   
Hardware: Macintosh   
OS: other   
Attachments: Patch

Description Timothy Wood 2012-09-21 04:58:20 UTC
Created attachment 29401 [details]

Using a clean build of httpd-2.4.3, I'm seeing the following behavior:

 * Make a collection
 * Get its ETag with PROPFIND Depth=0
 * Add an item to the collection
 * httpd reports a new ETag for the collection at this point
 * Attempt a MOVE of the collection to a new name, predicated with the OLD ETag, using an "If" header of </src/> (["ETag"])
 * Expect a precondition failure error

I get a proper 412 precondition failure if the source and destination are "/src/" and "/dst/", but if I use percent escapes in the URLs the MOVE spuriously succeeds (for example, if I use "/s%20r%20c/" and "/d%20s%20t/".

I've you'd like to see or try specifically my case, I've submitted a patch to litmus <http://lists.manyfish.co.uk/pipermail/litmus/2012-September/000344.html>.

A possible patch to fix the problem is attached -- when parsing the If header, make sure to unescape the URI in the If header. dav_validate_resource_state() compare a resource URI (which was already unescaped) with a dav_if_header's URI (which was not unescaped while parsing the If header).
Comment 1 Wim Lewis 2013-04-23 01:17:52 UTC
Some notes:

It isn't necessary for the resource being moved to be a collection; it can be a plain file as well and the bug is the same.
The bug is still in trunk/2.5 r1470683, and the patch still fixes it there.
The bug is still in 2.4.4, and the patch still fixes it there.
Comment 2 Graham Leggett 2013-04-23 13:16:54 UTC
Patch applied in http://svn.apache.org/r1470940.
Comment 3 Graham Leggett 2013-04-27 19:09:59 UTC
Can you verify whether trunk works for you?

Proposed for backport to v2.4.
Comment 4 Wim Lewis 2013-04-30 00:24:52 UTC
The change in r1470940 doesn't quite work --- the the call to ap_unescape_url() was moved to after the computation of uri_len, which ended up leaving dav_if_header->uri_len inconsistent.

I assume the reason was to do ap_getparents() before unescaping? Doing the unescaping after ap_getparents() but before computing uri_len works.
Comment 5 Graham Leggett 2013-04-30 14:11:44 UTC
Fixed in r1477530, backport proposal updated.
Comment 6 Graham Leggett 2013-05-23 13:17:53 UTC
Proposed for backport to v2.2.
Comment 7 Graham Leggett 2013-05-26 19:43:57 UTC
Backported to v2.4.5.
Comment 8 Christophe JAILLET 2014-08-19 08:29:04 UTC
Fixed and released in 2.2.25 and 2.4.5