Hyrum K. Wright wrote:
> 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.
Posted as issue 2676.
-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 Thu Dec 14 15:57:27 2006