From 1dd421568258fcd5f4fad7f438fd8d02e80751aa Mon Sep 17 00:00:00 2001
From: Franco Fichtner <franco@opnsense.org>
Date: Thu, 2 Mar 2023 09:01:55 +0100
Subject: [PATCH] dnsmasq: tweak the settings additions

cache size and ttl support zero value, which was ignored by the input.

Derive help text from the manual page: https://thekelleys.org.uk/dnsmasq/docs/dnsmasq-man.html

Avoid validation during config write to not mask issues with the code.
---
 src/www/services_dnsmasq.php | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/www/services_dnsmasq.php b/src/www/services_dnsmasq.php
index 47f258f9a..058cd207c 100644
--- a/src/www/services_dnsmasq.php
+++ b/src/www/services_dnsmasq.php
@@ -53,8 +53,8 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
     // simple text types
     $pconfig['port'] = !empty($a_dnsmasq['port']) ? $a_dnsmasq['port'] : "";
     $pconfig['dns_forward_max'] = !empty($a_dnsmasq['dns_forward_max']) ? $a_dnsmasq['dns_forward_max'] : "";
-    $pconfig['cache_size'] = !empty($a_dnsmasq['cache_size']) ? $a_dnsmasq['cache_size'] : "";
-    $pconfig['local_ttl'] = !empty($a_dnsmasq['local_ttl']) ? $a_dnsmasq['local_ttl'] : "";
+    $pconfig['cache_size'] = isset($a_dnsmasq['cache_size']) ? $a_dnsmasq['cache_size'] : '';
+    $pconfig['local_ttl'] = isset($a_dnsmasq['local_ttl']) ? $a_dnsmasq['local_ttl'] : '';
     // arrays
     $pconfig['interface'] = !empty($a_dnsmasq['interface']) ? explode(",", $a_dnsmasq['interface']) : array();
 
@@ -72,10 +72,10 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
         if (!empty($pconfig['dns_forward_max']) && !is_numericint($pconfig['dns_forward_max'])) {
             $input_errors[] = gettext("You must specify a valid maximum number of DNS queries");
         }
-        if (!empty($pconfig['cache_size']) && !is_numericint($pconfig['cache_size'])) {
+        if (isset($pconfig['cache_size']) && $pconfig['cache_size'] !== '' && !is_numericint($pconfig['cache_size'])) {
             $input_errors[] = gettext("You must specify a valid cache size");
         }
-        if (!empty($pconfig['local_ttl']) && !is_numericint($pconfig['local_ttl'])) {
+        if (isset($pconfig['local_ttl']) && $pconfig['local_ttl'] !== '' && !is_numericint($pconfig['local_ttl'])) {
             $input_errors[] = gettext("You must specify a valid TTL for local DNS");
         }
         $unbound_port = empty($config['unbound']['port']) ? "53" : $config['unbound']['port'];
@@ -117,12 +117,12 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
             } elseif (isset($a_dnsmasq['dns_forward_max'])) {
                 unset($a_dnsmasq['dns_forward_max']);
             }
-            if (!empty($pconfig['cache_size'])) {
+            if (isset($pconfig['cache_size']) && $pconfig['cache_size'] !== '') {
                 $a_dnsmasq['cache_size'] = $pconfig['cache_size'];
             } elseif (isset($a_dnsmasq['cache_size'])) {
                 unset($a_dnsmasq['cache_size']);
             }
-            if (!empty($pconfig['local_ttl'])) {
+            if (isset($pconfig['local_ttl'])&& $pconfig['local_ttl'] !== '') {
                 $a_dnsmasq['local_ttl'] = $pconfig['local_ttl'];
             } elseif (isset($a_dnsmasq['local_ttl'])) {
                 unset($a_dnsmasq['local_ttl']);
@@ -392,7 +392,7 @@ $( document ).ready(function() {
                   <td><i class="fa fa-info-circle text-muted"></i> <?=gettext('No Hosts Lookup') ?></td>
                   <td>
                     <input name="no_hosts" type="checkbox" id="no_hosts" value="yes" <?= !empty($pconfig['no_hosts']) ? 'checked="checked"' : '' ?> />
-                    <?= gettext('Don\'t read the hostnames in /etc/hosts.') ?>
+                    <?= gettext('Do not read hostnames in /etc/hosts') ?>
                   </td>
                 </tr>
                 <tr>
@@ -403,20 +403,20 @@ $( document ).ready(function() {
                   </td>
                 </tr>
                 <tr>
-                  <td><a id="help_for_dns_forward_max" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("Max number DNS queries");?></td>
+                  <td><a id="help_for_dns_forward_max" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?= gettext('Maximum concurrent queries') ?></td>
                   <td>
-                    <input name="dns_forward_max" type="text" id="dns_forward_max" size="6" placeholder="5000" <?=!empty($pconfig['dns_forward_max']) ? "value=\"{$pconfig['dns_forward_max']}\"" : "";?> />
+                    <input name="dns_forward_max" type="text" id="dns_forward_max" size="6" placeholder="5000" <?= !empty($pconfig['dns_forward_max']) ? 'value="' . html_safe($pconfig['dns_forward_max']) . '"' : '' ?> />
                     <div class="hidden" data-for="help_for_dns_forward_max">
-                      <?=gettext("The maximum number of forwarded DNS queries");?>
+                      <?= gettext('Set the maximum number of concurrent DNS queries. On configurations with tight resources, this value may need to be reduced.') ?>
                     </div>
                   </td>
                 </tr>
                 <tr>
-                  <td><a id="help_for_cache_size" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("Cache size");?></td>
+                  <td><a id="help_for_cache_size" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?= gettext('Cache size') ?></td>
                   <td>
-                    <input name="cache_size" type="text" id="cache_size" size="8" placeholder="10000" <?=!empty($pconfig['cache_size']) ? "value=\"{$pconfig['cache_size']}\"" : "";?> />
+                    <input name="cache_size" type="text" id="cache_size" size="8" placeholder="10000" value="<?= html_safe($pconfig['cache_size']) ?>" />
                     <div class="hidden" data-for="help_for_cache_size">
-                      <?=gettext("The number of entries in DNS cache");?>
+                      <?= gettext('Set the size of the cache. Setting the cache size to zero disables caching. Please note that huge cache size impacts performance.') ?>
                     </div>
                   </td>
                 </tr>
@@ -425,7 +425,7 @@ $( document ).ready(function() {
                   <td>
                     <input name="local_ttl" type="text" id="local_ttl" size="5" placeholder="1" <?=!empty($pconfig['local_ttl']) ? "value=\"{$pconfig['local_ttl']}\"" : "";?> />
                     <div class="hidden" data-for="help_for_local_ttl">
-                      <?=gettext("The Time-To-Live in seconds for local DNS entries, i.e. /etc/hosts or DHCP leases");?>
+                      <?=gettext("This option allows a time-to-live (in seconds) to be given for local DNS entries, i.e. /etc/hosts or DHCP leases. This will reduce the load on the server at the expense of clients using stale data under some circumstances. A value of zero will disable disable client-side caching.");?>
                     </div>
                   </td>
                 </tr>