subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cmpil...@apache.org
Subject svn commit: r1071025 - /subversion/trunk/subversion/svnadmin/main.c
Date Tue, 15 Feb 2011 19:36:40 GMT
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. */



Mime
View raw message