From 48138d32b6acfe8c20dff097537e69a11df02fbf Mon Sep 17 00:00:00 2001
From: Adam Warner <me@adamwarner.co.uk>
Date: Wed, 16 Mar 2022 17:42:01 +0000
Subject: [PATCH 1/5] Adjust addOrEditKeyValPair to optionally take two or
 three arguments (adjust test to suit)

Add a removeKey function with test

update webpage.sh to reference functions in utils.sh (this can likely be abstracted/refactored further)

Signed-off-by: Adam Warner <me@adamwarner.co.uk>
---
 advanced/Scripts/utils.sh   | 50 +++++++++++++++++++++++++++++++------
 advanced/Scripts/webpage.sh | 27 +++++++++-----------
 test/test_any_utils.py      | 19 +++++++++++++-
 3 files changed, 73 insertions(+), 23 deletions(-)

diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh
index 97dca952..a006d43a 100755
--- a/advanced/Scripts/utils.sh
+++ b/advanced/Scripts/utils.sh
@@ -15,7 +15,10 @@
 #  - New functions must have a test added for them in test/test_any_utils.py
 
 #######################
-# Takes three arguments key, value, and file.
+# Takes either
+#   - Three arguments: key, value, and file.
+#   - Two arguments: key, and file
+#
 # Checks the target file for the existence of the key
 #   - If it exists, it changes the value
 #   - If it does not exist, it adds the value
@@ -25,13 +28,46 @@
 #######################
 addOrEditKeyValPair() {
   local key="${1}"
-  local value="${2}"
-  local file="${3}"
-  if grep -q "^${key}=" "${file}"; then
-    sed -i "/^${key}=/c\\${key}=${value}" "${file}"
-  else
-    echo "${key}=${value}" >> "${file}"
+  local value
+  local file
+  
+  # If two arguments have been passed, then the second one is the file - there is no value
+  if [ $# -lt 3 ]; then    
+    file="${2}"
+  else    
+    value="${2}"
+    file="${3}"
   fi
+
+  if [[ "${value}" != "" ]]; then
+    # value has a value, so it is a key pair
+    if grep -q "^${key}=" "${file}"; then
+      # Key already exists in file, modify the value
+      sed -i "/^${key}=/c\\${key}=${value}" "${file}"
+    else
+      # Key does not already exist, add it and it's value
+      echo "${key}=${value}" >> "${file}"
+    fi
+  else
+    # value has no value, so it is just a key. Add it if it does not already exist
+    if ! grep -q "^${key}" "${file}"; then
+      # Key does not exist, add it.
+      echo "${key}" >> "${file}"
+    fi
+  fi
+}
+
+#######################
+# Takes two arguments key, and file.
+# Deletes a key from target file
+#
+# Example usage:
+# removeKey "PIHOLE_DNS_1" "/etc/pihole/setupVars.conf"
+#######################
+removeKey() {
+  local key="${1}"
+  local file="${2}"
+  sed -i "/^${key}/d" "${file}"
 }
 
 #######################
diff --git a/advanced/Scripts/webpage.sh b/advanced/Scripts/webpage.sh
index 0f88c463..14cf5999 100755
--- a/advanced/Scripts/webpage.sh
+++ b/advanced/Scripts/webpage.sh
@@ -26,6 +26,9 @@ readonly PI_HOLE_FILES_DIR="/etc/.pihole"
 PH_TEST="true"
 source "${PI_HOLE_FILES_DIR}/automated install/basic-install.sh"
 
+readonly utilsfile="/opt/pihole/utils.sh"
+source "${utilsfile}"
+
 coltable="/opt/pihole/COL_TABLE"
 if [[ -f ${coltable} ]]; then
     source ${coltable}
@@ -51,41 +54,35 @@ Options:
 }
 
 add_setting() {
-    echo "${1}=${2}" >> "${setupVars}"
+    addOrEditKeyValPair "${1}" "${2}" "${setupVars}"
 }
 
 delete_setting() {
-    sed -i "/^${1}/d" "${setupVars}"
+    removeKey "${1}" "${setupVars}"
 }
 
 change_setting() {
-    delete_setting "${1}"
-    add_setting "${1}" "${2}"
+    addOrEditKeyValPair "${1}" "${2}" "${setupVars}"
 }
 
 addFTLsetting() {
-    echo "${1}=${2}" >> "${FTLconf}"
+    addOrEditKeyValPair "${1}" "${2}" "${FTLconf}"
 }
 
 deleteFTLsetting() {
-    sed -i "/^${1}/d" "${FTLconf}"
+    removeKey "${1}" "${FTLconf}"
 }
 
 changeFTLsetting() {
-    deleteFTLsetting "${1}"
-    addFTLsetting "${1}" "${2}"
+    addOrEditKeyValPair "${1}" "${2}" "${FTLconf}"
 }
 
 add_dnsmasq_setting() {
-    if [[ "${2}" != "" ]]; then
-        echo "${1}=${2}" >> "${dnsmasqconfig}"
-    else
-        echo "${1}" >> "${dnsmasqconfig}"
-    fi
+    addOrEditKeyValPair "${1}" "${2}" "${dnsmasqconfig}"
 }
 
 delete_dnsmasq_setting() {
-    sed -i "/^${1}/d" "${dnsmasqconfig}"
+    removeKey "${1}" "${dnsmasqconfig}"
 }
 
 SetTemperatureUnit() {
@@ -183,7 +180,7 @@ ProcessDNSSettings() {
     fi
 
     delete_dnsmasq_setting "dnssec"
-    delete_dnsmasq_setting "trust-anchor="
+    delete_dnsmasq_setting "trust-anchor"
 
     if [[ "${DNSSEC}" == true ]]; then
         echo "dnssec
diff --git a/test/test_any_utils.py b/test/test_any_utils.py
index 8ad27997..f73cc1b2 100644
--- a/test/test_any_utils.py
+++ b/test/test_any_utils.py
@@ -6,11 +6,28 @@ def test_key_val_replacement_works(host):
     addOrEditKeyValPair "KEY_TWO" "value2" "./testoutput"
     addOrEditKeyValPair "KEY_ONE" "value3" "./testoutput"
     addOrEditKeyValPair "KEY_FOUR" "value4" "./testoutput"
+    addOrEditKeyValPair "KEY_FIVE_NO_VALUE" "./testoutput"
+    addOrEditKeyValPair "KEY_FIVE_NO_VALUE" "./testoutput"
     ''')
     output = host.run('''
     cat ./testoutput
     ''')
-    expected_stdout = 'KEY_ONE=value3\nKEY_TWO=value2\nKEY_FOUR=value4\n'
+    expected_stdout = 'KEY_ONE=value3\nKEY_TWO=value2\nKEY_FOUR=value4\nKEY_FIVE_NO_VALUE\n'
+    assert expected_stdout == output.stdout
+
+def test_key_val_removal_works(host):
+    ''' Confirms addOrEditKeyValPair provides the expected output '''
+    host.run('''
+    source /opt/pihole/utils.sh
+    addOrEditKeyValPair "KEY_ONE" "value1" "./testoutput"
+    addOrEditKeyValPair "KEY_TWO" "value2" "./testoutput"
+    addOrEditKeyValPair "KEY_THREE" "value3" "./testoutput"
+    removeKey "KEY_TWO" "./testoutput"
+    ''')
+    output = host.run('''
+    cat ./testoutput
+    ''')
+    expected_stdout = 'KEY_ONE=value1\nKEY_THREE=value3\n'
     assert expected_stdout == output.stdout
 
 

From 59fc3804be28b3b26f3c6b333a36e04701be18d9 Mon Sep 17 00:00:00 2001
From: Adam Warner <me@adamwarner.co.uk>
Date: Wed, 16 Mar 2022 20:30:31 +0000
Subject: [PATCH 2/5] Make utils.sh posix compatible per request

Signed-off-by: Adam Warner <me@adamwarner.co.uk>
---
 advanced/Scripts/utils.sh | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh
index a006d43a..86a7e0b4 100755
--- a/advanced/Scripts/utils.sh
+++ b/advanced/Scripts/utils.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/usr/bin/env sh
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2017 Pi-hole, LLC (https://pi-hole.net)
 # Network-wide ad blocking via your own hardware.
@@ -39,7 +39,7 @@ addOrEditKeyValPair() {
     file="${3}"
   fi
 
-  if [[ "${value}" != "" ]]; then
+  if [ "${value}" != "" ]; then
     # value has a value, so it is a key pair
     if grep -q "^${key}=" "${file}"; then
       # Key already exists in file, modify the value
@@ -74,23 +74,23 @@ removeKey() {
 # returns FTL's current telnet API port
 #######################
 getFTLAPIPort(){
-  local -r FTLCONFFILE="/etc/pihole/pihole-FTL.conf"
-  local -r DEFAULT_PORT_FILE="/run/pihole-FTL.port"
-  local -r DEFAULT_FTL_PORT=4711
+  local FTLCONFFILE="/etc/pihole/pihole-FTL.conf"
+  local DEFAULT_PORT_FILE="/run/pihole-FTL.port"
+  local DEFAULT_FTL_PORT=4711
   local PORTFILE
   local ftl_api_port
 
-  if [[ -f "$FTLCONFFILE" ]]; then
+  if [ -f "$FTLCONFFILE" ]; then
     # if PORTFILE is not set in pihole-FTL.conf, use the default path
     PORTFILE="$( (grep "^PORTFILE=" $FTLCONFFILE || echo "$DEFAULT_PORT_FILE") | cut -d"=" -f2-)"
   fi
 
-  if [[ -s "$PORTFILE" ]]; then
+  if [ -s "$PORTFILE" ]; then
     # -s: FILE exists and has a size greater than zero
-    ftl_api_port=$(<"$PORTFILE")
+    ftl_api_port=$(cat "${PORTFILE}")
     # Exploit prevention: unset the variable if there is malicious content
-    # Verify that the value read from the file is numeric
-    [[ "$ftl_api_port" =~ [^[:digit:]] ]] && unset ftl_api_port
+    # Verify that the value read from the file is numeric    
+    expr "$ftl_api_port" : "[^[:digit:]]" > /dev/null && unset ftl_api_port
   fi
 
   # echo the port found in the portfile or default to the default port

From 7fa8cdd03ee1884b61add34d923d0741da8a6a3a Mon Sep 17 00:00:00 2001
From: Adam Warner <me@adamwarner.co.uk>
Date: Wed, 16 Mar 2022 20:46:15 +0000
Subject: [PATCH 3/5] Address:  - Review Comments  - Stickler Complaints

---
 advanced/Scripts/utils.sh   | 22 ++++++++--------------
 advanced/Scripts/webpage.sh | 12 +++++++-----
 pihole                      |  8 ++++----
 test/test_any_utils.py      | 19 ++++++++++---------
 4 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh
index 86a7e0b4..0906ce49 100755
--- a/advanced/Scripts/utils.sh
+++ b/advanced/Scripts/utils.sh
@@ -1,4 +1,6 @@
 #!/usr/bin/env sh
+# shellcheck disable=SC3043 #https://github.com/koalaman/shellcheck/wiki/SC3043#exceptions
+
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2017 Pi-hole, LLC (https://pi-hole.net)
 # Network-wide ad blocking via your own hardware.
@@ -16,28 +18,20 @@
 
 #######################
 # Takes either
-#   - Three arguments: key, value, and file.
-#   - Two arguments: key, and file
+#   - Three arguments: file, key, and value.
+#   - Two arguments: file, and key.
 #
 # Checks the target file for the existence of the key
 #   - If it exists, it changes the value
 #   - If it does not exist, it adds the value
 #
 # Example usage:
-# addOrEditKeyValuePair "BLOCKING_ENABLED" "true" "/etc/pihole/setupVars.conf"
+# addOrEditKeyValuePair "/etc/pihole/setupVars.conf" "BLOCKING_ENABLED" "true"
 #######################
 addOrEditKeyValPair() {
-  local key="${1}"
-  local value
-  local file
-  
-  # If two arguments have been passed, then the second one is the file - there is no value
-  if [ $# -lt 3 ]; then    
-    file="${2}"
-  else    
-    value="${2}"
-    file="${3}"
-  fi
+  local file="${1}"
+  local key="${2}"
+  local value="${3}"
 
   if [ "${value}" != "" ]; then
     # value has a value, so it is a key pair
diff --git a/advanced/Scripts/webpage.sh b/advanced/Scripts/webpage.sh
index 14cf5999..f63fd0ca 100755
--- a/advanced/Scripts/webpage.sh
+++ b/advanced/Scripts/webpage.sh
@@ -1,5 +1,7 @@
 #!/usr/bin/env bash
 # shellcheck disable=SC1090
+# shellcheck disable=SC2154
+
 
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2017 Pi-hole, LLC (https://pi-hole.net)
@@ -54,7 +56,7 @@ Options:
 }
 
 add_setting() {
-    addOrEditKeyValPair "${1}" "${2}" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "${1}" "${2}"
 }
 
 delete_setting() {
@@ -62,11 +64,11 @@ delete_setting() {
 }
 
 change_setting() {
-    addOrEditKeyValPair "${1}" "${2}" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "${1}" "${2}"
 }
 
 addFTLsetting() {
-    addOrEditKeyValPair "${1}" "${2}" "${FTLconf}"
+    addOrEditKeyValPair "${FTLconf}" "${1}" "${2}"
 }
 
 deleteFTLsetting() {
@@ -74,11 +76,11 @@ deleteFTLsetting() {
 }
 
 changeFTLsetting() {
-    addOrEditKeyValPair "${1}" "${2}" "${FTLconf}"
+    addOrEditKeyValPair "${FTLconf}" "${1}" "${2}"
 }
 
 add_dnsmasq_setting() {
-    addOrEditKeyValPair "${1}" "${2}" "${dnsmasqconfig}"
+    addOrEditKeyValPair "${dnsmasqconfig}" "${1}" "${2}"
 }
 
 delete_dnsmasq_setting() {
diff --git a/pihole b/pihole
index d73fd5aa..bdce6663 100755
--- a/pihole
+++ b/pihole
@@ -226,7 +226,7 @@ Time:
       fi
 
       local str="Pi-hole Disabled"
-      addOrEditKeyValPair "BLOCKING_ENABLED" "false" "${setupVars}"
+      addOrEditKeyValPair "${setupVars}" "BLOCKING_ENABLED" "false"
     fi
   else
     # Enable Pi-hole
@@ -238,7 +238,7 @@ Time:
     echo -e "  ${INFO} Enabling blocking"
     local str="Pi-hole Enabled"
 
-    addOrEditKeyValPair "BLOCKING_ENABLED" "true" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "BLOCKING_ENABLED" "true"
   fi
 
   restartDNS reload-lists
@@ -261,7 +261,7 @@ Options:
   elif [[ "${1}" == "off" ]]; then
     # Disable logging
     sed -i 's/^log-queries/#log-queries/' /etc/dnsmasq.d/01-pihole.conf
-    addOrEditKeyValPair "QUERY_LOGGING" "false" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "false"
     if [[ "${2}" != "noflush" ]]; then
       # Flush logs
       "${PI_HOLE_BIN_DIR}"/pihole -f
@@ -271,7 +271,7 @@ Options:
   elif [[ "${1}" == "on" ]]; then
     # Enable logging
     sed -i 's/^#log-queries/log-queries/' /etc/dnsmasq.d/01-pihole.conf
-    addOrEditKeyValPair "QUERY_LOGGING" "true" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "true"
     echo -e "  ${INFO} Enabling logging..."
     local str="Logging has been enabled!"
   else
diff --git a/test/test_any_utils.py b/test/test_any_utils.py
index f73cc1b2..1c8f9531 100644
--- a/test/test_any_utils.py
+++ b/test/test_any_utils.py
@@ -2,12 +2,12 @@ def test_key_val_replacement_works(host):
     ''' Confirms addOrEditKeyValPair provides the expected output '''
     host.run('''
     source /opt/pihole/utils.sh
-    addOrEditKeyValPair "KEY_ONE" "value1" "./testoutput"
-    addOrEditKeyValPair "KEY_TWO" "value2" "./testoutput"
-    addOrEditKeyValPair "KEY_ONE" "value3" "./testoutput"
-    addOrEditKeyValPair "KEY_FOUR" "value4" "./testoutput"
-    addOrEditKeyValPair "KEY_FIVE_NO_VALUE" "./testoutput"
-    addOrEditKeyValPair "KEY_FIVE_NO_VALUE" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_ONE" "value1"
+    addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2"
+    addOrEditKeyValPair "./testoutput" "KEY_ONE" "value3"
+    addOrEditKeyValPair "./testoutput" "KEY_FOUR" "value4"
+    addOrEditKeyValPair "./testoutput" "KEY_FIVE_NO_VALUE"
+    addOrEditKeyValPair "./testoutput" "KEY_FIVE_NO_VALUE"
     ''')
     output = host.run('''
     cat ./testoutput
@@ -15,13 +15,14 @@ def test_key_val_replacement_works(host):
     expected_stdout = 'KEY_ONE=value3\nKEY_TWO=value2\nKEY_FOUR=value4\nKEY_FIVE_NO_VALUE\n'
     assert expected_stdout == output.stdout
 
+
 def test_key_val_removal_works(host):
     ''' Confirms addOrEditKeyValPair provides the expected output '''
     host.run('''
     source /opt/pihole/utils.sh
-    addOrEditKeyValPair "KEY_ONE" "value1" "./testoutput"
-    addOrEditKeyValPair "KEY_TWO" "value2" "./testoutput"
-    addOrEditKeyValPair "KEY_THREE" "value3" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_ONE" "value1"
+    addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2"
+    addOrEditKeyValPair "./testoutput" "KEY_THREE" "value3"
     removeKey "KEY_TWO" "./testoutput"
     ''')
     output = host.run('''

From 4d31d5ee1148f1de8e8608a7bf0fed255136a6e1 Mon Sep 17 00:00:00 2001
From: Adam Warner <me@adamwarner.co.uk>
Date: Mon, 4 Apr 2022 22:02:26 +0100
Subject: [PATCH 4/5] Address review comments

Signed-off-by: Adam Warner <me@adamwarner.co.uk>
---
 advanced/Scripts/utils.sh   | 6 +++---
 advanced/Scripts/webpage.sh | 6 +++---
 pihole                      | 4 ++--
 test/test_any_utils.py      | 4 ++--
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh
index 0906ce49..9d80e446 100755
--- a/advanced/Scripts/utils.sh
+++ b/advanced/Scripts/utils.sh
@@ -56,11 +56,11 @@ addOrEditKeyValPair() {
 # Deletes a key from target file
 #
 # Example usage:
-# removeKey "PIHOLE_DNS_1" "/etc/pihole/setupVars.conf"
+# removeKey "/etc/pihole/setupVars.conf" "PIHOLE_DNS_1"
 #######################
 removeKey() {
-  local key="${1}"
-  local file="${2}"
+  local file="${1}"
+  local key="${2}"
   sed -i "/^${key}/d" "${file}"
 }
 
diff --git a/advanced/Scripts/webpage.sh b/advanced/Scripts/webpage.sh
index f63fd0ca..15418ee0 100755
--- a/advanced/Scripts/webpage.sh
+++ b/advanced/Scripts/webpage.sh
@@ -60,7 +60,7 @@ add_setting() {
 }
 
 delete_setting() {
-    removeKey "${1}" "${setupVars}"
+    removeKey "${setupVars}" "${1}"
 }
 
 change_setting() {
@@ -72,7 +72,7 @@ addFTLsetting() {
 }
 
 deleteFTLsetting() {
-    removeKey "${1}" "${FTLconf}"
+    removeKey "${FTLconf}" "${1}"
 }
 
 changeFTLsetting() {
@@ -84,7 +84,7 @@ add_dnsmasq_setting() {
 }
 
 delete_dnsmasq_setting() {
-    removeKey "${1}" "${dnsmasqconfig}"
+    removeKey "${dnsmasqconfig}" "${1}"
 }
 
 SetTemperatureUnit() {
diff --git a/pihole b/pihole
index bdce6663..83d1f45c 100755
--- a/pihole
+++ b/pihole
@@ -260,7 +260,7 @@ Options:
     exit 0
   elif [[ "${1}" == "off" ]]; then
     # Disable logging
-    sed -i 's/^log-queries/#log-queries/' /etc/dnsmasq.d/01-pihole.conf
+    addOrEditKeyValPair /etc/dnsmasq.d/01-pihole.conf "log-queries"
     addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "false"
     if [[ "${2}" != "noflush" ]]; then
       # Flush logs
@@ -270,7 +270,7 @@ Options:
     local str="Logging has been disabled!"
   elif [[ "${1}" == "on" ]]; then
     # Enable logging
-    sed -i 's/^#log-queries/log-queries/' /etc/dnsmasq.d/01-pihole.conf
+    removeKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
     addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "true"
     echo -e "  ${INFO} Enabling logging..."
     local str="Logging has been enabled!"
diff --git a/test/test_any_utils.py b/test/test_any_utils.py
index 1c8f9531..998c1c84 100644
--- a/test/test_any_utils.py
+++ b/test/test_any_utils.py
@@ -17,13 +17,13 @@ def test_key_val_replacement_works(host):
 
 
 def test_key_val_removal_works(host):
-    ''' Confirms addOrEditKeyValPair provides the expected output '''
+    ''' Confirms removeKey provides the expected output '''
     host.run('''
     source /opt/pihole/utils.sh
     addOrEditKeyValPair "./testoutput" "KEY_ONE" "value1"
     addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2"
     addOrEditKeyValPair "./testoutput" "KEY_THREE" "value3"
-    removeKey "KEY_TWO" "./testoutput"
+    removeKey "./testoutput" "KEY_TWO"
     ''')
     output = host.run('''
     cat ./testoutput

From 9b4f6c84cd770d333bca1579a8494472bfe5fa62 Mon Sep 17 00:00:00 2001
From: yubiuser <ckoenig@posteo.de>
Date: Mon, 4 Apr 2022 23:14:10 +0200
Subject: [PATCH 5/5] Minor review comments

---
 advanced/Scripts/utils.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh
index 9d80e446..f457427f 100755
--- a/advanced/Scripts/utils.sh
+++ b/advanced/Scripts/utils.sh
@@ -34,7 +34,7 @@ addOrEditKeyValPair() {
   local value="${3}"
 
   if [ "${value}" != "" ]; then
-    # value has a value, so it is a key pair
+    # value has a value, so it is a key-value pair
     if grep -q "^${key}=" "${file}"; then
       # Key already exists in file, modify the value
       sed -i "/^${key}=/c\\${key}=${value}" "${file}"
@@ -52,7 +52,7 @@ addOrEditKeyValPair() {
 }
 
 #######################
-# Takes two arguments key, and file.
+# Takes two arguments file, and key.
 # Deletes a key from target file
 #
 # Example usage: