From 8cd955bce0d2f7585ce1181fbd09a90ad5788229 2020-07-05 00:32:20
From: Branko Majic <branko@majic.rs>
Date: 2020-07-05 00:32:20
Subject: [PATCH] Noticket: Refactor validation of server settings in Factorio Manager:

- Introduce dedicated validation function.
- Drop some unused variables.
- Validate settings while they are being prepared for the JSON file.

---

diff --git a/games/factorio_manager.sh b/games/factorio_manager.sh
index ac5156c4e9cadae074d43b7c941f085a50294c3a..2c8d3ab8bbfbca3b0a6fde334247e12c1bf8fec6 100755
--- a/games/factorio_manager.sh
+++ b/games/factorio_manager.sh
@@ -385,6 +385,84 @@ function critical_confirmation() {
     fi
 }
 
+#
+# Validates that the specified value conforms to designated setting.
+#
+# This is a small helper function used within read_server_settings
+# function to validate the settings.
+#
+# Arguments:
+#
+#   $1 (name)
+#     Name of the setting. Used to show erros to the user.
+#
+#   $2 (value)
+#     Value to validate.
+#
+#   $3 (type)
+#
+#     Value type. Currently supports the following types:
+#
+#       - bool (boolean)
+#       - int (integer/number)
+#       - str (string, essentially anything can pass this validation)
+#       - list (space-separated list of strings, essentially anything
+#         can pass this validation)
+#       - VAL_1|...|VAL_N (choice between different values)
+#
+#     When using the VAL_1|...|VAL_N variant, validation is slightly
+#     relaxed for string values to allow both quoted and unquoted
+#     variants. For example, if type was set to true|false|"admins",
+#     then both 'admin' and '"admins"' will validate successfully
+#     against the type.
+#
+# Returns:
+#
+#   0 if validation has passed, 1 if validation failed, and 2 if
+#   passed-in type is not supported.
+#
+function validate_server_setting_value() {
+    local name="$1"
+    local value="$2"
+    local type="$3"
+
+    declare -a possible_values
+
+    local i
+
+    # Assume failure.
+    local result=1
+
+    if [[ $type == "bool" ]]; then
+        [[ $value == true || $value == false ]] && result=0 || colorecho "red" "$name must be a boolean [true|false]."
+
+    elif [[ $type == "int" ]]; then
+        [[ $value =~ ^[[:digit:]]+$ ]] && result=0 || colorecho "red" "$name must be a number."
+
+    elif [[ $type == "str" ]]; then
+        result=0
+
+    elif [[ $type == "list" ]]; then
+        # This is free-form space-delimited list.
+        result=0
+
+    elif [[ $type =~ ^.+\|.+$ ]]; then
+        readarray -d "|" -t possible_values < <(echo -n "$type")
+
+        for i in "${possible_values[@]}"; do
+            # Allow strings without quotes to be specified by the user.
+            [[ $value == $i || \"$value\" == $i ]] && result=0
+        done
+
+        [[ $result == 0 ]] || colorecho red "$name must be one of listed values [$type]."
+    else
+        error "Unsupported type associated with '$name': $type"
+        result=2
+    fi
+
+    return "$result"
+}
+
 #
 # Prompts user to provide settings for a server instance.
 #
@@ -432,10 +510,8 @@ function read_server_settings() {
     declare -A settings_prompt=()
     declare -A settings_description=()
     declare -A settings_type=()
-    declare -A option=()
 
     declare -a settings_order=()
-    declare -a possible_options=()
 
     # Global variables set by the function.
     declare -g -A settings_value=()
@@ -695,39 +771,14 @@ function read_server_settings() {
                 colorecho "white" "${settings_description[$key]}" | fold -s
 
                 # Keep prompting user until a valid value is provided.
-                validation_passed=false
-                until [[ $validation_passed == true ]]; do
+                validation_result=""
+                until [[ $validation_result == 0 ]]; do
 
                     read -p "$prompt" -e -i "${settings_value[$key]}" value
 
-                    if [[ ${settings_type[$key]} == "bool" ]]; then
-                        [[ $value == true || $value == false ]] && validation_passed=true || colorecho "red" "Value must be boolean [true|false]."
-
-                    elif [[ ${settings_type[$key]} == "int" ]]; then
-                        [[ $value =~ ^[[:digit:]]+$ ]] && validation_passed=true || colorecho "red" "Value must be a number."
-
-                    elif [[ ${settings_type[$key]} == "str" ]]; then
-                        validation_passed=true
-
-                    elif [[ ${settings_type[$key]} == "list" ]]; then
-                        # This is free-form space-delimited list.
-                        validation_passed=true
-
-                    elif [[ ${settings_type[$key]} =~ ^.+\|.+$ ]]; then
-                        readarray -d "|" -t possible_values < <(echo -n "${settings_type[$key]}")
-
-                        for i in "${possible_values[@]}"; do
-                            [[ $value == $i ]] && validation_passed=true
-
-                            # Convenience for allowing strings without quotes specified by the user.
-                            [[ \"$value\" == $i ]] && value="\"$value\"" && validation_passed=true
-                        done
-
-                        [[ $validation_passed == true ]] || colorecho red "Value must be one of selection [${settings_type[$key]}]"
-                    else
-                        error "Unsupported type: ${settings_type[$key]}"
-                        return 1
-                    fi
+                    validate_server_setting_value "${settings_prompt[$key]}" "$value" "${settings_type[$key]}"
+                    validation_result="$?"
+                    [[ $validation_result == 2 ]] && error "Internal error, type not set correctly for setting: $key." && exit "$ERROR_GENERAL"
                 done
 
                 settings_value[$key]="$value"
@@ -739,12 +790,14 @@ function read_server_settings() {
     done
 
     # Prepare values so they can be used within the JSON file.
-    #
-    # @TODO: Perform same validation as for user input (since script
-    # itself could have a typo).
     for key in "${settings_order[@]}"; do
         debug "Raw value for $key: ${settings_value[$key]}"
 
+        if ! validate_server_setting_value "$key (${settings_prompt[$key]})" "${settings_value[$key]}" "${settings_type[$key]}"; then
+            error "Failed to validate the settings value. This is most likely an internal bug in $program."
+            exit "$ERROR_GENERAL"
+        fi
+
         if [[ ${settings_type[$key]} == "str" ]]; then
             settings_value[$key]="\"${settings_value[$key]}\""