From b25f10949a016cc4dc6d2e3ca1c8a63f764059c9 Mon Sep 17 00:00:00 2001
From: David Steele <david@pgmasters.net>
Date: Mon, 15 Aug 2016 20:15:17 -0400
Subject: [PATCH] Fixed #229: Check fails on target db host.

Options that should not allow multiple values could be specified multiple times in pgbackrest.conf without an error being raised.
---
 doc/xml/release.xml                          |  8 ++++++++
 lib/pgBackRest/Common/Exception.pm           |  2 ++
 lib/pgBackRest/Config/Config.pm              |  6 ++++++
 test/lib/pgBackRestTest/Config/ConfigTest.pm | 18 +++++++++++++++++-
 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/doc/xml/release.xml b/doc/xml/release.xml
index 23db1113a..2015be49b 100644
--- a/doc/xml/release.xml
+++ b/doc/xml/release.xml
@@ -119,6 +119,14 @@
 
                         <p>Fixed an issue where a tablespace link that referenced another link would not produce an error, but instead skip the tablespace entirely.</p>
                     </release-item>
+
+                    <release-item>
+                        <release-item-contributor-list>
+                            <release-item-ideator id="vitale.michael"/>
+                        </release-item-contributor-list>
+
+                        <p>Fixed an issue where options that should not allow multiple values could be specified multiple times in <file>pgbackrest.conf</file> without an error being raised.</p>
+                    </release-item>
                 </release-bug-list>
 
                 <release-feature-list>
diff --git a/lib/pgBackRest/Common/Exception.pm b/lib/pgBackRest/Common/Exception.pm
index 17ef5ff7b..1b6b7880d 100644
--- a/lib/pgBackRest/Common/Exception.pm
+++ b/lib/pgBackRest/Common/Exception.pm
@@ -136,6 +136,8 @@ use constant ERROR_ARCHIVE_TIMEOUT                                  => ERROR_MIN
     push @EXPORT, qw(ERROR_ARCHIVE_TIMEOUT);
 use constant ERROR_FILE_MODE                                        => ERROR_MINIMUM + 58;
     push @EXPORT, qw(ERROR_FILE_MODE);
+use constant ERROR_OPTION_MULTIPLE_VALUE                            => ERROR_MINIMUM + 59;
+    push @EXPORT, qw(ERROR_OPTION_MULTIPLE_VALUE);
 
 use constant ERROR_INVALID_VALUE                                    => ERROR_MAXIMUM - 1;
     push @EXPORT, qw(ERROR_INVALID_VALUE);
diff --git a/lib/pgBackRest/Config/Config.pm b/lib/pgBackRest/Config/Config.pm
index 1525f65ec..e89c69cfe 100644
--- a/lib/pgBackRest/Config/Config.pm
+++ b/lib/pgBackRest/Config/Config.pm
@@ -2011,6 +2011,12 @@ sub optionValidate
                                 $$strValue{$strHashKey} = $strHashValue;
                             }
                         }
+                        # In all other cases the value should be scalar
+                        elsif (ref(\$strValue) ne 'SCALAR')
+                        {
+                            confess &log(
+                                ERROR, "option '${strOption}' cannot be specified multiple times", ERROR_OPTION_MULTIPLE_VALUE);
+                        }
 
                         $oOption{$strOption}{source} = SOURCE_CONFIG;
                     }
diff --git a/test/lib/pgBackRestTest/Config/ConfigTest.pm b/test/lib/pgBackRestTest/Config/ConfigTest.pm
index d36a0900c..8745ab620 100755
--- a/test/lib/pgBackRestTest/Config/ConfigTest.pm
+++ b/test/lib/pgBackRestTest/Config/ConfigTest.pm
@@ -156,6 +156,10 @@ sub configLoadExpect
                 $strError = "'${strErrorParam1}' is not valid for '${strErrorParam2}' option" .
                             (defined($strErrorParam3) ? "\nHINT: ${strErrorParam3}." : '');
             }
+            elsif ($iExpectedError == ERROR_OPTION_MULTIPLE_VALUE)
+            {
+                $strError = "option '${strErrorParam1}' cannot be specified multiple times";
+            }
             elsif ($iExpectedError == ERROR_OPTION_INVALID_RANGE)
             {
                 $strError = "'${strErrorParam1}' is not valid for '${strErrorParam2}' option";
@@ -646,7 +650,6 @@ sub configTestRun
             optionTestExpect(OPTION_THREAD_MAX, 3);
         }
 
-
         if (testRun(++$iRun, 'load from config global command section - option ' . OPTION_THREAD_MAX))
         {
             $oConfig = {};
@@ -888,6 +891,19 @@ sub configTestRun
             optionTestExpect(OPTION_REPO_PATH, '/repo');
         }
 
+        if (testRun(++$iRun, CMD_BACKUP . ' option ' . OPTION_REPO_PATH . ' multiple times'))
+        {
+            $oConfig = {};
+            $$oConfig{&CONFIG_SECTION_GLOBAL}{&OPTION_REPO_PATH} = ['/repo', '/repo2'];
+            iniSave($strConfigFile, $oConfig, true);
+
+            optionSetTest($oOption, OPTION_STANZA, $strStanza);
+            optionSetTest($oOption, OPTION_DB_PATH, '/db');
+            optionSetTest($oOption, OPTION_CONFIG, $strConfigFile);
+
+            configLoadExpect($oOption, CMD_BACKUP, ERROR_OPTION_MULTIPLE_VALUE, OPTION_REPO_PATH);
+        }
+
         # Cleanup
         testCleanup();
     }