From 34898ce471214e234e14d9532bbb34b8095381cc Mon Sep 17 00:00:00 2001
From: Cynthia Shang <cynthia.shang@crunchydata.com>
Date: Thu, 8 Mar 2018 14:15:05 -0500
Subject: [PATCH] Error when multiple arguments are set in the config file for
 an option that does not accept multiple arguments.

Contributed by Cynthia Shang.
---
 doc/xml/release.xml                |  8 ++++++++
 src/config/parse.c                 | 15 ++++++++-------
 test/src/module/config/parseTest.c | 20 +++++++++++++++++++-
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/doc/xml/release.xml b/doc/xml/release.xml
index 046a2def5..1e61b99ac 100644
--- a/doc/xml/release.xml
+++ b/doc/xml/release.xml
@@ -33,6 +33,14 @@
                 </release-bug-list>
 
                 <release-improvement-list>
+                    <release-item>
+                        <release-item-contributor-list>
+                            <release-item-contributor id="shang.cynthia"/>
+                        </release-item-contributor-list>
+
+                        <p>Error when multiple arguments are set in the config file for an option that does not accept multiple arguments.</p>
+                    </release-item>
+
                     <release-item>
                         <release-item-contributor-list>
                             <release-item-contributor id="vondendriesch.adrian"/>
diff --git a/src/config/parse.c b/src/config/parse.c
index a0ce4a431..ee63212a2 100644
--- a/src/config/parse.c
+++ b/src/config/parse.c
@@ -202,13 +202,6 @@ configParse(unsigned int argListSize, const char *argList[])
                         if (parseOptionList[optionId].reset != reset)
                             THROW(OptionInvalidError, "option '%s' cannot be set and reset", cfgOptionName(optionId));
 
-                        // Error if this option does not allow multiple arguments
-                        if (!(cfgDefOptionType(cfgOptionDefIdFromId(optionId)) == cfgDefOptTypeHash ||
-                              cfgDefOptionType(cfgOptionDefIdFromId(optionId)) == cfgDefOptTypeList))
-                        {
-                            THROW(OptionInvalidError, "option '%s' cannot have multiple arguments", cfgOptionName(optionId));
-                        }
-
                         // Add the argument
                         strLstAdd(parseOptionList[optionId].valueList, strNew(optarg));
                     }
@@ -437,6 +430,14 @@ configParse(unsigned int argListSize, const char *argList[])
                             cfgCommandName(cfgCommand()));
                     }
 
+                    // Error if this option does not allow multiple arguments
+                    if (parseOption->valueList != NULL && strLstSize(parseOption->valueList) > 1 &&
+                        !(cfgDefOptionType(cfgOptionDefIdFromId(optionId)) == cfgDefOptTypeHash ||
+                          cfgDefOptionType(cfgOptionDefIdFromId(optionId)) == cfgDefOptTypeList))
+                    {
+                        THROW(OptionInvalidError, "option '%s' cannot have multiple arguments", cfgOptionName(optionId));
+                    }
+
                     // Is the option valid for this command?  If not, mark it as resolved since there is nothing more to do.
                     cfgOptionValidSet(optionId, cfgDefOptionValid(commandDefId, optionDefId));
 
diff --git a/test/src/module/config/parseTest.c b/test/src/module/config/parseTest.c
index 0b692d47f..6f032b8e8 100644
--- a/test/src/module/config/parseTest.c
+++ b/test/src/module/config/parseTest.c
@@ -97,6 +97,7 @@ testRun()
         // -------------------------------------------------------------------------------------------------------------------------
         argList = strLstNew();
         strLstAdd(argList, strNew(TEST_BACKREST_EXE));
+        strLstAdd(argList, strNew(TEST_COMMAND_BACKUP));
         strLstAdd(argList, strNew("--compress-level=3"));
         strLstAdd(argList, strNew("--compress-level=3"));
         TEST_ERROR(
@@ -296,6 +297,23 @@ testRun()
             strLstSize(argList), strLstPtr(argList)), OptionInvalidError,
             strPtr(strNewFmt("'%s' contains duplicate options ('db-path', 'pg1-path') in section '[db]'", strPtr(configFile))));
 
+        // -------------------------------------------------------------------------------------------------------------------------
+        argList = strLstNew();
+        strLstAdd(argList, strNew(TEST_BACKREST_EXE));
+        strLstAdd(argList, strNew("--stanza=db"));
+        strLstAdd(argList, strNewFmt("--config=%s", strPtr(configFile)));
+        strLstAdd(argList, strNew(TEST_COMMAND_BACKUP));
+
+        storagePut(storageLocal(), configFile, bufNewStr(strNew(
+            "[db]\n"
+            "pg1-path=/path/to/db\n"
+            "pg1-path=/also/path/to/db\n"
+        )));
+
+        TEST_ERROR(configParse(strLstSize(argList), strLstPtr(argList)),
+            OptionInvalidError,
+            "option 'pg1-path' cannot have multiple arguments");
+
         // -------------------------------------------------------------------------------------------------------------------------
         argList = strLstNew();
         strLstAdd(argList, strNew(TEST_BACKREST_EXE));
@@ -410,7 +428,7 @@ testRun()
                     strPtr(configFile), strPtr(configFile), strPtr(configFile), strPtr(configFile), strPtr(configFile),
                     strPtr(configFile))));
 
-        TEST_RESULT_BOOL(cfgOptionTest(cfgOptPgHost), false, "    pg1-path is not set");
+        TEST_RESULT_BOOL(cfgOptionTest(cfgOptPgHost), false, "    pg1-host is not set (command line reset override)");
         TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptPgPath)), "/path/to/db", "    pg1-path is set");
         TEST_RESULT_INT(cfgOptionSource(cfgOptPgPath), cfgSourceConfig, "    pg1-path is source config");
         TEST_RESULT_BOOL(cfgOptionBool(cfgOptCompress), false, "    compress not is set");