Martin Hauner wrote:
> Hi,
>
> here is a small patch for a new function svn_path_from_user_input that
> helps any svn client to pass a well formed and escaped path/url to the
> api.
>
> There is a small change in the handling of the administrative dir. In
> the old code the administrative dir was skipped. In the new code it
> reports an error.
> I don't think it is worth to check the error of the new function to
> mimic the old behavior.
Ping...
Has a committer had a chance to look at this patch? If nothing happens
with it in a few days, I'll file an issue.
Thanks,
-Hyrum
> [[[
> Moved path and url preparation code from svn_opt_args_to_target_array2
> to its own function so it is available to any subversion api client.
>
> * subversion/include/svn_error_codes.h
> (SVN_ERR_WC_BAD_PATH_ADM_DIR): new error.
>
> * subversion/include/svn_path.h
> subversion/libsvn_subr/path.c
> (svn_path_from_user_input): new function, extracted from
> svn_opt_args_to_target_array2.
>
> * subversion/libsvn_subr/opt.c
> (svn_opt_args_to_target_array2): use the new function.
> ]]]
>
>
> Max Bowsher wrote:
>> Martin Hauner wrote:
>>> Hi,
>>>
>>> I received a bug report for subcommander that it doesn't properly
>>> handle urls with spaces.
>>>
>>> Grepping through the subversion code the magic trick (which is well
>>> hidden for someone who is working with the svn_client api ;-) seems
>>> to be in the loop of svn_opt_args_to_target_array2.
>>>
>>> The code is basically:
>>>
>>> if( svn_path_is_url(pathOrUrl) )
>>> {
>>> // do anything necessary to create a proper url
>>> path = svn_path_uri_from_iri( );
>>> path = svn_path_uri_autoescape( );
>>> path = svn_path_canonicalize( );
>>>
>>> ... some additional checks if it is a valid url
>>> ... error if not
>>> }
>>> else
>>> {
>>> path = svn_path_canonicalize( );
>>> ...
>>> }
>>>
>>> I also found a nearly 1:1 copy of this code in the java bindings and a
>>> stripped down version in mucc.
>>>
>>> It looks like that i have to prepare any path or url i pass to the
>>> subversion api (talking about svn_client here..) with the above code.
>>>
>>> If that is the case, I would like to move the above code into its own
>>> function (named svn_path_prepare?) so it is available to any subversion
>>> client without copy and paste.
>>>
>>> What do you think?
>>
>> svn_path_from_user_input() perhaps? Yes, this is clearly a good idea to
>> abstract the standard logic to a reusable function.
>>
>> Max.
>>
>
>
>
> ------------------------------------------------------------------------
>
> Index: D:/dev/src/subversion/svn-trunk/subversion/include/svn_error_codes.h
> ===================================================================
> --- D:/dev/src/subversion/svn-trunk/subversion/include/svn_error_codes.h (Revision 22335)
> +++ D:/dev/src/subversion/svn-trunk/subversion/include/svn_error_codes.h (Arbeitskopie)
> @@ -391,6 +391,11 @@
> SVN_ERR_WC_CATEGORY_START + 25,
> "Invalid switch")
>
> + /** @since New in 1.5. */
> + SVN_ERRDEF(SVN_ERR_WC_BAD_PATH_ADM_DIR,
> + SVN_ERR_WC_CATEGORY_START + 26,
> + "Operation is not allowed on administrative dir")
> +
> /* fs errors */
>
> SVN_ERRDEF(SVN_ERR_FS_GENERAL,
> Index: D:/dev/src/subversion/svn-trunk/subversion/include/svn_path.h
> ===================================================================
> --- D:/dev/src/subversion/svn-trunk/subversion/include/svn_path.h (Revision 22335)
> +++ D:/dev/src/subversion/svn-trunk/subversion/include/svn_path.h (Arbeitskopie)
> @@ -1,7 +1,7 @@
> /**
> * @copyright
> * ====================================================================
> - * Copyright (c) 2000-2004 CollabNet. All rights reserved.
> + * Copyright (c) 2000-2006 CollabNet. All rights reserved.
> *
> * This software is licensed as described in the file COPYING, which
> * you should have received as part of this distribution. The terms
> @@ -46,6 +46,27 @@
>
>
>
> +/**
> + * Prepare the given UTF-8 encoded @a path_or_url_utf8 so it can be passed
> + * to a subversion api call. You are expected to call this before entering
> + * the svn api universe if @a path_or_url_utf8 is based on user input (gui
> + * or console).
> + *
> + * A working copy path is canonicalized. A repository URL is canonicalized
> + * and converted to a well formed, autoescaped URL.
> + *
> + * An error is returned if the transformation failed or if the result is not
> + * safe to use (eg. an url is not uri safe or it contains a backpath element
> + * '..'). @a path_svn is undefined if an error is returned.
> + *
> + * @since New in 1.5.
> + */
> +svn_error_t *
> +svn_path_from_user_input(const char **path_svn,
> + const char *path_or_url_utf8,
> + apr_pool_t *pool);
> +
> +
> /** Convert @a path from the local style to the canonical internal style. */
> const char *svn_path_internal_style(const char *path, apr_pool_t *pool);
>
> Index: D:/dev/src/subversion/svn-trunk/subversion/libsvn_subr/opt.c
> ===================================================================
> --- D:/dev/src/subversion/svn-trunk/subversion/libsvn_subr/opt.c (Revision 22335)
> +++ D:/dev/src/subversion/svn-trunk/subversion/libsvn_subr/opt.c (Arbeitskopie)
> @@ -819,79 +819,10 @@
> for (i = 0; i < input_targets->nelts; i++)
> {
> const char *utf8_target = APR_ARRAY_IDX(input_targets, i, const char *);
> - const char *target; /* after all processing is finished */
> + const char *target;
>
> - /* URLs and wc-paths get treated differently. */
> - if (svn_path_is_url(utf8_target))
> - {
> - /* No need to canonicalize a URL's case or path separators. */
> + SVN_ERR(svn_path_from_user_input(&target, utf8_target, pool));
>
> - /* Convert to URI. */
> - target = svn_path_uri_from_iri(utf8_target, pool);
> - /* Auto-escape some ASCII characters. */
> - target = svn_path_uri_autoescape(target, pool);
> -
> - /* The above doesn't guarantee a valid URI. */
> - if (! svn_path_is_uri_safe(target))
> - return svn_error_createf(SVN_ERR_BAD_URL, 0,
> - _("URL '%s' is not properly URI-encoded"),
> - utf8_target);
> -
> - /* Verify that no backpaths are present in the URL. */
> - if (svn_path_is_backpath_present(target))
> - return svn_error_createf(SVN_ERR_BAD_URL, 0,
> - _("URL '%s' contains a '..' element"),
> - utf8_target);
> -
> - /* strip any trailing '/' */
> - target = svn_path_canonicalize(target, pool);
> - }
> - else /* not a url, so treat as a path */
> - {
> - const char *apr_target;
> - const char *base_name;
> - char *truenamed_target; /* APR-encoded */
> - apr_status_t apr_err;
> -
> - /* canonicalize case, and change all separators to '/'. */
> - SVN_ERR(svn_path_cstring_from_utf8(&apr_target, utf8_target,
> - pool));
> - apr_err = apr_filepath_merge(&truenamed_target, "", apr_target,
> - APR_FILEPATH_TRUENAME, pool);
> -
> - if (!apr_err)
> - /* We have a canonicalized APR-encoded target now. */
> - apr_target = truenamed_target;
> - else if (APR_STATUS_IS_ENOENT(apr_err))
> - /* It's okay for the file to not exist, that just means we
> - have to accept the case given to the client. We'll use
> - the original APR-encoded target. */
> - ;
> - else
> - return svn_error_createf(apr_err, NULL,
> - _("Error resolving case of '%s'"),
> - svn_path_local_style(utf8_target,
> - pool));
> -
> - /* convert back to UTF-8. */
> - SVN_ERR(svn_path_cstring_to_utf8(&target, apr_target, pool));
> - target = svn_path_canonicalize(target, pool);
> -
> - /* If the target has the same name as a Subversion
> - working copy administrative dir, skip it. */
> - base_name = svn_path_basename(target, pool);
> - /* FIXME:
> - The canonical list of administrative directory names is
> - maintained in libsvn_wc/adm_files.c:svn_wc_set_adm_dir().
> - That list can't be used here, because that use would
> - create a circular dependency between libsvn_wc and
> - libsvn_subr. Make sure changes to the lists are always
> - synchronized! */
> - if (0 == strcmp(base_name, ".svn")
> - || 0 == strcmp(base_name, "_svn"))
> - continue;
> - }
> -
> (*((const char **) apr_array_push(output_targets))) = target;
> }
>
> Index: D:/dev/src/subversion/svn-trunk/subversion/libsvn_subr/path.c
> ===================================================================
> --- D:/dev/src/subversion/svn-trunk/subversion/libsvn_subr/path.c (Revision 22335)
> +++ D:/dev/src/subversion/svn-trunk/subversion/libsvn_subr/path.c (Arbeitskopie)
> @@ -2,7 +2,7 @@
> * paths.c: a path manipulation library using svn_stringbuf_t
> *
> * ====================================================================
> - * Copyright (c) 2000-2004 CollabNet. All rights reserved.
> + * Copyright (c) 2000-2006 CollabNet. All rights reserved.
> *
> * This software is licensed as described in the file COPYING, which
> * you should have received as part of this distribution. The terms
> @@ -1364,3 +1364,82 @@
>
> return SVN_NO_ERROR;
> }
> +
> +svn_error_t *
> +svn_path_from_user_input(const char **path_svn,
> + const char *path_or_url_utf8,
> + apr_pool_t *pool)
> +{
> + const char *target; /* after all processing is finished */
> +
> + /* URLs and wc-paths get treated differently. */
> + if (svn_path_is_url(path_or_url_utf8))
> + {
> + /* No need to canonicalize a URL's case or path separators. */
> +
> + /* Convert to URI. */
> + target = svn_path_uri_from_iri(path_or_url_utf8, pool);
> + /* Auto-escape some ASCII characters. */
> + target = svn_path_uri_autoescape(target, pool);
> +
> + /* The above doesn't guarantee a valid URI. */
> + if (! svn_path_is_uri_safe(target))
> + return svn_error_createf(SVN_ERR_BAD_URL, 0,
> + _("URL '%s' is not properly URI-encoded"), path_or_url_utf8);
> +
> + /* Verify that no backpaths are present in the URL. */
> + if (svn_path_is_backpath_present(target))
> + return svn_error_createf(SVN_ERR_BAD_URL, 0,
> + _("URL '%s' contains a '..' element"), path_or_url_utf8);
> +
> + /* strip any trailing '/' */
> + target = svn_path_canonicalize(target, pool);
> + }
> + else /* not a url, so treat as a path */
> + {
> + const char *apr_target;
> + char *truenamed_target; /* APR-encoded */
> + apr_status_t apr_err;
> +
> + /* canonicalize case, and change all separators to '/'. */
> + SVN_ERR(svn_path_cstring_from_utf8(&apr_target, path_or_url_utf8,
> + pool));
> + apr_err = apr_filepath_merge(&truenamed_target, "", path_or_url_utf8,
> + APR_FILEPATH_TRUENAME, pool);
> +
> + if (!apr_err)
> + /* We have a canonicalized APR-encoded target now. */
> + apr_target = truenamed_target;
> + else if (APR_STATUS_IS_ENOENT(apr_err))
> + /* It's okay for the file to not exist, that just means we
> + have to accept the case given to the client. We'll use
> + the original APR-encoded target. */
> + ;
> + else
> + return svn_error_createf(apr_err, NULL,
> + _("Error resolving case of '%s'"),
> + svn_path_local_style(path_or_url_utf8, pool));
> +
> + /* convert back to UTF-8. */
> + SVN_ERR(svn_path_cstring_to_utf8(&target, apr_target, pool));
> + target = svn_path_canonicalize(target, pool);
> +
> + /* If the target has the same name as a Subversion
> + working copy administrative dir, skip it. */
> + base_name = svn_path_basename(target, pool);
> + /* FIXME:
> + The canonical list of administrative directory names is
> + maintained in libsvn_wc/adm_files.c:svn_wc_set_adm_dir().
> + That list can't be used here, because that use would
> + create a circular dependency between libsvn_wc and
> + libsvn_subr. Make sure changes to the lists are always
> + synchronized! */
> + if (0 == strcmp(base_name, ".svn")
> + || 0 == strcmp(base_name, "_svn"))
> + return svn_error_createf(SVN_ERR_WC_BAD_PATH_ADM_DIR, 0,
> + _("Path '%s' is the administrative directory"), base_name);
> + }
> +
> + *path_svn = target;
> + return SVN_NO_ERROR;
> +}
Received on Mon Dec 4 20:37:57 2006