httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wr...@apache.org
Subject svn commit: r1611189 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS modules/dav/fs/repos.c modules/dav/main/mod_dav.c modules/dav/main/mod_dav.h modules/dav/main/props.c
Date Wed, 16 Jul 2014 21:03:30 GMT
Author: wrowe
Date: Wed Jul 16 21:03:30 2014
New Revision: 1611189

URL: http://svn.apache.org/r1611189
Log:
Fix PR 56480: PROPFIND walker doesn't encode hrefs properly

Reverts r1529559 partially (specifically the dav_xml_escape_uri) bit.
Reverts r1531505 entirely.

* modules/dav/main/mod_dav.c
  (dav_xml_escape_uri): Revert the piece of r1529559 that removes the URI
    escaping from this function.

* modules/dav/main/props.c
  (dav_do_prop_subreq): Escape the URI before doing a sub request with it.
    This resolves some properties like getcontenttype from failing to be
    returned for files that contain characters that require encoding in their
    path.

* modules/dav/main/mod_dav.h
  (dav_resource): Note the inconsistency in the documentation.

* modules/dav/fs/repos.c
  (dav_fs_get_resource): Don't use the unparsed_uri to set the uri field of
    the resource.  This is the correct fix for the double encoding in mod_dav_fs
    that led to the dav_xml_escape_uri() change and r1531505.
  (dav_fs_walker, dav_fs_append_uri): Revert r1531505 changes.

Submitted by: breser
PR: 56480
Backports: r1602338
Reviewed by: breser, rpluem, ylavic

Modified:
    httpd/httpd/branches/2.2.x/CHANGES
    httpd/httpd/branches/2.2.x/STATUS
    httpd/httpd/branches/2.2.x/modules/dav/fs/repos.c
    httpd/httpd/branches/2.2.x/modules/dav/main/mod_dav.c
    httpd/httpd/branches/2.2.x/modules/dav/main/mod_dav.h
    httpd/httpd/branches/2.2.x/modules/dav/main/props.c

Modified: httpd/httpd/branches/2.2.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?rev=1611189&r1=1611188&r2=1611189&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Wed Jul 16 21:03:30 2014
@@ -1,4 +1,4 @@
-                                                         -*- coding: utf-8 -*-
+                                                         -*- coding: utf-8 -*-
 Changes with Apache 2.2.28
 
   *) SECURITY: CVE-2014-0231 (cve.mitre.org)
@@ -14,6 +14,8 @@ Changes with Apache 2.2.28
      Fix a race condition in scoreboard handling, which could lead to
      a heap buffer overflow.  [Joe Orton, Eric Covener, Jeff Trawick]
 
+  *) mod_dav: Fix improper encoding in PROPFIND responses.  PR 56480.
+
   *) mod_ssl: Extend the scope of SSLSessionCacheTimeout to sessions
      resumed by TLS session resumption (RFC 5077). [Rainer Jung]
 

Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=1611189&r1=1611188&r2=1611189&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Wed Jul 16 21:03:30 2014
@@ -99,17 +99,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-   * mod_dav: PROPFIND not encoding hrefs properly.  PR 56480.
-              This was an unintentional regression in the PR 55397 fixes previously
-              released to fix the regressions caused by PR 54611.  Causes problems with
-              mod_dav_svn.  Subversion's trunk test suite will show an XPASS test when
-              this is fixed.
-     trunk patch: http://svn.apache.org/r1602338
-     2.2.x patch: run svn merge -c 1602338 ^/httpd/httpd/trunk
-                  and add the following CHANGES entry:
-     *) mod_dav: Fix improper encoding in PROPFIND responses.  PR 56480.
-     +1: breser, rpluem, ylavic
-
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

Modified: httpd/httpd/branches/2.2.x/modules/dav/fs/repos.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/dav/fs/repos.c?rev=1611189&r1=1611188&r2=1611189&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/dav/fs/repos.c (original)
+++ httpd/httpd/branches/2.2.x/modules/dav/fs/repos.c Wed Jul 16 21:03:30 2014
@@ -683,13 +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->unparsed_uri);
-    if (len > 1 && r->unparsed_uri[len - 1] == '/') {
-        s = apr_pstrmemdup(r->pool, r->unparsed_uri, len-1);
+    len = strlen(r->uri);
+    if (len > 1 && r->uri[len - 1] == '/') {
+        s = apr_pstrmemdup(r->pool, r->uri, len-1);
         resource->uri = s;
     }
     else {
-        resource->uri = r->unparsed_uri;
+        resource->uri = r->uri;
     }
 
     if (r->finfo.filetype != 0) {
@@ -1406,18 +1406,6 @@ static dav_error * dav_fs_remove_resourc
     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.    */
@@ -1472,7 +1460,6 @@ static dav_error * dav_fs_walker(dav_fs_
     }
     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);
@@ -1512,7 +1499,7 @@ static dav_error * dav_fs_walker(dav_fs_
 
         /* copy the file to the URI, too. NOTE: we will pad an extra byte
            for the trailing slash later. */
-        escaped_len = dav_fs_append_uri(pool, &fsctx->uri_buf, dirent.name, 1);
+        dav_buffer_place_mem(pool, &fsctx->uri_buf, dirent.name, len + 1, 1);
 
         /* if there is a secondary path, then do that, too */
         if (fsctx->path2.buf != NULL) {
@@ -1545,7 +1532,7 @@ static dav_error * dav_fs_walker(dav_fs_
             fsctx->path2.cur_len += len;
 
             /* adjust URI length to incorporate subdir and a slash */
-            fsctx->uri_buf.cur_len += escaped_len + 1;
+            fsctx->uri_buf.cur_len += len + 1;
             fsctx->uri_buf.buf[fsctx->uri_buf.cur_len - 1] = '/';
             fsctx->uri_buf.buf[fsctx->uri_buf.cur_len] = '\0';
 
@@ -1611,8 +1598,8 @@ static dav_error * dav_fs_walker(dav_fs_
             */
             dav_buffer_place_mem(pool, &fsctx->path1,
                                  fsctx->locknull_buf.buf + offset, len + 1, 0);
-            dav_fs_append_uri(pool, &fsctx->uri_buf,
-                              fsctx->locknull_buf.buf + offset, 0);
+            dav_buffer_place_mem(pool, &fsctx->uri_buf,
+                                 fsctx->locknull_buf.buf + offset, len + 1, 0);
             if (fsctx->path2.buf != NULL) {
                 dav_buffer_place_mem(pool, &fsctx->path2,
                                      fsctx->locknull_buf.buf + offset,

Modified: httpd/httpd/branches/2.2.x/modules/dav/main/mod_dav.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/dav/main/mod_dav.c?rev=1611189&r1=1611188&r2=1611189&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/dav/main/mod_dav.c (original)
+++ httpd/httpd/branches/2.2.x/modules/dav/main/mod_dav.c Wed Jul 16 21:03:30 2014
@@ -400,9 +400,11 @@ static int dav_error_response_tag(reques
  */
 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(uri, '&') == NULL)
-        return uri;
+    if (ap_strchr_c(e_uri, '&') == NULL)
+        return e_uri;
 
     /* there was a '&', so more work is needed... sigh. */
 
@@ -410,7 +412,7 @@ static const char *dav_xml_escape_uri(ap
      * 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, uri, 0);
+    return apr_xml_quote_string(p, e_uri, 0);
 }
 
 

Modified: httpd/httpd/branches/2.2.x/modules/dav/main/mod_dav.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/dav/main/mod_dav.h?rev=1611189&r1=1611188&r2=1611189&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/dav/main/mod_dav.h (original)
+++ httpd/httpd/branches/2.2.x/modules/dav/main/mod_dav.h Wed Jul 16 21:03:30 2014
@@ -370,7 +370,9 @@ typedef struct dav_resource {
                          * REGULAR and WORKSPACE resources,
                          * and is always 1 for WORKING */
 
-    const char *uri;    /* the escaped URI for this resource */
+    const char *uri;    /* the URI for this resource;
+                         * currently has an ABI flaw where sometimes it is
+                         * assumed to be encoded and sometimes not */
 
     dav_resource_private *info;         /* the provider's private info */
 

Modified: httpd/httpd/branches/2.2.x/modules/dav/main/props.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/dav/main/props.c?rev=1611189&r1=1611188&r2=1611189&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/dav/main/props.c (original)
+++ httpd/httpd/branches/2.2.x/modules/dav/main/props.c Wed Jul 16 21:03:30 2014
@@ -321,10 +321,14 @@ static int dav_rw_liveprop(dav_propdb *p
 /* do a sub-request to fetch properties for the target resource's URI. */
 static void dav_do_prop_subreq(dav_propdb *propdb)
 {
+    /* need to escape the uri that's in the resource struct because during
+     * the property walker it's not encoded. */
+    const char *e_uri = ap_escape_uri(propdb->resource->pool,
+                                      propdb->resource->uri);
+
     /* perform a "GET" on the resource's URI (note that the resource
        may not correspond to the current request!). */
-    propdb->subreq = ap_sub_req_lookup_uri(propdb->resource->uri, propdb->r,
-                                           NULL);
+    propdb->subreq = ap_sub_req_lookup_uri(e_uri, propdb->r, NULL);
 }
 
 static dav_error * dav_insert_coreprop(dav_propdb *propdb,



Mime
View raw message