Troy,
Thanks. This is looking good, but I have some questions and comments.
Troy Curtis Jr wrote:
>>From my original patch (http://svn.haxx.se/dev/archive-2007-11/0708.shtml)
> through discussions about a better implementation
> (http://svn.haxx.se/dev/archive-2007-11/1113.shtml and
> http://svn.haxx.se/dev/archive-2007-12/0047.shtml) I have returned with a
> better relative url implementation for the svn client, at least so I believe.
>
> In this patch I implement the following logic for parsing the svn CLI
> client args:
> - If there is a relative url in the target list
> - Then use the repository root url from the other targets to resolve the
> relative url.
... and error if there are multiple different roots.
We need to be clear whether "args" and "targets" means all targets including
those specified with "--targets", or just direct command-line arguments. Which
should it be, and which have you implemented? (I haven't thought enough to be
sure which it should be.)
Do we need to allow backtracking with ".." before canonicalisation, in either of:
(a) inside a repository. Examples:
svn log ^/trunk/dir1/../dir2/
MYBRANCH=^/branches/br1 svn log $MYBRANCH/../br2
(b) in the URL-of-repository, to get from one repository to another, e.g.
svn log ^/../there's-another-repos-here/their-trunk
svn log ^/trunk/../../repos2/their-trunk
?
I come back to this question below. I can't remember whether we decided a long
time ago what should be allowed. Right now, off the top of my head, I think (a)
should be allowed pre-canonicalisation, if and only if that is consistent with
what we do for other URLs and paths. I don't think (b) should be allowed.
What do you think is best for the initial implementation of this feature?
> - If there are no other targets, use the root url of the current directory.
> - If the current directory is not a working copy, issue an error.
>
> This seems to get most of the use cases right. It does leave multi-repository
> urls out, but they are much less frequent and we couldn't come up with a
> simple (to understand) method of resolving them.
Tha above logic (command-line semantics), as well as the syntax, ought to be
documented somewhere in the code (and in the book, eventually). I've pointed
out below a place where it could go in the code.
> This patch contains what is likely a ridiculous amount of svn tests, basically
> one for every command of interest to relative urls. It might be too much
> because ultimately they all run the same function. However, I had them from
> my previous attempt (in which each svn subcommand had it's own implementation)
> and I figured they could be useful in making sure checks and translations
> don't get made before the call to svn_cl__args_to_target_array() that break
> relative url support.
There is a rather large amount of test code for what it achieves. Test code has
to be maintained too. Without studying and comparing it all, it looks like
duplicating several large test cases just to create a variant that uses a
relative URL. (Is that the case?) Rather than that, perhaps you can either
factor out the common code in those cases, or just don't add most of these
tests because we know there's no special need to test relative URLs for lots of
different commands. Just leave a couple of the small ones, or a set of tests
that specifically check attempts to use a relative URL in conjunction with 0, 1
and 2 absolute URLs to exercise all the variants of the designed behaviour.
> [[[
> Implement repository root relative url support for the svn command-line
> client.
I'd add just another sentence or two explaining what "repository-relative URL
support" means/is/does, since this is a new concept being introduced.
> Create some infrastructure:
>
> * subversion/libsvn_subr/path.c
> (svn_path_is_relative_url): New function to test whether a given string is a
> relative url.
> (svn_path_resolve_relative_url): New function to resolve a relative url
> given the repository root url.
>
> * subversion/include/svn_path.h
> (svn_path_is_relative_url): New function prototype.
> (svn_path_resolve_relative_url): New function prototype.
>
> * subversion/tests/libsvn_subr/path-test.c
> (test_is_relative_url): New test.
> (test_resolve_relative_url): New test.
> (test_funcs): Run new tests.
>
> * subversion/libsvn_subr/opt.c
> (svn_opt_args_to_target_array3): Allow a bare relative url for the root of a
> repository ('^/') by testing whether a given argument is a relative url
> before attempting to canonicalize it.
What exactly is this change, though?
You're making the function continue to process full URLs as it did, including
uri_from_iri, autoescape, well-formedness check, and canonicalise. But you're
making it bypass all but the first of these for repository-relative URLs, with
a comment saying "they will be processed later". This leads the user of this
function to ask, "How should I process these later?"
In fact, I can't see that you are processing these later.
Wouldn't it be better to have as much consistency with the processing of full
URLs as possible?
Why bypass the auto-escape? If it's only to avoid messing up the "^" character
then just auto-escape the rest of it.
I see that you require a trailing slash to remain on a bare "^/" argument, so
you bypass the canonicalisation, but in fact you could special-case this and
still do the rest of canonicalisation if that's what's needed.
And whatever it does, you need to describe in the function's doc string.
> * subversion/libsvn_client/externals.c
> (resolve_relative_external_url): Include a brief note mentioning the fact
> there is a partial reimplementation in libsvn_subr/path.c.
>
> Add relative url support to the svn command-line client:
>
> * subversion/svn/util.c
> (svn_cl__args_to_target_array_print_reserved): Replace this function with a
> more generically named svn_cl__args_to_target_array().
> (svn_cl__args_to_target_array): Resolve repository root relative urls in the
> arguments and known_targets.
>
> * subversion/svn/cl.h
> (svn_cl__args_to_target_array_print_reserved): Deleted old function
> prototype.
> (svn_cl__args_to_target_array): Add new function prototype.
>
> * subversion/svn/propdel-cmd.c,
> subversion/svn/merge-cmd.c,
> subversion/svn/checkout-cmd.c,
> subversion/svn/move-cmd.c,
> subversion/svn/mkdir-cmd.c,
> subversion/svn/cat-cmd.c,
> subversion/svn/revert-cmd.c,
> subversion/svn/diff-cmd.c,
> subversion/svn/copy-cmd.c,
> subversion/svn/mergeinfo-cmd.c,
> subversion/svn/list-cmd.c,
> subversion/svn/blame-cmd.c,
> subversion/svn/propget-cmd.c,
> subversion/svn/changelist-cmd.c,
> subversion/svn/log-cmd.c,
> subversion/svn/resolved-cmd.c,
> subversion/svn/cleanup-cmd.c,
> subversion/svn/commit-cmd.c,
> subversion/svn/add-cmd.c,
> subversion/svn/propset-cmd.c,
> subversion/svn/switch-cmd.c,
> subversion/svn/delete-cmd.c,
> subversion/svn/import-cmd.c,
> subversion/svn/proplist-cmd.c,
> subversion/svn/export-cmd.c,
> subversion/svn/status-cmd.c,
> subversion/svn/propedit-cmd.c,
> subversion/svn/lock-cmd.c,
> subversion/svn/info-cmd.c,
> subversion/svn/unlock-cmd.c
> (svn_cl__add, svn_cl__blame, svn_cl__cat, svn_cl__changelist,
> svn_cl__checkout, svn_cl__cleanup, svn_cl__commit, svn_cl__copy,
> svn_cl__delete, svn_cl__diff, svn_cl__export, svn_cl__import, svn_cl__info,
> svn_cl__list, svn_cl__lock, svn_cl__log, svn_cl__merge, svn_cl__mergeinfo,
> svn_cl__mkdir, svn_cl__move, svn_cl__propdel, svn_cl__propedit,
> svn_cl__propget, svn_cl__proplist, svn_cl__propset, svn_cl__resolved,
> svn_cl__revert, svn_cl__status, svn_cl__switch, svn_cl__unlock):
> Replaced the usage of svn_cl__args_to_target_array_print_reserved() with
> the more generically named svn_cl__args_to_target_array() which combines
> the original function's functionality with relative url resolution.
> (svn_cl__diff): Add access to the client context handle.
>
> * subversion/svn/update-cmd.c
> (svn_cl__update): Replace svn_opt_args_to_target_array3() with
> svn_cl__args_to_target_array().
>
> * subversion/tests/cmdline/cat_tests.py
> (cat_relative_url): New test.
> (test_list): Run it.
>
> * subversion/tests/cmdline/checkout_tests.py
> (co_with_relative_url): New test.
> (test_list): Run it.
>
> * subversion/tests/cmdline/copy_tests.py
> (copy_with_relative_urls): New test.
> (test_list): Run it.
>
> * subversion/tests/cmdline/diff_tests.py
> (diff_relative_url): New test.
> (test_list): Run it.
>
> * subversion/tests/cmdline/export_tests.py
> (export_relative_url): New test.
> (test_list): Run it.
>
> * subversion/tests/cmdline/import_tests.py
> (import_to_relative_url): New test.
> (test_list): Run it.
>
> * subversion/tests/cmdline/lock_tests.py
> (lock_file_with_relative_url): New test.
> (test_list): Run it.
>
> * subversion/tests/cmdline/log_tests.py
> (log_with_relative_urls): New test.
> (test_list): Run it.
>
> * subversion/tests/cmdline/merge_tests.py
> (merge_with_relative_urls): New test.
> (test_list): Run it.
>
> * subversion/tests/cmdline/prop_tests.py
> (url_props_relative_url): New test.
> (test_list): Run it.
>
> * subversion/tests/cmdline/switch_tests.py
> (switch_with_relative_url): New test.
> (test_list): Run it.
>
> * subversion/tests/cmdline/blame_tests.py
> (blame_relative_url): New test.
> (test_list): Run it.
> ]]]
>
> Troy
>
>
> ------------------------------------------------------------------------
>
> Index: subversion/include/svn_path.h
> ===================================================================
> --- subversion/include/svn_path.h (revision 29310)
> +++ subversion/include/svn_path.h (working copy)
> @@ -456,9 +456,28 @@
> /** Return TRUE iff @a path looks like a valid absolute URL. */
> svn_boolean_t svn_path_is_url(const char *path);
>
> +/**
> + * Return true iff @a url is a relative URL. Specifically that it starts
> + * with the characters "^/".
> + */
> +svn_boolean_t svn_path_is_relative_url(const char *url);
> +
> /** Return @c TRUE iff @a path is URI-safe, @c FALSE otherwise. */
> svn_boolean_t svn_path_is_uri_safe(const char *path);
>
> +/**
> + * Convert the possibly relative url in @a relative_url to an absolute
> + * url using @a repos_root_url and stick it in @a absolute_url. If the
> + * @a relative_url starts with the characters '^/', they are replaced by
> + * the repository root url, otherwise the @a relative_url is returned.
> + * Backpaths ("../") within the URL are also supported.
> + */
"Backpaths...supported": Why do we need this? There's no need to be able to
backtrack within a repository, as we're always starting at its root. The only
ability backtracking could bring is, starting with a URL for one repository, to
construct a URL that points to a different repository". The code currently
allows it but I don't believe we want to do that with this feature. (It feels
to me a bit yucky to mix URL-of-repository and path-inside-repository into one
continuum that can be traversed up and down arbitrarily.)
(Also, looking at the backtracking code in resolve_relative_external_url() that
you copied, it silently ignores ".." components, which I would have thought was
a bug.)
> +svn_error_t *
> +svn_path_resolve_relative_url(const char **absolute_url,
> + const char *relative_url,
> + const char *repos_root_url,
> + apr_pool_t *pool);
These two are not general path functions but command-line-client-specific
functions. Please make them private functions in the source file that needs
them. We can always make them public later if we need to.
> Index: subversion/libsvn_subr/opt.c
> ===================================================================
> --- subversion/libsvn_subr/opt.c (revision 29310)
> +++ subversion/libsvn_subr/opt.c (working copy)
> @@ -964,9 +964,19 @@
> utf8_target,
> peg_start - utf8_target);
>
> - /* URLs and wc-paths get treated differently. */
> - if (svn_path_is_url(utf8_target))
> + /* URLs, relative URLs, and wc-paths each get treated differently. */
> + if (svn_path_is_relative_url(utf8_target))
> {
> + /* Convert to URI. */
> + target = svn_path_uri_from_iri(utf8_target, pool);
> +
> + /* Do not try to canonicalize or auto escape relative urls, they
> + * will be processed later and we may very well need the
> + * trailing slash, i.e. '^/'.
> + */
> + }
> + else if (svn_path_is_url(utf8_target))
> + {
> /* No need to canonicalize a URL's case or path separators. */
>
> /* Convert to URI. */
> Index: subversion/libsvn_subr/path.c
> ===================================================================
> --- subversion/libsvn_subr/path.c (revision 29310)
> +++ subversion/libsvn_subr/path.c (working copy)
> @@ -23,6 +23,7 @@
>
> #include <apr_file_info.h>
> #include <apr_lib.h>
> +#include <apr_uri.h>
>
> #include "svn_string.h"
> #include "svn_path.h"
> @@ -871,8 +872,12 @@
> return skip_uri_scheme(path) ? TRUE : FALSE;
> }
>
> +svn_boolean_t
> +svn_path_is_relative_url(const char *url)
> +{
> + return(0 == strncmp("^/", url, 2) ? TRUE : FALSE);
> +}
>
> -
> /* Here is the BNF for path components in a URI. "pchar" is a
> character in a path component.
>
> @@ -1174,7 +1179,91 @@
> return SVN_NO_ERROR;
> }
>
> +svn_error_t *
> +svn_path_resolve_relative_url(const char **absolute_url,
> + const char *relative_url,
> + const char *repos_root_url,
> + apr_pool_t *pool)
> +{
> + apr_status_t status;
> + apr_array_header_t *base_components;
> + apr_array_header_t *relative_components;
> + apr_uri_t parsed_uri;
> + int i;
>
> + if (svn_path_is_url(relative_url))
> + {
> + *absolute_url = apr_pstrdup(pool, relative_url);
> +
> + return SVN_NO_ERROR;
> + }
> +
> + /*
> + * This is a partial re-implementation of the relative external support
> + * found in:
> + * libsvn_client/externals.c: resolve_relative_external_url()
> + *
> + * Updates to this code MAY be applicable to the other code
> + * (and vice versa).
> + */
> +
> + if (0 == strncmp("^/", relative_url, 2))
> + {
> + /* Is there something beyond '^/'? If not just use what we have
> + * (the repository root URL)
> + */
> + if (strlen(relative_url) == 2)
> + {
> + *absolute_url = apr_pstrdup(pool, repos_root_url);
> + }
> + else
> + {
> + status = apr_uri_parse(pool, repos_root_url, &parsed_uri);
> + if (status)
> + return svn_error_createf(SVN_ERR_BAD_URL, 0,
> + _("Illegal repository root URL '%s'"),
> + repos_root_url);
> +
> + base_components = svn_path_decompose(parsed_uri.path,
> + pool);
> +
> + relative_components = svn_path_decompose(relative_url + 2,
> + pool);
> +
> + for (i = 0; i < relative_components->nelts; ++i)
> + {
> + const char *component = APR_ARRAY_IDX(relative_components,
> + i,
> + const char *);
> + if (0 == strcmp("..", component))
> + {
> + /* Constructing the final absolute URL together with
> + * apr_uri_unparse() requires that the path be absolute,
> + * so only pop a component if the component being popped
> + * is not the component for the root directory. */
> + if (base_components->nelts > 1)
> + apr_array_pop(base_components);
> + }
> + else
> + APR_ARRAY_PUSH(base_components, const char *) = component;
> + }
> +
> + parsed_uri.path = (char *)svn_path_compose(base_components,
> + pool);
> + parsed_uri.query = NULL;
> + parsed_uri.fragment = NULL;
> +
> + *absolute_url = apr_uri_unparse(pool, &parsed_uri, 0);
> + }
> + }
> + else
> + return svn_error_createf(SVN_ERR_BAD_URL, 0,
> + _("Improper relative URL '%s'"),
> + relative_url);
> +
> + return SVN_NO_ERROR;
> +}
> +
> svn_error_t *
> svn_path_split_if_file(const char *path,
> const char **pdirectory,
> Index: subversion/libsvn_client/externals.c
> ===================================================================
> --- subversion/libsvn_client/externals.c (revision 29310)
> +++ subversion/libsvn_client/externals.c (working copy)
> @@ -368,7 +368,12 @@
> repository root. The backpaths may only remove path elements,
> not the hostname. This allows an external to refer to another
> repository in the same server relative to the location of this
> - repository, say using SVNParentPath. */
> + repository, say using SVNParentPath.
> +
> + Note: There is a partial re-implementation of this code in
> + libsvn_subr/path.c: svn_path_resolve_relative_url().
> + Updates to this code MAY be applicable to the other code
> + (and vice versa). */
> if ((0 == strncmp("../", uncanonicalized_url, 3)) ||
> (0 == strncmp("^/", uncanonicalized_url, 2)))
> {
> Index: subversion/tests/libsvn_subr/path-test.c
> ===================================================================
> --- subversion/tests/libsvn_subr/path-test.c (revision 29310)
> +++ subversion/tests/libsvn_subr/path-test.c (working copy)
> @@ -1238,6 +1237,111 @@
> return SVN_NO_ERROR;
> }
>
> +static svn_error_t *
> +test_is_relative_url(const char **msg,
> + svn_boolean_t msg_only,
> + svn_test_opts_t *opts,
> + apr_pool_t *pool)
> +{
> + apr_size_t i;
> +
> + /* Paths to test and their expected results. */
> + struct {
> + const char *path;
> + svn_boolean_t result;
> + } tests[] = {
> + { "", FALSE },
> + { "/blah/blah", FALSE },
> + { "//blah/blah", FALSE },
> + { "://blah/blah", FALSE },
> + { "http://svn.collab.net/repos/svn", FALSE },
> + { "scheme/with", FALSE },
> + { "^/trunk", TRUE },
> + { "^/branches/../trunk", TRUE },
> + { "^trunk", FALSE },
> + { "./^/trunk", FALSE },
> + { "scheme/with:", FALSE },
> + { "scheme/with:/", FALSE },
> + { "file:///path/to/repository", FALSE },
> + { "file://", FALSE },
> + { "file:/", FALSE },
> + { "file:", FALSE },
> + { "file", FALSE },
> + };
> +
> + *msg = "test svn_path_is_relative_url";
> +
> + if (msg_only)
> + return SVN_NO_ERROR;
> +
> + for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++)
> + {
> + svn_boolean_t retval;
> +
> + retval = svn_path_is_relative_url(tests[i].path);
> + if (tests[i].result != retval)
> + return svn_error_createf
> + (SVN_ERR_TEST_FAILED, NULL,
> + "svn_path_is_relative_url (%s) returned %s instead of %s",
> + tests[i].path, retval ? "TRUE" : "FALSE",
> + tests[i].result ? "TRUE" : "FALSE");
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> +static svn_error_t *
> +test_resolve_relative_url(const char **msg,
> + svn_boolean_t msg_only,
> + svn_test_opts_t *opts,
> + apr_pool_t *pool)
> +{
> + apr_size_t i;
> +
> + /* Relative urls to test with their root urls and expected results */
> + struct {
> + const char *rel_url;
> + const char *root_url;
> + const char *abs_url;
> + } tests[] = {
> + { "file:///repos/path/trunk", "file:///repos/path",
> + "file:///repos/path/trunk" },
> + { "http://svn.collab.net/repos/svn/trunk", "http://svn.collab.net/repos/svn",
> + "http://svn.collab.net/repos/svn/trunk" },
> + { "^/trunk", "file:///repos/path",
> + "file:///repos/path/trunk" },
> + { "^/trunk", "http://svn.collab.net/repos/svn",
> + "http://svn.collab.net/repos/svn/trunk" },
> + { "^/branches/../trunk", "file:///repos/path",
> + "file:///repos/path/trunk" },
> + { "^/branches/../trunk", "http://svn.collab.net/repos/svn",
> + "http://svn.collab.net/repos/svn/trunk" },
> + };
> +
> + *msg = "test svn_path_resolve_relative_url";
> +
> + if (msg_only)
> + return SVN_NO_ERROR;
> +
> + for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++)
> + {
> + const char *rel_url = tests[i].rel_url;
> + const char *root_url = tests[i].root_url;
> + const char *abs_url;
> +
> + svn_path_resolve_relative_url(&abs_url, rel_url, root_url, pool);
> + if ((strcmp(tests[i].abs_url, abs_url)))
> + return svn_error_createf
> + (SVN_ERR_TEST_FAILED, NULL,
> + "svn_path_resolve_relative_url (%s, %s) returned '%s' "
> + "instead of '%s'",
> + rel_url, root_url, abs_url,
> + tests[i].abs_url);
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> /* local define to support XFail-ing tests on Windows/Cygwin only */
> #if defined(WIN32) || defined(__CYGWIN__)
> #define WINDOWS_OR_CYGWIN TRUE
> @@ -1273,5 +1377,7 @@
> SVN_TEST_PASS(test_get_longest_ancestor),
> SVN_TEST_PASS(test_splitext),
> SVN_TEST_PASS(test_compose),
> + SVN_TEST_PASS(test_is_relative_url),
> + SVN_TEST_PASS(test_resolve_relative_url),
> SVN_TEST_NULL
> };
> Index: subversion/svn/cl.h
> ===================================================================
> --- subversion/svn/cl.h (revision 29310)
> +++ subversion/svn/cl.h (working copy)
> @@ -556,6 +556,20 @@
> const char *svn_cl__node_kind_str(svn_node_kind_t kind);
>
>
> +/* Command-line client wrapper function for the svn_opt_args_to_target_array3()
> + * function which performs a few extra command-line specific operations:
> + * - Prints a warning if a reserved filename is specified in the arguments.
> + * - Resolves any relative urls present in the arguments or known
> + * targets based on the other arguments, or failing that on the current
> + * directory.
More precision, and mention of the function arguments by name, would be good.
Here is where a concise description of the syntax and semantics could usefully
go, that I mentioned at the beginning of this email.
> + */
> +svn_error_t *
> +svn_cl__args_to_target_array(apr_array_header_t **targets_ret,
> + apr_getopt_t *os,
> + apr_array_header_t *known_targets,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
> +
> /* If PROPNAME is one of the svn: properties with a boolean value, and
> * PROPVAL looks like an attempt to turn the property off (i.e., it's
> * "off", "no", "false", or ""), then print a warning to the user that
> @@ -576,12 +590,6 @@
> svn_client_ctx_t *ctx,
> apr_pool_t *pool);
>
> -svn_error_t *
> -svn_cl__args_to_target_array_print_reserved(apr_array_header_t **targets_p,
> - apr_getopt_t *os,
> - apr_array_header_t *known_targets,
> - apr_pool_t *pool);
> -
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
> Index: subversion/svn/util.c
> ===================================================================
> --- subversion/svn/util.c (revision 29310)
> +++ subversion/svn/util.c (working copy)
> @@ -1009,15 +1009,20 @@
> }
> }
>
> -
> svn_error_t *
> -svn_cl__args_to_target_array_print_reserved(apr_array_header_t **targets,
> - apr_getopt_t *os,
> - apr_array_header_t *known_targets,
> - apr_pool_t *pool)
> +svn_cl__args_to_target_array(apr_array_header_t **targets_ret,
> + apr_getopt_t *os,
> + apr_array_header_t *known_targets,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> {
> - svn_error_t *error = svn_opt_args_to_target_array3(targets, os,
> - known_targets, pool);
> + int i;
> + const char *root_url = NULL;
> + apr_array_header_t *targets;
> + svn_error_t *error;
> +
> + error = svn_opt_args_to_target_array3(&targets, os, known_targets, pool);
> +
> if (error)
> {
> if (error->apr_err == SVN_ERR_RESERVED_FILENAME_SPECIFIED)
> @@ -1028,10 +1033,96 @@
> else
> return error;
> }
> +
> + /*
> + * First determine whether to even bother with root urls,
> + * at least one relative url for it to matter.
> + */
> + for (i = 0; i < targets->nelts; i++)
> + {
> + const char *target = APR_ARRAY_IDX(targets, i, const char *);
> +
> + if (svn_path_is_relative_url(target))
> + break;
> + }
> +
> + /* Was a relative url found? */
> + if (i >= targets->nelts)
> + {
> + *targets_ret = targets;
> + return SVN_NO_ERROR;
> + }
> +
> + /*
> + * Now determine the common root url to use.
> + */
> + for (i = 0; i < targets->nelts; i++)
> + {
> + const char *target = APR_ARRAY_IDX(targets, i, const char *);
> + const char *tmp_root_url;
> +
> + if (svn_path_is_relative_url(target))
> + continue;
> +
> + if ((error = svn_client_root_url_from_path(&tmp_root_url, target,
> + ctx, pool)))
> + {
> + if ( (error->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
> + || (error->apr_err == SVN_ERR_WC_NOT_DIRECTORY))
> + {
> + svn_error_clear(error);
> + continue;
> + }
> + else
> + return error;
> + }
> + else if ( (root_url != NULL)
> + && (strcmp(root_url, tmp_root_url) != 0))
> + return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
> + _("All non-relative paths must have the same root url.\n"));
> + else
> + root_url = tmp_root_url;
> + }
> +
> + /*
> + * Use the current directory's root url if one wasn't found using the
> + * arguments.
> + */
> + if (root_url == NULL)
> + {
> + if ((error = svn_client_root_url_from_path(&root_url,
> + svn_path_canonicalize(".", pool),
> + ctx, pool)))
> + return error;
> + }
> +
> + /*
> + * Finally, resolve any relative urls found in the targets array.
> + */
> +
> + *targets_ret = apr_array_make(pool, targets->nelts, sizeof(const char *));
> +
> + for (i = 0; i < targets->nelts; i++)
> + {
> + const char *target = APR_ARRAY_IDX(targets, i, const char *);
> +
> + if (svn_path_is_relative_url(target))
> + {
> + const char *abs_target;
> +
> + SVN_ERR(svn_path_resolve_relative_url(&abs_target, target,
> + root_url, pool));
> + APR_ARRAY_PUSH(*targets_ret, const char *) = abs_target;
> + }
> + else
> + {
> + APR_ARRAY_PUSH(*targets_ret, const char *) = target;
> + }
> + }
> +
> return SVN_NO_ERROR;
> }
Is that the function in which we might expect canonicalisation and
auto-escaping to be done?
Regards,
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-22 13:02:19 CET