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.
[[[
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.
>
--
Martin
Subcommander 1.2.1 - http://subcommander.tigris.org
a cross platform Win32/Unix/MacOSX subversion GUI client & diff/merge tool.
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;
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Nov 20 22:09:52 2006