You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2011/02/15 20:36:40 UTC
svn commit: r1071025 - /subversion/trunk/subversion/svnadmin/main.c
Author: cmpilato
Date: Tue Feb 15 19:36:39 2011
New Revision: 1071025
URL: http://svn.apache.org/viewvc?rev=1071025&view=rev
Log:
Overhaul svnadmin's input validation to consistently check not just
for missing command-line arguments, but also for extra ones.
* subversion/svnadmin/main.c
(parse_local_repos_path): Remove as unused.
(struct svnadmin_opt_state): Remove 'new_repository_path' member.
(target_arg_to_dirent, parse_args): New helper functions.
(set_revprop): Now expect 'filename' to be prepared by callers.
(subcommand_hotcopy): Use parse_args() and target_arg_to_dirent()
instead of parse_local_repos_path().
(subcommand_crashtest, subcommand_create, subcommand_deltify,
subcommand_dump, subcommand_help, subcommand_load,
subcommand_list_dblogs, subcommand_list_unused_dblogs,
subcommand_lslocks, subcommand_lstxns, subcommand_pack,
subcommand_recover, subcommand_setlog, subcommand_setrevprop,
subcommand_setuuid, subcommand_upgrade, subcommand_verify): Update
to validate that the proper number of arguments was provided.
(main): Handle repository path parsing directly instead of using
parse_local_repos_path(), and defer new_repository_path parsing to
subcommand_hotcopy() (the one function that needs it).
Modified:
subversion/trunk/subversion/svnadmin/main.c
Modified: subversion/trunk/subversion/svnadmin/main.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/main.c?rev=1071025&r1=1071024&r2=1071025&view=diff
==============================================================================
--- subversion/trunk/subversion/svnadmin/main.c (original)
+++ subversion/trunk/subversion/svnadmin/main.c Tue Feb 15 19:36:39 2011
@@ -107,41 +107,6 @@ create_stdio_stream(svn_stream_t **strea
}
-/* Helper to parse local repository path. Try parsing next parameter
- * of OS as a local path to repository. If successfull *REPOS_PATH
- * will contain internal style path to the repository.
- */
-static svn_error_t *
-parse_local_repos_path(apr_getopt_t *os,
- const char ** repos_path,
- apr_pool_t *pool)
-{
- *repos_path = NULL;
-
- /* Check to see if there is one more parameter. */
- if (os->ind < os->argc)
- {
- const char * path = os->argv[os->ind++];
- SVN_ERR(svn_utf_cstring_to_utf8(repos_path, path, pool));
- *repos_path = svn_dirent_internal_style(*repos_path, pool);
- }
-
- if (*repos_path == NULL)
- {
- return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
- _("Repository argument required"));
- }
- else if (svn_path_is_url(*repos_path))
- {
- return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
- _("'%s' is an URL when it should be a path"),
- *repos_path);
- }
-
- return SVN_NO_ERROR;
-}
-
-
/* Custom filesystem warning function. */
static void
warning_func(void *baton,
@@ -493,7 +458,6 @@ static const svn_opt_subcommand_desc2_t
struct svnadmin_opt_state
{
const char *repository_path;
- const char *new_repository_path; /* hotcopy dest. path */
const char *fs_type; /* --fs-type */
svn_boolean_t pre_1_4_compatible; /* --pre-1.4-compatible */
svn_boolean_t pre_1_5_compatible; /* --pre-1.5-compatible */
@@ -551,6 +515,61 @@ get_revnum(svn_revnum_t *revnum, const s
return SVN_NO_ERROR;
}
+/* Set *PATH to an internal-style, UTF8-encoded, local dirent path
+ allocated from POOL and parsed from raw command-line argument ARG. */
+static svn_error_t *
+target_arg_to_dirent(const char **dirent,
+ const char *arg,
+ apr_pool_t *pool)
+{
+ const char *path;
+
+ SVN_ERR(svn_utf_cstring_to_utf8(&path, arg, pool));
+ if (svn_path_is_url(path))
+ return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ "Path '%s' is not a local path", path);
+ *dirent = svn_dirent_internal_style(path, pool);
+ return SVN_NO_ERROR;
+}
+
+/* Parse the remaining command-line arguments from OS, returning them
+ in a new array *ARGS (allocated from POOL) and optionally verifying
+ that we got the expected number thereof. If MIN_EXPECTED is not
+ negative, return an error if the function would return fewer than
+ MIN_EXPECTED arguments. If MAX_EXPECTED is not negative, return an
+ error if the function would return more than MAX_EXPECTED
+ arguments.
+
+ As a special case, when MIN_EXPECTED and MAX_EXPECTED are both 0,
+ allow ARGS to be NULL. */
+static svn_error_t *
+parse_args(apr_array_header_t **args,
+ apr_getopt_t *os,
+ int min_expected,
+ int max_expected,
+ apr_pool_t *pool)
+{
+ int num_args = os->argc - os->ind;
+
+ if (min_expected || max_expected)
+ SVN_ERR_ASSERT(args);
+
+ if ((min_expected >= 0) && (num_args < min_expected))
+ return svn_error_create(SVN_ERR_CL_INSUFFICIENT_ARGS, 0,
+ "Not enough arguments");
+ if ((max_expected >= 0) && (num_args > max_expected))
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, 0,
+ "Too many arguments");
+ if (args)
+ {
+ *args = apr_array_make(pool, num_args, sizeof(const char *));
+ while (os->ind < os->argc)
+ APR_ARRAY_PUSH(*args, const char *) =
+ apr_pstrdup(pool, os->argv[os->ind++]);
+ }
+
+ return SVN_NO_ERROR;
+}
/* This implements `svn_opt_subcommand_t'. */
static svn_error_t *
@@ -561,6 +580,9 @@ subcommand_create(apr_getopt_t *os, void
apr_hash_t *config;
apr_hash_t *fs_config = apr_hash_make(pool);
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
apr_hash_set(fs_config, SVN_FS_CONFIG_BDB_TXN_NOSYNC,
APR_HASH_KEY_STRING,
(opt_state->bdb_txn_nosync ? "1" : "0"));
@@ -614,6 +636,9 @@ subcommand_deltify(apr_getopt_t *os, voi
svn_revnum_t youngest, revision;
apr_pool_t *subpool = svn_pool_create(pool);
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
fs = svn_repos_fs(repos);
SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool));
@@ -864,6 +889,9 @@ subcommand_dump(apr_getopt_t *os, void *
svn_revnum_t youngest;
svn_stream_t *progress_stream = NULL;
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
fs = svn_repos_fs(repos);
SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool));
@@ -921,6 +949,9 @@ subcommand_help(apr_getopt_t *os, void *
svn_stringbuf_t *version_footer;
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
version_footer = svn_stringbuf_create(fs_desc_start, pool);
SVN_ERR(svn_fs_print_modules(version_footer, pool));
@@ -944,6 +975,9 @@ subcommand_load(apr_getopt_t *os, void *
svn_repos_t *repos;
svn_stream_t *stdin_stream, *stdout_stream = NULL;
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
/* Read the stream from STDIN. Users can redirect a file. */
@@ -981,6 +1015,9 @@ subcommand_lstxns(apr_getopt_t *os, void
apr_array_header_t *txns;
int i;
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
fs = svn_repos_fs(repos);
SVN_ERR(svn_fs_list_transactions(&txns, fs, pool));
@@ -1006,6 +1043,9 @@ subcommand_recover(apr_getopt_t *os, voi
struct svnadmin_opt_state *opt_state = baton;
svn_stream_t *stdout_stream;
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
SVN_ERR(create_stdio_stream(&stdout_stream, apr_file_open_stdout, pool));
/* Restore default signal handlers until after we have acquired the
@@ -1059,6 +1099,9 @@ list_dblogs(apr_getopt_t *os, void *bato
apr_array_header_t *logfiles;
int i;
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
SVN_ERR(svn_repos_db_logfiles(&logfiles,
opt_state->repository_path,
only_unused,
@@ -1094,6 +1137,9 @@ subcommand_list_dblogs(apr_getopt_t *os,
static svn_error_t *
subcommand_list_unused_dblogs(apr_getopt_t *os, void *baton, apr_pool_t *pool)
{
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
SVN_ERR(list_dblogs(os, baton, TRUE, pool));
return SVN_NO_ERROR;
}
@@ -1111,11 +1157,11 @@ subcommand_rmtxns(apr_getopt_t *os, void
int i;
apr_pool_t *subpool = svn_pool_create(pool);
+ SVN_ERR(svn_opt_parse_all_args(&args, os, pool));
+
SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
fs = svn_repos_fs(repos);
- SVN_ERR(svn_opt_parse_all_args(&args, os, pool));
-
/* All the rest of the arguments are transaction names. */
for (i = 0; i < args->nelts; i++)
{
@@ -1173,12 +1219,8 @@ set_revprop(const char *prop_name, const
svn_repos_t *repos;
svn_string_t *prop_value = svn_string_create("", pool);
svn_stringbuf_t *file_contents;
- const char *filename_utf8;
- SVN_ERR(svn_utf_cstring_to_utf8(&filename_utf8, filename, pool));
- filename_utf8 = svn_dirent_internal_style(filename_utf8, pool);
-
- SVN_ERR(svn_stringbuf_from_file2(&file_contents, filename_utf8, pool));
+ SVN_ERR(svn_stringbuf_from_file2(&file_contents, filename, pool));
prop_value->data = file_contents->data;
prop_value->len = file_contents->len;
@@ -1208,6 +1250,13 @@ subcommand_setrevprop(apr_getopt_t *os,
{
struct svnadmin_opt_state *opt_state = baton;
apr_array_header_t *args;
+ const char *prop_name, *filename;
+
+ /* Expect two more arguments: NAME FILE */
+ SVN_ERR(parse_args(&args, os, 2, 2, pool));
+ prop_name = APR_ARRAY_IDX(args, 0, const char *);
+ filename = APR_ARRAY_IDX(args, 1, const char *);
+ SVN_ERR(target_arg_to_dirent(&filename, filename, pool));
if (opt_state->start_revision.kind != svn_opt_revision_number)
return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
@@ -1216,16 +1265,7 @@ subcommand_setrevprop(apr_getopt_t *os,
return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
_("Only one revision allowed"));
- SVN_ERR(svn_opt_parse_all_args(&args, os, pool));
-
- if (args->nelts != 2)
- return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
- _("Exactly one property name and one file "
- "argument required"));
-
- return set_revprop(APR_ARRAY_IDX(args, 0, const char *),
- APR_ARRAY_IDX(args, 1, const char *),
- opt_state, pool);
+ return set_revprop(prop_name, filename, opt_state, pool);
}
@@ -1239,10 +1279,8 @@ subcommand_setuuid(apr_getopt_t *os, voi
svn_fs_t *fs;
const char *uuid = NULL;
- SVN_ERR(svn_opt_parse_all_args(&args, os, pool));
-
- if (args->nelts > 1)
- return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL, NULL);
+ /* Expect zero or one more arguments: [UUID] */
+ SVN_ERR(parse_args(&args, os, 0, 1, pool));
if (args->nelts == 1)
uuid = APR_ARRAY_IDX(args, 0, const char *);
@@ -1258,6 +1296,12 @@ subcommand_setlog(apr_getopt_t *os, void
{
struct svnadmin_opt_state *opt_state = baton;
apr_array_header_t *args;
+ const char *filename;
+
+ /* Expect one more argument: FILE */
+ SVN_ERR(parse_args(&args, os, 1, 1, pool));
+ filename = APR_ARRAY_IDX(args, 0, const char *);
+ SVN_ERR(target_arg_to_dirent(&filename, filename, pool));
if (opt_state->start_revision.kind != svn_opt_revision_number)
return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
@@ -1266,12 +1310,6 @@ subcommand_setlog(apr_getopt_t *os, void
return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
_("Only one revision allowed"));
- SVN_ERR(svn_opt_parse_all_args(&args, os, pool));
-
- if (args->nelts != 1)
- return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
- _("Exactly one file argument required"));
-
/* set_revprop() responds only to pre-/post-revprop-change opts. */
if (!opt_state->bypass_hooks)
{
@@ -1279,9 +1317,7 @@ subcommand_setlog(apr_getopt_t *os, void
opt_state->use_post_revprop_change_hook = TRUE;
}
- return set_revprop(SVN_PROP_REVISION_LOG,
- APR_ARRAY_IDX(args, 0, const char *),
- opt_state, pool);
+ return set_revprop(SVN_PROP_REVISION_LOG, filename, opt_state, pool);
}
@@ -1293,6 +1329,9 @@ subcommand_pack(apr_getopt_t *os, void *
svn_repos_t *repos;
svn_stream_t *progress_stream = NULL;
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
/* Progress feedback goes to STDOUT, unless they asked to suppress it. */
@@ -1315,6 +1354,9 @@ subcommand_verify(apr_getopt_t *os, void
svn_revnum_t youngest, lower, upper;
svn_stream_t *progress_stream;
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
fs = svn_repos_fs(repos);
SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool));
@@ -1339,19 +1381,21 @@ subcommand_verify(apr_getopt_t *os, void
progress_stream, check_cancel, NULL, pool);
}
-
/* This implements `svn_opt_subcommand_t'. */
svn_error_t *
subcommand_hotcopy(apr_getopt_t *os, void *baton, apr_pool_t *pool)
{
struct svnadmin_opt_state *opt_state = baton;
+ apr_array_header_t *targets;
+ const char *new_repos_path;
- SVN_ERR(svn_repos_hotcopy(opt_state->repository_path,
- opt_state->new_repository_path,
- opt_state->clean_logs,
- pool));
+ /* Expect one more argument: NEW_REPOS_PATH */
+ SVN_ERR(parse_args(&targets, os, 1, 1, pool));
+ new_repos_path = APR_ARRAY_IDX(targets, 0, const char *);
+ SVN_ERR(target_arg_to_dirent(&new_repos_path, new_repos_path, pool));
- return SVN_NO_ERROR;
+ return svn_repos_hotcopy(opt_state->repository_path, new_repos_path,
+ opt_state->clean_logs, pool);
}
@@ -1503,6 +1547,9 @@ subcommand_upgrade(apr_getopt_t *os, voi
struct svnadmin_opt_state *opt_state = baton;
svn_stream_t *stdout_stream;
+ /* Expect no more arguments. */
+ SVN_ERR(parse_args(NULL, os, 0, 0, pool));
+
SVN_ERR(create_stdio_stream(&stdout_stream, apr_file_open_stdout, pool));
/* Restore default signal handlers. */
@@ -1787,7 +1834,7 @@ main(int argc, const char *argv[])
subcommand = svn_opt_get_canonical_subcommand2(cmd_table, first_arg);
if (subcommand == NULL)
{
- const char* first_arg_utf8;
+ const char *first_arg_utf8;
err = svn_utf_cstring_to_utf8(&first_arg_utf8, first_arg, pool);
if (err)
return svn_cmdline_handle_exit_error(err, pool, "svnadmin: ");
@@ -1801,28 +1848,34 @@ main(int argc, const char *argv[])
}
}
- /* If there's a second argument, it's probably the repository.
- Every subcommand except `help' requires one, so we parse it out
- here and store it in opt_state. */
+ /* Every subcommand except `help' requires a second argument -- the
+ repository path. Parse it out here and store it in opt_state. */
if (subcommand->cmd_func != subcommand_help)
{
- err = parse_local_repos_path(os,
- &(opt_state.repository_path),
- pool);
- if (err)
- return svn_cmdline_handle_exit_error(err, pool, "svnadmin: ");
- }
+ const char *repos_path = NULL;
+
+ if (os->ind >= os->argc)
+ {
+ err = svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("Repository argument required"));
+ return svn_cmdline_handle_exit_error(err, pool, "svnadmin: ");
+ }
+ if ((err = svn_utf_cstring_to_utf8(&repos_path,
+ os->argv[os->ind++], pool)))
+ {
+ return svn_cmdline_handle_exit_error(err, pool, "svnadmin: ");
+ }
- /* If command is hot copy the third argument will be the new
- repository path. */
- if (subcommand->cmd_func == subcommand_hotcopy)
- {
- err = parse_local_repos_path(os,
- &(opt_state.new_repository_path),
- pool);
- if (err)
- return svn_cmdline_handle_exit_error(err, pool, "svnadmin: ");
+ if (svn_path_is_url(repos_path))
+ {
+ err = svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("'%s' is a URL when it should be a "
+ "local path"), repos_path);
+ return svn_cmdline_handle_exit_error(err, pool, "svnadmin: ");
+ }
+
+ opt_state.repository_path = repos_path;
}
/* Check that the subcommand wasn't passed any inappropriate options. */