You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2010/08/02 14:49:46 UTC
svn commit: r981489 - in /subversion/trunk/subversion:
include/svn_dirent_uri.h libsvn_client/patch.c libsvn_subr/dirent_uri.c
libsvn_wc/update_editor.c tests/libsvn_subr/dirent_uri-test.c
Author: rhuijben
Date: Mon Aug 2 12:49:45 2010
New Revision: 981489
URL: http://svn.apache.org/viewvc?rev=981489&view=rev
Log:
Make svn_dirent_is_under_root() more consistent with our normal apis: ensure
that the resulting string is canonical. Also document that it doesn't return
an abspath even though it's argument name indicates otherwise Also allow
returning errors when there is something else going on then the path not
being under the root.
* subversion/include/svn_dirent_uri.h
(svn_dirent_is_under_root): Update prototype to return an error instead
of a boolean. Update documentation. Remove TODO marker.
* subversion/libsvn_client/patch.c
(resolve_target_path): Update caller. Remove canonicalization but make
the path absolute.
* subversion/libsvn_subr/dirent_uri.c
(svn_dirent_is_under_root): Return an error for every error except
APR_EABOVEROOT. Canonicalize the path.
* subversion/libsvn_wc/update_editor.c
(check_path_under_root): Update caller.
* subversion/tests/libsvn_subr/dirent_uri-test.c
(test_dirent_is_under_root): New function.
(test_funcs): Add test_dirent_is_under_root.
Found by: dannas
Modified:
subversion/trunk/subversion/include/svn_dirent_uri.h
subversion/trunk/subversion/libsvn_client/patch.c
subversion/trunk/subversion/libsvn_subr/dirent_uri.c
subversion/trunk/subversion/libsvn_wc/update_editor.c
subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
Modified: subversion/trunk/subversion/include/svn_dirent_uri.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_dirent_uri.h?rev=981489&r1=981488&r2=981489&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_dirent_uri.h (original)
+++ subversion/trunk/subversion/include/svn_dirent_uri.h Mon Aug 2 12:49:45 2010
@@ -766,23 +766,22 @@ svn_uri_condense_targets(const char **pc
apr_pool_t *scratch_pool);
/** Check that when @a path is joined to @a base_path, the resulting path
- * is still under BASE_PATH in the local filesystem. If not, return @c FALSE.
- * If @c TRUE is returned, @a *full_path will be set to the absolute path
- * of @a path, allocated in @a pool.
+ * is still under BASE_PATH in the local filesystem. If not, set @a under_root
+ * to @c FALSE. If @a under_root is @c TRUE is returned, and @a result_path is
+ * not @c NULL, then @a *result_path will be set to the absolute path of @a
+ * path, allocated in @a result_pool.
*
* Note: Use of this function is strongly encouraged. Do not roll your own.
* (http://cve.mitre.org/cgi-bin/cvename.cgi?name=2007-3846)
*
- * ### @todo If path is "", then full_path will have a trailing '/' and thus
- * not be canonicalized.
- *
* @since New in 1.7.
*/
-svn_boolean_t
-svn_dirent_is_under_root(char **full_path,
+svn_error_t *
+svn_dirent_is_under_root(svn_boolean_t *under_root,
+ const char **result_path,
const char *base_path,
const char *path,
- apr_pool_t *pool);
+ apr_pool_t *result_pool);
/** Set @a *dirent to the path corresponding to the file:// URL @a url, using
* the platform-specific file:// rules.
Modified: subversion/trunk/subversion/libsvn_client/patch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=981489&r1=981488&r2=981489&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/patch.c (original)
+++ subversion/trunk/subversion/libsvn_client/patch.c Mon Aug 2 12:49:45 2010
@@ -340,6 +340,7 @@ resolve_target_path(patch_target_t *targ
char *full_path;
svn_wc_status3_t *status;
svn_error_t *err;
+ svn_boolean_t under_root;
target->canon_path_from_patchfile = svn_dirent_internal_style(
path_from_patchfile, result_pool);
@@ -384,8 +385,11 @@ resolve_target_path(patch_target_t *targ
/* Make sure the path is secure to use. We want the target to be inside
* of the working copy and not be fooled by symlinks it might contain. */
- if (! svn_dirent_is_under_root(&full_path, local_abspath,
- target->local_relpath, result_pool))
+ SVN_ERR(svn_dirent_is_under_root(&under_root,
+ &full_path, local_abspath,
+ target->local_relpath, result_pool));
+
+ if (! under_root)
{
/* The target path is outside of the working copy. Skip it. */
target->skipped = TRUE;
@@ -393,13 +397,8 @@ resolve_target_path(patch_target_t *targ
return SVN_NO_ERROR;
}
- target->local_abspath = full_path;
-
- /* ### Joining a path with "" in svn_dirent_is_under_root() creates a
- * ### non-canonicalized path. Until that behaviour is fixed, we do an
- * ### extra canonicalization step. */
- target->local_abspath = svn_dirent_canonicalize( target->local_abspath,
- result_pool);
+ SVN_ERR(svn_dirent_get_absolute(&target->local_abspath, full_path,
+ result_pool));
/* Skip things we should not be messing with. */
err = svn_wc_status3(&status, wc_ctx, target->local_abspath,
Modified: subversion/trunk/subversion/libsvn_subr/dirent_uri.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/dirent_uri.c?rev=981489&r1=981488&r2=981489&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/dirent_uri.c (original)
+++ subversion/trunk/subversion/libsvn_subr/dirent_uri.c Mon Aug 2 12:49:45 2010
@@ -2254,21 +2254,42 @@ svn_uri_condense_targets(const char **pc
return SVN_NO_ERROR;
}
-svn_boolean_t
-svn_dirent_is_under_root(char **full_path,
+svn_error_t *
+svn_dirent_is_under_root(svn_boolean_t *under_root,
+ const char **abspath,
const char *base_path,
const char *path,
apr_pool_t *pool)
{
apr_status_t status;
+ char *full_path;
+
+ *under_root = FALSE;
+ if (abspath)
+ *abspath = NULL;
+
+ status = apr_filepath_merge(&full_path,
+ base_path,
+ path,
+ APR_FILEPATH_NOTABOVEROOT
+ | APR_FILEPATH_SECUREROOTTEST,
+ pool);
- status = apr_filepath_merge(
- full_path, base_path, path,
- APR_FILEPATH_NOTABOVEROOT | APR_FILEPATH_SECUREROOTTEST,
- pool);
+ if (status == APR_SUCCESS)
+ {
+ if (abspath)
+ *abspath = svn_dirent_canonicalize(full_path, pool);
+ *under_root = TRUE;
+ return SVN_NO_ERROR;
+ }
+ else if (status == APR_EABOVEROOT)
+ {
+ *under_root = FALSE;
+ return SVN_NO_ERROR;
+ }
- return status == APR_SUCCESS ? TRUE : FALSE;
+ return svn_error_wrap_apr(status, NULL);
}
svn_error_t *
Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=981489&r1=981488&r2=981489&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Mon Aug 2 12:49:45 2010
@@ -1279,9 +1279,12 @@ check_path_under_root(const char *base_p
const char *add_path,
apr_pool_t *pool)
{
- char *full_path;
+ const char *full_path;
+ svn_boolean_t under_root;
- if (! svn_dirent_is_under_root(&full_path, base_path, add_path, pool))
+ SVN_ERR(svn_dirent_is_under_root(&under_root, &full_path, base_path, add_path, pool));
+
+ if (! under_root)
{
return svn_error_createf(
SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
Modified: subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c?rev=981489&r1=981488&r2=981489&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c Mon Aug 2 12:49:45 2010
@@ -2792,6 +2792,73 @@ test_file_url_from_dirent(apr_pool_t *po
return SVN_NO_ERROR;
}
+static svn_error_t *
+test_dirent_is_under_root(apr_pool_t *pool)
+{
+ struct {
+ const char *base_path;
+ const char *path;
+ svn_boolean_t under_root;
+ const char *result;
+ } tests[] = {
+ { "/", "/base", FALSE},
+ { "/aa", "/aa/bb", FALSE},
+ { "/base", "/base2", FALSE},
+ { "/b", "bb", TRUE, "/b/bb"},
+ { "/b", "../bb", FALSE},
+ { "/b", "r/./bb", TRUE, "/b/r/bb"},
+ { "/b", "r/../bb", TRUE, "/b/bb"},
+ { "/b", "r/../../bb", FALSE},
+ { "/b", "./bb", TRUE, "/b/bb"},
+ { "/b", ".", TRUE, "/b"},
+ { "/b", "", TRUE, "/b"},
+ { "b", "b", TRUE, "b/b"},
+#ifdef SVN_USE_DOS_PATHS
+ { "C:/file", "a\\d", TRUE, "C:/file/a/d"},
+ { "C:/file", "aa\\..\\d", TRUE, "C:/file/d"},
+ { "C:/file", "aa\\..\\..\\d", FALSE},
+#else
+ { "C:/file", "a\\d", TRUE, "C:/file/a\\d"},
+ { "C:/file", "aa\\..\\d", TRUE, "C:/file/aa\\..\\d"},
+ { "C:/file", "aa\\..\\..\\d", TRUE, "C:/file/aa\\..\\..\d"},
+#endif
+ };
+ int i;
+
+ for (i = 0; i < COUNT_OF(tests); i++)
+ {
+ svn_boolean_t under_root;
+ const char *result;
+
+ SVN_ERR(svn_dirent_is_under_root(&under_root,
+ &result,
+ tests[i].base_path,
+ tests[i].path,
+ pool));
+
+ if (under_root != tests[i].under_root)
+ return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "svn_dirent_is_under_root(..\"%s\", \"%s\"..)"
+ " returned %s expected %s.",
+ tests[i].base_path,
+ tests[i].path,
+ under_root ? "TRUE" : "FALSE",
+ tests[i].under_root ? "TRUE" : "FALSE");
+
+ if (under_root
+ && strcmp(result, tests[i].result) != 0)
+ return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "svn_dirent_is_under_root(..\"%s\", \"%s\"..)"
+ " found \"%s\" expected \"%s\".",
+ tests[i].base_path,
+ tests[i].path,
+ result,
+ tests[i].result);
+ }
+
+ return SVN_NO_ERROR;
+}
+
/* The test table. */
@@ -2890,5 +2957,7 @@ struct svn_test_descriptor_t test_funcs[
"test svn_uri_get_dirent_from_file_url errors"),
SVN_TEST_PASS2(test_file_url_from_dirent,
"test svn_uri_get_file_url_from_dirent"),
+ SVN_TEST_PASS2(test_dirent_is_under_root,
+ "test svn_dirent_is_under_root"),
SVN_TEST_NULL
};