You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by tr...@apache.org on 2014/07/12 19:53:03 UTC
svn commit: r1610013 - in /httpd/httpd/branches/2.4.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
Author: trawick
Date: Sat Jul 12 17:53:03 2014
New Revision: 1610013
URL: http://svn.apache.org/r1610013
Log:
Merge r1602338 from trunk:
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
Reviewed by: ylavic, covener
Modified:
httpd/httpd/branches/2.4.x/ (props changed)
httpd/httpd/branches/2.4.x/CHANGES
httpd/httpd/branches/2.4.x/STATUS
httpd/httpd/branches/2.4.x/modules/dav/fs/repos.c
httpd/httpd/branches/2.4.x/modules/dav/main/mod_dav.c
httpd/httpd/branches/2.4.x/modules/dav/main/mod_dav.h
httpd/httpd/branches/2.4.x/modules/dav/main/props.c
Propchange: httpd/httpd/branches/2.4.x/
------------------------------------------------------------------------------
Merged /httpd/httpd/trunk:r1602338
Modified: httpd/httpd/branches/2.4.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1610013&r1=1610012&r2=1610013&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Sat Jul 12 17:53:03 2014
@@ -2,6 +2,9 @@
Changes with Apache 2.4.10
+ *) mod_dav: Fix improper encoding in PROPFIND responses. PR 56480.
+ [Ben Reser]
+
*) WinNT MPM: Improve error handling for termination events in child.
[Jeff Trawick]
Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1610013&r1=1610012&r2=1610013&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Sat Jul 12 17:53:03 2014
@@ -100,16 +100,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.4.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, ylavic, covener
PATCHES PROPOSED TO BACKPORT FROM TRUNK:
Modified: httpd/httpd/branches/2.4.x/modules/dav/fs/repos.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/dav/fs/repos.c?rev=1610013&r1=1610012&r2=1610013&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/dav/fs/repos.c (original)
+++ httpd/httpd/branches/2.4.x/modules/dav/fs/repos.c Sat Jul 12 17:53:03 2014
@@ -717,13 +717,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 != APR_NOFILE) {
@@ -1482,18 +1482,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. */
@@ -1549,7 +1537,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;
len = strlen(dirent.name);
@@ -1592,7 +1579,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) {
@@ -1625,7 +1612,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';
@@ -1691,8 +1678,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.4.x/modules/dav/main/mod_dav.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/dav/main/mod_dav.c?rev=1610013&r1=1610012&r2=1610013&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/dav/main/mod_dav.c (original)
+++ httpd/httpd/branches/2.4.x/modules/dav/main/mod_dav.c Sat Jul 12 17:53:03 2014
@@ -396,9 +396,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. */
@@ -406,7 +408,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.4.x/modules/dav/main/mod_dav.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/dav/main/mod_dav.h?rev=1610013&r1=1610012&r2=1610013&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/dav/main/mod_dav.h (original)
+++ httpd/httpd/branches/2.4.x/modules/dav/main/mod_dav.h Sat Jul 12 17:53:03 2014
@@ -386,7 +386,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.4.x/modules/dav/main/props.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/dav/main/props.c?rev=1610013&r1=1610012&r2=1610013&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/dav/main/props.c (original)
+++ httpd/httpd/branches/2.4.x/modules/dav/main/props.c Sat Jul 12 17:53:03 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,