[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: bug report: ra_serf gets PROPFIND failure on certain non-ASCII paths

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Thu, 25 Feb 2010 13:06:37 +0000

Lieven Govaerts wrote:
> [[[
> ra_serf: Fix support for international characters in paths.
>
> Found by: cmpilato
>
> * subversion/libsvn_ra_serf/property.c
> (end_propfind): Replace call to svn_uri_canonicalize with a call to
> svn_ra_serf__uri_to_internal.
> * subversion/libsvn_ra_serf/merge.c
> (end_merge): Here too.
> * subversion/libsvn_ra_serf/ra_serf.h
> (svn_ra_serf__uri_to_internal): New function declaration.
> * subversion/libsvn_ra_serf/util.c
> (svn_ra_serf__uri_to_internal): New function definition.
> ]]]

> [[[
> Index: subversion/libsvn_ra_serf/merge.c
> ===================================================================
> --- subversion/libsvn_ra_serf/merge.c (revision 911289)
> +++ subversion/libsvn_ra_serf/merge.c (working copy)
> @@ -364,7 +364,7 @@
> info->prop_val = apr_pstrmemdup(info->pool, info->prop_val,
> info->prop_val_len);
> if (strcmp(info->prop_name, "href") == 0)
> - info->prop_val = svn_uri_canonicalize(info->prop_val,
> info->pool);
> + info->prop_val = svn_ra_serf__uri_to_internal(info->prop_val,
> info->pool);

As I mentioned on IRC, you canonicalize here when the URI is being put
in to the hash. But when it gets read out from the hash (about 70 lines
higher up) you also need to canonicalize the URL that it is compared
with. Here:

[line 299]
    href = apr_hash_get(info->props, "href", APR_HASH_KEY_STRING);
    if (! svn_uri_is_ancestor(ctx->merge_url, href))

... ctx->merge_url comes from session->repos_url, and that comes
from ... somewhere higher up.

You could canonicalize it right here, but really we need to fix the
global svn_uri_canonicalize() to do this kind of canonicalization
throughout Subversion, not just in ra_serf. Currently,
svn_uri_canonicalize() simply isn't producing a canonical URI.

- Julian

> /* Set our property. */
> apr_hash_set(info->props, info->prop_name, APR_HASH_KEY_STRING,
> Index: subversion/libsvn_ra_serf/util.c
> ===================================================================
> --- subversion/libsvn_ra_serf/util.c (revision 911289)
> +++ subversion/libsvn_ra_serf/util.c (working copy)
> @@ -1772,3 +1772,20 @@
>
> return SVN_NO_ERROR;
> }
> +
> +const char *
> +svn_ra_serf__uri_to_internal(const char *uri_in, apr_pool_t *pool)
> +{
> + const char *target;
> +
> + /* Convert to URI, unescaping all internatonal characters. */
> + target = svn_path_uri_decode(uri_in, pool);
> +
> + /* Now escape the international characters again. */
> + target = svn_path_uri_from_iri(target, pool);
> +
> + /* Strip any trailing '/' and collapse other redundant elements. */
> + target = svn_uri_canonicalize(target, pool);
> +
> + return target;
> +}
> Index: subversion/libsvn_ra_serf/property.c
> ===================================================================
> --- subversion/libsvn_ra_serf/property.c (revision 911289)
> +++ subversion/libsvn_ra_serf/property.c (working copy)
> @@ -29,6 +29,7 @@
> #include "svn_xml.h"
> #include "svn_props.h"
> #include "svn_dirent_uri.h"
> +#include "svn_path.h"
>
> #include "private/svn_dav_protocol.h"
> #include "svn_private_config.h"
> @@ -332,7 +333,7 @@
> {
> if (strcmp(ctx->depth, "1") == 0)
> {
> - ctx->current_path = svn_uri_canonicalize(info->val,
> ctx->pool);
> + ctx->current_path =
> svn_ra_serf__uri_to_internal(info->val, ctx->pool);
> }
> else
> {
> Index: subversion/libsvn_ra_serf/ra_serf.h
> ===================================================================
> --- subversion/libsvn_ra_serf/ra_serf.h (revision 911289)
> +++ subversion/libsvn_ra_serf/ra_serf.h (working copy)
> @@ -1521,6 +1521,13 @@
> svn_error_t *
> svn_ra_serf__error_on_status(int status_code, const char *path);
>
> +/**
> + * Handle an external uri so that it can be compared with other
> uri's.
> + * (canonicalize + re-encode international characters).
> + */
> +const char *
> +svn_ra_serf__uri_to_internal(const char *uri_in, apr_pool_t *pool);
> +
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
> ]]]
>
Received on 2010-02-25 14:07:15 CET

This is an archived mail posted to the Subversion Dev mailing list.