Author: stsp
Date: Mon Jul 19 16:27:09 2010
New Revision: 965551
URL: http://svn.apache.org/viewvc?rev=965551&view=rev
Log:
As part of addressing issue #3620, make 'svn add' properly validate targets.
This fixes a user-triggable assertion.
This is just a small start of a comprehensive path/URL input validation audit
that needs to be performed to properly fix issue #3620. So similar commits
for other subcommands will follow.
Also, introduce a new regression test suite for testing input validation.
This only tests 'svn add' for now, but more tests will follow.
Input validation is done both right beneath the client API layer and
within the CLI client. This makes sure that our CLI client behaves well,
i.e. it won't ask the client library to perform operations it knows might
fail due to invalid input. The checks within the client library help third-
party clients which don't perform proper input validation even though they
should.
* subversion/libsvn_client/add.c
(svn_client_add4): Raise an error if the path to be added looks like a URL.
* subversion/svn/add-cmd.c
(svn_cl__add): Raise an error if any of the targets supplied by the
user look like URLs. Heed our own API requirements by canonicalizing
paths before passing them to the client library!!!
Rename a subpool to iterpool while here, since the pool is used in a loop.
* subversion/tests/cmdline/input_validation_tests.py: New.
Patch by: Uwe Stuehler <subversion-li...@bsdx.de>
me
Added:
subversion/trunk/subversion/tests/cmdline/input_validation_tests.py (with
props)
Modified:
subversion/trunk/subversion/libsvn_client/add.c
subversion/trunk/subversion/svn/add-cmd.c
Modified: subversion/trunk/subversion/libsvn_client/add.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/add.c?rev=965551&r1=965550&r2=965551&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/add.c (original)
+++ subversion/trunk/subversion/libsvn_client/add.c Mon Jul 19 16:27:09 2010
@@ -573,6 +573,11 @@ svn_client_add4(const char *path,
const char *local_abspath;
struct add_with_write_lock_baton baton;
+ if (svn_path_is_url(path))
+ return svn_error_return(svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
+ _("'%s' is not a local path"),
+ path));
+
SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
/* ### this is a hack.
Modified: subversion/trunk/subversion/svn/add-cmd.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/add-cmd.c?rev=965551&r1=965550&r2=965551&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/add-cmd.c (original)
+++ subversion/trunk/subversion/svn/add-cmd.c Mon Jul 19 16:27:09 2010
@@ -29,11 +29,13 @@
#define APR_WANT_STDIO
#include <apr_want.h>
+#include "svn_path.h"
#include "svn_client.h"
#include "svn_error.h"
#include "svn_pools.h"
#include "cl.h"
+#include "svn_private_config.h"
/*** Code. ***/
@@ -48,7 +50,7 @@ svn_cl__add(apr_getopt_t *os,
svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
apr_array_header_t *targets;
int i;
- apr_pool_t *subpool;
+ apr_pool_t *iterpool;
SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
opt_state->targets,
@@ -66,24 +68,37 @@ svn_cl__add(apr_getopt_t *os,
SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
- subpool = svn_pool_create(pool);
+ /* Don't even attempt to modify the working copy if any of the
+ * targets look like URLs. URLs are invalid input. */
for (i = 0; i < targets->nelts; i++)
{
const char *target = APR_ARRAY_IDX(targets, i, const char *);
- svn_pool_clear(subpool);
+ if (svn_path_is_url(target))
+ return svn_error_return(svn_error_createf(SVN_ERR_ILLEGAL_TARGET,
+ NULL,
+ _("'%s' is not a local
path"),
+ target));
+ }
+
+ iterpool = svn_pool_create(pool);
+ for (i = 0; i < targets->nelts; i++)
+ {
+ const char *target = APR_ARRAY_IDX(targets, i, const char *);
+
+ svn_pool_clear(iterpool);
SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
SVN_ERR(svn_cl__try
- (svn_client_add4(target,
+ (svn_client_add4(svn_dirent_canonicalize(target, iterpool),
opt_state->depth,
opt_state->force, opt_state->no_ignore,
- opt_state->parents, ctx, subpool),
+ opt_state->parents, ctx, iterpool),
NULL, opt_state->quiet,
SVN_ERR_ENTRY_EXISTS,
SVN_ERR_WC_PATH_NOT_FOUND,
SVN_NO_ERROR));
}
- svn_pool_destroy(subpool);
+ svn_pool_destroy(iterpool);
return SVN_NO_ERROR;
}
Added: subversion/trunk/subversion/tests/cmdline/input_validation_tests.py
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/input_validation_tests.py?rev=965551&view=auto
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/input_validation_tests.py (added)
+++ subversion/trunk/subversion/tests/cmdline/input_validation_tests.py Mon Jul
19 16:27:09 2010
@@ -0,0 +1,83 @@
+#!/usr/bin/env python
+#
+# input_validation_tests.py: testing input validation
+#
+# Subversion is a tool for revision control.
+# See http://subversion.apache.org for more information.
+#
+# ====================================================================
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+######################################################################
+
+import os
+import svntest
+
+# (abbreviation)
+Skip = svntest.testcase.Skip
+SkipUnless = svntest.testcase.SkipUnless
+XFail = svntest.testcase.XFail
+Item = svntest.wc.StateItem
+
+
+######################################################################
+# Utilities
+
+# Common URL targets to pass where only path arguments are expected.
+_invalid_wc_path_targets = ['file:///', '^/']
+
+def run_and_verify_svn_in_wc(sbox, expected_stderr, *varargs):
+ """Like svntest.actions.run_and_verify_svn, but temporarily
+ changes the current working directory to the sandboxes'
+ working copy and only checks the expected error output."""
+
+ wc_dir = sbox.wc_dir
+ old_dir = os.getcwd()
+ try:
+ os.chdir(wc_dir)
+ svntest.actions.run_and_verify_svn(None, [], expected_stderr,
+ *varargs)
+ finally:
+ os.chdir(old_dir)
+
+
+######################################################################
+# Tests
+#
+# Each test must return on success or raise on failure.
+#----------------------------------------------------------------------
+
+def invalid_wcpath_add(sbox):
+ "non-working copy paths for 'add'"
+ sbox.build()
+ for target in _invalid_wc_path_targets:
+ run_and_verify_svn_in_wc(sbox, "svn:.*is not a local path", 'add', target)
+
+########################################################################
+# Run the tests
+
+# list all tests here, starting with None:
+test_list = [ None,
+ invalid_wcpath_add,
+ ]
+
+if __name__ == '__main__':
+ svntest.main.run_tests(test_list)
+ # NOTREACHED
+
+
+### End of file.
Propchange: subversion/trunk/subversion/tests/cmdline/input_validation_tests.py
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: subversion/trunk/subversion/tests/cmdline/input_validation_tests.py
------------------------------------------------------------------------------
svn:executable = *
