Merge r1529559, r1531505 from trunk:

Fix PR 55397: dav_resource->uri treated as an unparsed uri.

The change made for PR 54611 caused this field to be treated as
unescaped.  mod_dav_svn however, provided escaped URIs.  Essentially
breaking support for paths with non-URI safe characters in SVN.

Adjust the code so that dav_resource->uri is assumed to be escaped and
adjust mod_dav_fs so that it uses escaped URIs in this field.

* modules/dav/fs/repos.c
  (dav_fs_get_resource): Use the unparsed_uri to contruct the resource uri.

* modules/dav/main/mod_dav.c
  (dav_xml_escape_uri): Do not uri escape, just handle xml escaping.
  (dav_created): Assume that locn if provided is escaped.
  (dav_method_copymove, dav_method_bind): Use the unparsed_uri on the request
    when calling dav_created() to adjust to locn assuming it is escaped.

* modules/dav/main/mod_dav.h
  (dav_resource): Document that uri is escaped.

Followup to r1529559: mod_dav_fs: Fix encoding of hrefs in PROPFIND response.

Previous commit missed encoding the names of the children of the PROPFIND
request when the depth wasn't 0.

* modules/dav/fs/repos.c
  (dav_fs_append_uri): New function
  (dav_fs_walker): Use dav_fs_append_uri() and adjust length calculations to
    use the encoded length.

Submitted by: breser
Reviewed/backported by: jim

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@1540730 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 79e272dc7177e23f5883ca280f21350da1b7ef11..0f3088781860b4ee8a4282f484bc6b86f243e303 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.2.26
 
+  *) mod_dav: dav_resource->uri treated as unencoded. This was an
+     unnecessary ABI changed introduced in 2.2.25  PR 55397.  [Ben Reser]
+
   *) mod_dav: Do not validate locks against parent collection of COPY
      source URI.  PR 55304.  [Ben Reser]
 
@@ -19,6 +22,9 @@ Changes with Apache 2.2.26
      support for SSLv2. Problem was introduced in 2.2.25. PR 55194.
      [Rainer Jung, Kaspar Brand]
 
+  *) mod_dav: Fix double encoding of URIs in XML and Location header (caused
+     by unintential ABI change in 2.2.25).  PR 55397.  [Ben Reser] 
+
 Changes with Apache 2.2.25
 
   *) SECURITY: CVE-2013-1896 (cve.mitre.org)

diff --git a/STATUS b/STATUS
index 74772fa003082a6435ab88459758321bc52d9adc..fc10f7c989da6defecb80078aa303e5732ea2f90 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -105,14 +105,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
     2.2.x patch: trunk patch to ssl_engine_config.c (above) applies with offset
     +1: trawick, jorton, wrowe
 
-  * mod_dav: Fix 55397.  dav_resource->uri treated as unencoded. This was an
-    unnecessary ABI changed introduced in 2.2.25
-    trunk patches: https://svn.apache.org/r1529559
-                   https://svn.apache.org/r1531505
-    2.4.x: trunk works, CHANGES needs to be written when merging
-    2.2.x: https://people.apache.org/~breser/httpd/2.2.x/patches/pr55397.patch 
-    +1: breser, minfrin, wrowe
-
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

diff --git a/modules/dav/fs/repos.c b/modules/dav/fs/repos.c
index 0502808c652512c6e147619ee7a6ebcba97482f3..02daca0b79c5b9541efd165b1f0c873a4a87a2a6 100644 (file)
--- a/modules/dav/fs/repos.c
+++ b/modules/dav/fs/repos.c
@@ -683,14 +683,13 @@ static dav_error * dav_fs_get_resource(
     resource->pool = r->pool;
 
     /* make sure the URI does not have a trailing "/" */
-    len = strlen(r->uri);
-    if (len > 1 && r->uri[len - 1] == '/') {
-        s = apr_pstrdup(r->pool, r->uri);
-        s[len - 1] = '\0';
+    len = strlen(r->unparsed_uri);
+    if (len > 1 && r->unparsed_uri[len - 1] == '/') {
+        s = apr_pstrmemdup(r->pool, r->unparsed_uri, len-1);
         resource->uri = s;
     }
     else {
-        resource->uri = r->uri;
+        resource->uri = r->unparsed_uri;
     }
 
     if (r->finfo.filetype != 0) {
@@ -1407,6 +1406,18 @@ static dav_error * dav_fs_remove_resource(dav_resource *resource,
     return dav_fs_deleteset(info->pool, resource);
 }
 
+/* Take an unescaped path component and escape it and append it onto a
+ * dav_buffer for a URI */
+static apr_size_t dav_fs_append_uri(apr_pool_t *p, dav_buffer *pbuf,
+                                    const char *path, apr_size_t pad)
+{
+    const char *epath = ap_escape_uri(p, path);
+    apr_size_t epath_len = strlen(epath);
+
+    dav_buffer_place_mem(p, pbuf, epath, epath_len + 1, pad);
+    return epath_len;
+}
+
 /* ### move this to dav_util? */
 /* Walk recursively down through directories, *
  * including lock-null resources as we go.    */
@@ -1461,6 +1472,7 @@ static dav_error * dav_fs_walker(dav_fs_walker_context *fsctx, int depth)
     }
     while ((apr_dir_read(&dirent, APR_FINFO_DIRENT, dirp)) == APR_SUCCESS) {
         apr_size_t len;
+        apr_size_t escaped_len;
         apr_status_t status;
 
         len = strlen(dirent.name);
@@ -1500,7 +1512,7 @@ static dav_error * dav_fs_walker(dav_fs_walker_context *fsctx, int depth)
 
         /* copy the file to the URI, too. NOTE: we will pad an extra byte
            for the trailing slash later. */
-        dav_buffer_place_mem(pool, &fsctx->uri_buf, dirent.name, len + 1, 1);
+        escaped_len = dav_fs_append_uri(pool, &fsctx->uri_buf, dirent.name, 1);
 
         /* if there is a secondary path, then do that, too */
         if (fsctx->path2.buf != NULL) {
@@ -1533,7 +1545,7 @@ static dav_error * dav_fs_walker(dav_fs_walker_context *fsctx, int depth)
             fsctx->path2.cur_len += len;
 
             /* adjust URI length to incorporate subdir and a slash */
-            fsctx->uri_buf.cur_len += len + 1;
+            fsctx->uri_buf.cur_len += escaped_len + 1;
             fsctx->uri_buf.buf[fsctx->uri_buf.cur_len - 1] = '/';
             fsctx->uri_buf.buf[fsctx->uri_buf.cur_len] = '\0';
 
@@ -1599,8 +1611,8 @@ static dav_error * dav_fs_walker(dav_fs_walker_context *fsctx, int depth)
             */
             dav_buffer_place_mem(pool, &fsctx->path1,
                                  fsctx->locknull_buf.buf + offset, len + 1, 0);
-            dav_buffer_place_mem(pool, &fsctx->uri_buf,
-                                 fsctx->locknull_buf.buf + offset, len + 1, 0);
+            dav_fs_append_uri(pool, &fsctx->uri_buf,
+                              fsctx->locknull_buf.buf + offset, 0);
             if (fsctx->path2.buf != NULL) {
                 dav_buffer_place_mem(pool, &fsctx->path2,
                                      fsctx->locknull_buf.buf + offset,

diff --git a/modules/dav/main/mod_dav.c b/modules/dav/main/mod_dav.c
index ddb9af01a9ce39c403c326f74cdcb5781eefd338..ba0117e51ee9edffd1a5e6669fe34f0e1d479d04 100644 (file)
--- a/modules/dav/main/mod_dav.c
+++ b/modules/dav/main/mod_dav.c
@@ -400,11 +400,9 @@ static int dav_error_response_tag(request_rec *r,
  */
 static const char *dav_xml_escape_uri(apr_pool_t *p, const char *uri)
 {
-    const char *e_uri = ap_escape_uri(p, uri);
-
     /* check the easy case... */
-    if (ap_strchr_c(e_uri, '&') == NULL)
-        return e_uri;
+    if (ap_strchr_c(uri, '&') == NULL)
+        return uri;
 
     /* there was a '&', so more work is needed... sigh. */
 
@@ -412,7 +410,7 @@ static const char *dav_xml_escape_uri(apr_pool_t *p, const char *uri)
      * Note: this is a teeny bit of overkill since we know there are no
      * '<' or '>' characters, but who cares.
      */
-    return apr_xml_quote_string(p, e_uri, 0);
+    return apr_xml_quote_string(p, uri, 0);
 }
 
 
@@ -616,7 +614,8 @@ static int dav_handle_err(request_rec *r, dav_error *err,
     return DONE;
 }
 
-/* handy function for return values of methods that (may) create things */
+/* handy function for return values of methods that (may) create things.
+ * locn if provided is assumed to be escaped. */
 static int dav_created(request_rec *r, const char *locn, const char *what,
                        int replaced)
 {
@@ -624,8 +623,6 @@ static int dav_created(request_rec *r, const char *locn, const char *what,
 
     if (locn == NULL) {
         locn = r->unparsed_uri;
-    } else {
-        locn = ap_escape_uri(r->pool, locn);
     }
 
     /* did the target resource already exist? */
@@ -2972,7 +2969,7 @@ static int dav_method_copymove(request_rec *r, int is_move)
     }
 
     /* return an appropriate response (HTTP_CREATED or HTTP_NO_CONTENT) */
-    return dav_created(r, lookup.rnew->uri, "Destination",
+    return dav_created(r, lookup.rnew->unparsed_uri, "Destination",
                        resnew_state == DAV_RESOURCE_EXISTS);
 }
 
@@ -4562,7 +4559,7 @@ static int dav_method_bind(request_rec *r)
 
     /* return an appropriate response (HTTP_CREATED) */
     /* ### spec doesn't say what happens when destination was replaced */
-    return dav_created(r, lookup.rnew->uri, "Binding", 0);
+    return dav_created(r, lookup.rnew->unparsed_uri, "Binding", 0);
 }
 
 

diff --git a/modules/dav/main/mod_dav.h b/modules/dav/main/mod_dav.h
index 8f2560dce041ce5cf4d38af4c6471c7bcb83f8d2..64e0e22764ee0d92ece2e60f029ebf79ae218aba 100644 (file)
--- a/modules/dav/main/mod_dav.h
+++ b/modules/dav/main/mod_dav.h
@@ -370,7 +370,7 @@ typedef struct dav_resource {
                          * REGULAR and WORKSPACE resources,
                          * and is always 1 for WORKING */
 
-    const char *uri;    /* the URI for this resource */
+    const char *uri;    /* the escaped URI for this resource */
 
     dav_resource_private *info;         /* the provider's private info */