mirror of
https://github.com/pgbackrest/pgbackrest.git
synced 2025-03-03 14:52:21 +02:00
Error when parameters are passed to a command that does not accept parameters.
This behavior allowed a command like this to run without error: pgbackrest backup --stanza=db full Even though it actually performed an incremental backup in most circumstances because the `full` parameter was ignored. Instead, output an error and exit. Suggested by Jason O'Donnell.
This commit is contained in:
parent
cad6fedb7b
commit
60fe5b7365
@ -148,6 +148,7 @@ sub buildConfig
|
||||
ucfirst(lc($rhCommand->{&CFGDEF_LOG_LEVEL_STDERR_MAX})) . ")\n" .
|
||||
" CONFIG_COMMAND_LOCK_REQUIRED(" . ($rhCommand->{&CFGDEF_LOCK_REQUIRED} ? 'true' : 'false') . ")\n" .
|
||||
" CONFIG_COMMAND_LOCK_TYPE(lockType" . ucfirst(lc($rhCommand->{&CFGDEF_LOCK_TYPE})) . ")\n" .
|
||||
" CONFIG_COMMAND_PARAMETER_ALLOWED(" . ($rhCommand->{&CFGDEF_PARAMETER_ALLOWED} ? 'true' : 'false') . ")\n" .
|
||||
" )\n";
|
||||
|
||||
$iCommandTotal++;
|
||||
|
@ -493,6 +493,11 @@ use constant CFGDEF_LOCK_TYPE_ALL => 'all';
|
||||
use constant CFGDEF_LOCK_TYPE_NONE => 'none';
|
||||
push @EXPORT, qw(CFGDEF_LOCK_TYPE_NONE);
|
||||
|
||||
# Does the command allow parameters? If not then the config parser will automatically error out if parameters are detected. If so,
|
||||
# then the command is responsible for ensuring that the parameters are valid.
|
||||
use constant CFGDEF_PARAMETER_ALLOWED => 'parameter-allowed';
|
||||
push @EXPORT, qw(CFGDEF_PARAMETER_ALLOWED);
|
||||
|
||||
# Option defines
|
||||
#-----------------------------------------------------------------------------------------------------------------------------------
|
||||
use constant CFGDEF_ALLOW_LIST => 'allow-list';
|
||||
@ -567,6 +572,7 @@ my $rhCommandDefine =
|
||||
{
|
||||
&CFGDEF_LOG_FILE => false,
|
||||
&CFGDEF_LOCK_TYPE => CFGDEF_LOCK_TYPE_ARCHIVE,
|
||||
&CFGDEF_PARAMETER_ALLOWED => true,
|
||||
},
|
||||
|
||||
&CFGCMD_ARCHIVE_GET_ASYNC =>
|
||||
@ -574,12 +580,14 @@ my $rhCommandDefine =
|
||||
&CFGDEF_LOG_FILE => true,
|
||||
&CFGDEF_LOCK_REQUIRED => true,
|
||||
&CFGDEF_LOCK_TYPE => CFGDEF_LOCK_TYPE_ARCHIVE,
|
||||
&CFGDEF_PARAMETER_ALLOWED => true,
|
||||
},
|
||||
|
||||
&CFGCMD_ARCHIVE_PUSH =>
|
||||
{
|
||||
&CFGDEF_LOG_FILE => false,
|
||||
&CFGDEF_LOCK_TYPE => CFGDEF_LOCK_TYPE_ARCHIVE,
|
||||
&CFGDEF_PARAMETER_ALLOWED => true,
|
||||
},
|
||||
|
||||
&CFGCMD_BACKUP =>
|
||||
@ -603,6 +611,7 @@ my $rhCommandDefine =
|
||||
{
|
||||
&CFGDEF_LOG_FILE => false,
|
||||
&CFGDEF_LOG_LEVEL_DEFAULT => DEBUG,
|
||||
&CFGDEF_PARAMETER_ALLOWED => true,
|
||||
},
|
||||
|
||||
&CFGCMD_INFO =>
|
||||
@ -2479,6 +2488,12 @@ foreach my $strCommand (sort(keys(%{$rhCommandDefine})))
|
||||
confess &log(ERROR, "lock type is required for command '${strCommand}' and cannot be 'none'");
|
||||
}
|
||||
}
|
||||
|
||||
# Default parameter allowed is false
|
||||
if (!defined($rhCommandDefine->{$strCommand}{&CFGDEF_PARAMETER_ALLOWED}))
|
||||
{
|
||||
$rhCommandDefine->{$strCommand}{&CFGDEF_PARAMETER_ALLOWED} = false;
|
||||
}
|
||||
}
|
||||
|
||||
####################################################################################################################################
|
||||
|
@ -53,6 +53,14 @@
|
||||
<p>Enable socket keep-alive on older <proper>Perl</proper> versions.</p>
|
||||
</release-item>
|
||||
|
||||
<release-item>
|
||||
<release-item-contributor-list>
|
||||
<release-item-ideator id="jason.odonnell"/>
|
||||
</release-item-contributor-list>
|
||||
|
||||
<p>Error when parameters are passed to a command that does not accept parameters.</p>
|
||||
</release-item>
|
||||
|
||||
<release-item>
|
||||
<release-item-contributor-list>
|
||||
<release-item-ideator id="james.badger"/>
|
||||
|
@ -18,6 +18,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(false)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeArchive)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(true)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -29,6 +30,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(true)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeArchive)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(true)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -40,6 +42,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(false)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeArchive)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(true)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -51,6 +54,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(true)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeBackup)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -62,6 +66,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(false)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeNone)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -73,6 +78,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(true)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeBackup)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -84,6 +90,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(false)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeNone)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(true)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -95,6 +102,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(false)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeNone)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -106,6 +114,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelError)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(false)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeNone)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -117,6 +126,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelError)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(false)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeNone)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -128,6 +138,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(false)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeNone)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -139,6 +150,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(true)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeAll)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -150,6 +162,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(true)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeAll)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -161,6 +174,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(true)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeAll)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -172,6 +186,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(false)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeNone)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -183,6 +198,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(false)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeNone)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
|
||||
CONFIG_COMMAND
|
||||
@ -194,6 +210,7 @@ static ConfigCommandData configCommandData[CFG_COMMAND_TOTAL] = CONFIG_COMMAND_L
|
||||
CONFIG_COMMAND_LOG_LEVEL_STDERR_MAX(logLevelTrace)
|
||||
CONFIG_COMMAND_LOCK_REQUIRED(false)
|
||||
CONFIG_COMMAND_LOCK_TYPE(lockTypeNone)
|
||||
CONFIG_COMMAND_PARAMETER_ALLOWED(false)
|
||||
)
|
||||
)
|
||||
|
||||
|
@ -21,6 +21,8 @@ typedef struct ConfigCommandData
|
||||
bool logFile:1;
|
||||
unsigned int logLevelDefault:4;
|
||||
unsigned int logLevelStdErrMax:4;
|
||||
|
||||
bool parameterAllowed:1;
|
||||
} ConfigCommandData;
|
||||
|
||||
#define CONFIG_COMMAND_LIST(...) \
|
||||
@ -41,6 +43,8 @@ typedef struct ConfigCommandData
|
||||
.logLevelStdErrMax = logLevelStdErrMaxParam,
|
||||
#define CONFIG_COMMAND_NAME(nameParam) \
|
||||
.name = nameParam,
|
||||
#define CONFIG_COMMAND_PARAMETER_ALLOWED(parameterAllowedParam) \
|
||||
.parameterAllowed = parameterAllowedParam,
|
||||
|
||||
/***********************************************************************************************************************************
|
||||
Map options names and indexes to option definitions.
|
||||
@ -384,6 +388,19 @@ cfgLogLevelStdErrMax(void)
|
||||
FUNCTION_TEST_RETURN((LogLevel)configCommandData[cfgCommand()].logLevelStdErrMax);
|
||||
}
|
||||
|
||||
/***********************************************************************************************************************************
|
||||
Does this command allow parameters?
|
||||
***********************************************************************************************************************************/
|
||||
bool
|
||||
cfgParameterAllowed(void)
|
||||
{
|
||||
FUNCTION_TEST_VOID();
|
||||
|
||||
ASSERT(command != cfgCmdNone);
|
||||
|
||||
FUNCTION_TEST_RETURN(configCommandData[cfgCommand()].parameterAllowed);
|
||||
}
|
||||
|
||||
/***********************************************************************************************************************************
|
||||
Get the option define for this option
|
||||
***********************************************************************************************************************************/
|
||||
|
@ -38,6 +38,8 @@ bool cfgLogFile(void);
|
||||
LogLevel cfgLogLevelDefault(void);
|
||||
LogLevel cfgLogLevelStdErrMax(void);
|
||||
|
||||
bool cfgParameterAllowed(void);
|
||||
|
||||
const StringList *cfgCommandParam(void);
|
||||
|
||||
/***********************************************************************************************************************************
|
||||
|
@ -541,7 +541,12 @@ configParse(unsigned int argListSize, const char *argList[], bool resetLogLevel)
|
||||
|
||||
// Set command params
|
||||
if (commandParamList != NULL)
|
||||
{
|
||||
if (!cfgCommandHelp() && !cfgParameterAllowed())
|
||||
THROW(ParamInvalidError, "command does not allow parameters");
|
||||
|
||||
cfgCommandParamSet(commandParamList);
|
||||
}
|
||||
|
||||
// Enable logging (except for local and remote commands) so config file warnings will be output
|
||||
if (cfgCommand() != cfgCmdLocal && cfgCommand() != cfgCmdRemote && resetLogLevel)
|
||||
|
@ -64,6 +64,7 @@ testRun(void)
|
||||
TEST_RESULT_BOOL(cfgLogFile(), true, "log file is on");
|
||||
TEST_RESULT_BOOL(cfgLockRequired(), true, "lock is required");
|
||||
TEST_RESULT_INT(cfgLockType(), lockTypeBackup, "lock is type backup");
|
||||
TEST_RESULT_BOOL(cfgParameterAllowed(), false, "parameters not allowed");
|
||||
|
||||
TEST_RESULT_VOID(cfgCommandSet(cfgCmdInfo), "command set to info");
|
||||
TEST_RESULT_INT(cfgLogLevelDefault(), logLevelDebug, "default log level is debug");
|
||||
|
@ -575,6 +575,22 @@ testRun(void)
|
||||
configParse(strLstSize(argList), strLstPtr(argList), false), OptionInvalidError,
|
||||
"option '--pg1-host' requires argument");
|
||||
|
||||
// -------------------------------------------------------------------------------------------------------------------------
|
||||
argList = strLstNew();
|
||||
strLstAdd(argList, strNew(TEST_BACKREST_EXE));
|
||||
strLstAdd(argList, strNew("backup"));
|
||||
strLstAdd(argList, strNew("param1"));
|
||||
TEST_ERROR(
|
||||
configParse(strLstSize(argList), strLstPtr(argList), false), ParamInvalidError, "command does not allow parameters");
|
||||
|
||||
// -------------------------------------------------------------------------------------------------------------------------
|
||||
argList = strLstNew();
|
||||
strLstAdd(argList, strNew(TEST_BACKREST_EXE));
|
||||
strLstAdd(argList, strNew("help"));
|
||||
strLstAdd(argList, strNew("backup"));
|
||||
strLstAdd(argList, strNew("param1"));
|
||||
TEST_RESULT_VOID(configParse(strLstSize(argList), strLstPtr(argList), false), "ignore params when help command");
|
||||
|
||||
// -------------------------------------------------------------------------------------------------------------------------
|
||||
argList = strLstNew();
|
||||
strLstAdd(argList, strNew(TEST_BACKREST_EXE));
|
||||
|
Loading…
x
Reference in New Issue
Block a user