From 3161dece5f6efdde461e0f9984d0247b96b25b82 Mon Sep 17 00:00:00 2001
From: Franco Fichtner <franco@opnsense.org>
Date: Mon, 11 Apr 2016 11:26:56 +0200
Subject: [PATCH] configuration: pin down backup count+allow none+simplify GUI

---
 src/etc/config.xml.sample      |  1 +
 src/etc/inc/config.lib.inc     |  3 +-
 src/etc/inc/upgrade_config.inc |  4 +++
 src/www/diag_confbak.php       | 60 +++++++++++++++++++++++-----------
 4 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/src/etc/config.xml.sample b/src/etc/config.xml.sample
index 03da7a8b7..c34c2aa91 100644
--- a/src/etc/config.xml.sample
+++ b/src/etc/config.xml.sample
@@ -227,6 +227,7 @@
       <interval>monthly</interval>
     </bogons>
     <kill_states/>
+    <backupcount>60</backupcount>
   </system>
   <interfaces>
     <wan>
diff --git a/src/etc/inc/config.lib.inc b/src/etc/inc/config.lib.inc
index c1fc765fb..d65fefe06 100644
--- a/src/etc/inc/config.lib.inc
+++ b/src/etc/inc/config.lib.inc
@@ -288,7 +288,8 @@ function cleanup_backups()
     if (isset($config['system']['backupcount']) && is_numeric($config['system']['backupcount']) && ($config['system']['backupcount'] >= 0)) {
         $revisions = intval($config['system']['backupcount']);
     } else {
-        $revisions = 30;
+        /* XXX this value used to be left out of the config */
+        $revisions = 60;
     }
 
     $cnf = OPNsense\Core\Config::getInstance();
diff --git a/src/etc/inc/upgrade_config.inc b/src/etc/inc/upgrade_config.inc
index c66d049d1..bac5f8ad9 100644
--- a/src/etc/inc/upgrade_config.inc
+++ b/src/etc/inc/upgrade_config.inc
@@ -3432,4 +3432,8 @@ function upgrade_111_to_112()
 			}
 		}
 	}
+
+	if (!isset($config['system']['backupcount'])) {
+		$config['system']['backupcount'] = 60;
+	}
 }
diff --git a/src/www/diag_confbak.php b/src/www/diag_confbak.php
index 213fc4d57..635e9fbb2 100644
--- a/src/www/diag_confbak.php
+++ b/src/www/diag_confbak.php
@@ -33,6 +33,12 @@ require_once("guiconfig.inc");
 if ($_SERVER['REQUEST_METHOD'] === 'GET') {
     $cnf = OPNsense\Core\Config::getInstance();
     $confvers = $cnf->getBackups(true);
+    if (isset($config['system']['backupcount'])) {
+        $pconfig['backupcount'] = $config['system']['backupcount'];
+    } else {
+        # XXX fallback value for older configs
+        $pconfig['backupcount'] = 60;
+    }
     if (!empty($_GET['getcfg'])) {
         foreach ($confvers as $filename => $revision) {
             if ($revision['time'] == $_GET['getcfg']) {
@@ -79,16 +85,24 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
         }
     }
 } elseif ($_SERVER['REQUEST_METHOD'] === 'POST') {
+    $input_errors = array();
+    $pconfig = $_POST;
+
+    if (!empty($pconfig['save'])) {
+        if (!isset($pconfig['backupcount']) || !is_numeric($pconfig['backupcount']) || $pconfig['backupcount'] < 0) {
+            $input_errors[] = gettext('Backup count must be greater or equal to zero.');
+        }
+        if (count($input_errors) == 0) {
+            $config['system']['backupcount'] = $pconfig['backupcount'];
+            write_config(gettext('Changed backup revision count.'));
+            $savemsg = get_std_save_message();
+        }
+    }
+
     $cnf = OPNsense\Core\Config::getInstance();
     $confvers = $cnf->getBackups(true);
-    if (!empty($_POST['save'])) {
-        if (is_numeric($_POST['backupcount']) && ($_POST['backupcount'] >= 0)) {
-            $config['system']['backupcount'] = $_POST['backupcount'];
-        } elseif (isset($config['system']['backupcount'])) {
-            unset($config['system']['backupcount']);
-        }
-        write_config(gettext('Changed backup revision count.'));
-    } elseif (!empty($_POST['act']) && $_POST['act'] == "revert") {
+
+    if (!empty($_POST['act']) && $_POST['act'] == "revert") {
         foreach ($confvers as $filename => $revision) {
             if (isset($revision['time']) && $revision['time'] == $_POST['time']) {
                 if (config_restore($filename) == 0) {
@@ -113,14 +127,11 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
             }
         }
     }
-
 }
 
-
 include("head.inc");
 ?>
 
-
 <script type="text/javascript">
 //<![CDATA[
 $( document ).ready(function() {
@@ -176,10 +187,13 @@ $( document ).ready(function() {
 <body>
   <?php
     include("fbegin.inc");
-    if($savemsg)
+    if (isset($input_errors) && count($input_errors) > 0) {
+        print_input_errors($input_errors);
+    }
+    if ($savemsg) {
       print_info_box($savemsg);
+    }
   ?>
-
   <section class="page-content-main">
     <div class="container-fluid">
       <div class="row">
@@ -193,15 +207,18 @@ $( document ).ready(function() {
                   <th colspan="2" valign="top" class="listtopic"><?=gettext("Backup Count"); ?></th>
                 </tr>
                 <tr>
-                  <td><input name="backupcount" type="text" class="formfld unknown" size="5" value="<?=htmlspecialchars($config['system']['backupcount']);?>"/></td>
-                  <td><?= gettext("Enter the number of older configurations to keep in the local backup cache. By default this is 30."); ?></td>
+                  <td><input name="backupcount" type="text" class="formfld unknown" size="5" value="<?=htmlspecialchars($pconfig['backupcount']);?>"/></td>
+                  <td><?= gettext("Enter the number of older configurations to keep in the local backup cache."); ?></td>
                 </tr>
                 <tr>
                   <td>
                     <input name="save" type="submit" class="btn btn-primary" value="<?=gettext("Save"); ?>" />
                   </td>
                   <td>
-                    <?= gettext("Be aware of how much space is consumed by backups before adjusting this value. Current space used by backups: "); ?> <?= exec("/usr/bin/du -sh /conf/backup | /usr/bin/awk '{print $1;}'") ?>
+                    <?= gettext('Be aware of how much space is consumed by backups before adjusting this value.'); ?>
+                    <?php if (count($confvers)) {
+                      print gettext('Current space used:') . ' ' . exec("/usr/bin/du -sh /conf/backup | /usr/bin/awk '{print $1;}'");
+                    } ?>
                   </td>
                 </tr>
               </table>
@@ -245,6 +262,7 @@ $( document ).ready(function() {
             </div>
           </section>
           <?php endif; ?>
+          <?php if (count($confvers)): ?>
           <form method="get">
           <section>
             <div class="content-box">
@@ -290,14 +308,17 @@ $( document ).ready(function() {
                       <td><b><?=gettext("Current");?></b></td>
                     </tr>
 <?php
-                  $i = 0;
+                  $last = count($confvers);
+                  $curr = 1;
                   foreach($confvers as $version):?>
                     <tr>
                       <td>
-                        <input type="radio" name="oldtime" value="<?=$version['time'];?>" <?= (!isset($oldcheck) && $i == 0)  || (isset($oldcheck) && $oldcheck == $version['time']) ? 'checked="checked"' : '' ?>/>
+                        <input type="radio" name="oldtime" value="<?=$version['time'];?>" <?= (!isset($oldcheck) && $curr == 1)  || (isset($oldcheck) && $oldcheck == $version['time']) ? 'checked="checked"' : '' ?>/>
                       </td>
                       <td>
+                        <?php if ($curr != $last): ?>
                         <input type="radio" name="newtime" value="<?=$version['time'];?>" <?= isset($newcheck) && $newcheck == $version['time'] ? 'checked="checked"' : ''?>/>
+                        <?php endif ?>
                       </td>
                       <td> <?= date(gettext("n/j/y H:i:s"), $version['time']) ?></td>
                       <td> <?= $version['version'] ?></td>
@@ -316,7 +337,7 @@ $( document ).ready(function() {
                       </td>
                     </tr>
 <?php
-                  $i++;
+                  $curr++;
                   endforeach;?>
                   </tbody>
                 </table>
@@ -324,6 +345,7 @@ $( document ).ready(function() {
             </div>
           </section>
         </form>
+        <?php endif ?>
       </section>
     </div>
   </div>