Merge pull request #155 from safing/feature/config-option-validation-func

Add optional validation function to config options

config/get_test.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 
 	"github.com/safing/portbase/log"
@@ -13,7 +14,11 @@ func parseAndReplaceConfig(jsonData string) error {
 		return err
 	}
 
-	return replaceConfig(m)
+	validationErrors := replaceConfig(m)
+	if len(validationErrors) > 0 {
+		return fmt.Errorf("%d errors, first: %w", len(validationErrors), validationErrors[0])
+	}
+	return nil
 }
 
 func parseAndReplaceDefaultConfig(jsonData string) error {
@@ -22,7 +27,11 @@ func parseAndReplaceDefaultConfig(jsonData string) error {
 		return err
 	}
 
-	return replaceDefaultConfig(m)
+	validationErrors := replaceDefaultConfig(m)
+	if len(validationErrors) > 0 {
+		return fmt.Errorf("%d errors, first: %w", len(validationErrors), validationErrors[0])
+	}
+	return nil
 }
 
 func quickRegister(t *testing.T, key string, optType OptionType, defaultValue interface{}) {

config/main.go

@@ -64,7 +64,7 @@ func start() error {
 		return err
 	}
 
-	err = loadConfig()
+	err = loadConfig(false)
 	if err != nil && !os.IsNotExist(err) {
 		return err
 	}

config/option.go

@@ -6,6 +6,7 @@ import (
 	"regexp"
 	"sync"
 
+	"github.com/mitchellh/copystructure"
 	"github.com/tidwall/sjson"
 
 	"github.com/safing/portbase/database/record"
@@ -223,6 +224,10 @@ type Option struct {
 	// ValidationRegex is considered immutable after the option has
 	// been created.
 	ValidationRegex string
+	// ValidationFunc may contain a function to validate more complex values.
+	// The error is returned beyond the scope of this package and may be
+	// displayed to a user.
+	ValidationFunc func(value interface{}) error `json:"-"`
 	// PossibleValues may be set to a slice of values that are allowed
 	// for this configuration setting. Note that PossibleValues makes most
 	// sense when ExternalOptType is set to HintOneOf
@@ -286,6 +291,15 @@ func (option *Option) GetAnnotation(key string) (interface{}, bool) {
 	return val, ok
 }
 
+// copyOrNil returns a copy of the option, or nil if copying failed.
+func (option *Option) copyOrNil() *Option {
+	copied, err := copystructure.Copy(option)
+	if err != nil {
+		return nil
+	}
+	return copied.(*Option) //nolint:forcetypeassert
+}
+
 // Export expors an option to a Record.
 func (option *Option) Export() (record.Record, error) {
 	option.Lock()

config/persistence.go

@@ -2,16 +2,32 @@ package config
 
 import (
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"path"
 	"strings"
+	"sync"
 
 	"github.com/safing/portbase/log"
 )
 
-var configFilePath string
+var (
+	configFilePath string
 
-func loadConfig() error {
+	loadedConfigValidationErrors     []*ValidationError
+	loadedConfigValidationErrorsLock sync.Mutex
+)
+
+// GetLoadedConfigValidationErrors returns the encountered validation errors
+// from the last time loading config from disk.
+func GetLoadedConfigValidationErrors() []*ValidationError {
+	loadedConfigValidationErrorsLock.Lock()
+	defer loadedConfigValidationErrorsLock.Unlock()
+
+	return loadedConfigValidationErrors
+}
+
+func loadConfig(requireValidConfig bool) error {
 	// check if persistence is configured
 	if configFilePath == "" {
 		return nil
@@ -29,7 +45,17 @@ func loadConfig() error {
 		return err
 	}
 
-	return replaceConfig(newValues)
+	validationErrors := replaceConfig(newValues)
+	if requireValidConfig && len(validationErrors) > 0 {
+		return fmt.Errorf("encountered %d validation errors during config loading", len(validationErrors))
+	}
+
+	// Save validation errors.
+	loadedConfigValidationErrorsLock.Lock()
+	defer loadedConfigValidationErrorsLock.Unlock()
+	loadedConfigValidationErrors = validationErrors
+
+	return nil
 }
 
 // saveConfig saves the current configuration to file.

config/registry.go

@@ -92,9 +92,10 @@ func Register(option *Option) error {
 		}
 	}
 
-	option.activeFallbackValue, err = validateValue(option, option.DefaultValue)
+	var vErr *ValidationError
-	if err != nil {
+	option.activeFallbackValue, vErr = validateValue(option, option.DefaultValue)
-		return fmt.Errorf("config: invalid default value: %w", err)
+	if vErr != nil {
+		return fmt.Errorf("config: invalid default value: %w", vErr)
 	}
 
 	optionsLock.Lock()

config/set.go

@@ -2,7 +2,6 @@ package config
 
 import (
 	"errors"
-	"fmt"
 	"sync"
 
 	"github.com/tevino/abool"
@@ -39,9 +38,8 @@ func signalChanges() {
 }
 
 // replaceConfig sets the (prioritized) user defined config.
-func replaceConfig(newValues map[string]interface{}) error {
+func replaceConfig(newValues map[string]interface{}) []*ValidationError {
-	var firstErr error
+	var validationErrors []*ValidationError
-	var errCnt int
 
 	// RLock the options because we are not adding or removing
 	// options from the registration but rather only update the
@@ -59,10 +57,7 @@ func replaceConfig(newValues map[string]interface{}) error {
 			if err == nil {
 				option.activeValue = valueCache
 			} else {
-				errCnt++
+				validationErrors = append(validationErrors, err)
-				if firstErr == nil {
-					firstErr = err
-				}
 			}
 		}
 
@@ -72,20 +67,12 @@ func replaceConfig(newValues map[string]interface{}) error {
 
 	signalChanges()
 
-	if firstErr != nil {
+	return validationErrors
-		if errCnt > 0 {
-			return fmt.Errorf("encountered %d errors, first was: %w", errCnt, firstErr)
-		}
-		return firstErr
-	}
-
-	return nil
 }
 
 // replaceDefaultConfig sets the (fallback) default config.
-func replaceDefaultConfig(newValues map[string]interface{}) error {
+func replaceDefaultConfig(newValues map[string]interface{}) []*ValidationError {
-	var firstErr error
+	var validationErrors []*ValidationError
-	var errCnt int
 
 	// RLock the options because we are not adding or removing
 	// options from the registration but rather only update the
@@ -103,10 +90,7 @@ func replaceDefaultConfig(newValues map[string]interface{}) error {
 			if err == nil {
 				option.activeDefaultValue = valueCache
 			} else {
-				errCnt++
+				validationErrors = append(validationErrors, err)
-				if firstErr == nil {
-					firstErr = err
-				}
 			}
 		}
 		handleOptionUpdate(option, true)
@@ -115,14 +99,7 @@ func replaceDefaultConfig(newValues map[string]interface{}) error {
 
 	signalChanges()
 
-	if firstErr != nil {
+	return validationErrors
-		if errCnt > 0 {
-			return fmt.Errorf("encountered %d errors, first was: %w", errCnt, firstErr)
-		}
-		return firstErr
-	}
-
-	return nil
 }
 
 // SetConfigOption sets a single value in the (prioritized) user defined config.
@@ -140,10 +117,11 @@ func setConfigOption(key string, value interface{}, push bool) (err error) {
 	if value == nil {
 		option.activeValue = nil
 	} else {
-		var valueCache *valueCache
+		valueCache, vErr := validateValue(option, value)
-		valueCache, err = validateValue(option, value)
+		if vErr == nil {
-		if err == nil {
 			option.activeValue = valueCache
+		} else {
+			err = vErr
 		}
 	}
 
@@ -180,10 +158,11 @@ func setDefaultConfigOption(key string, value interface{}, push bool) (err error
 	if value == nil {
 		option.activeDefaultValue = nil
 	} else {
-		var valueCache *valueCache
+		valueCache, vErr := validateValue(option, value)
-		valueCache, err = validateValue(option, value)
+		if vErr == nil {
-		if err == nil {
 			option.activeDefaultValue = valueCache
+		} else {
+			err = vErr
 		}
 	}
 

config/set_test.go

@@ -24,9 +24,9 @@ func TestLayersGetters(t *testing.T) { //nolint:paralleltest
 		t.Fatal(err)
 	}
 
-	err = replaceConfig(mapData)
+	validationErrors := replaceConfig(mapData)
-	if err != nil {
+	if len(validationErrors) > 0 {
-		t.Fatal(err)
+		t.Fatalf("%d errors, first: %s", len(validationErrors), validationErrors[0].Error())
 	}
 
 	// Test missing values

config/validate.go

@@ -61,109 +61,167 @@ func isAllowedPossibleValue(opt *Option, value interface{}) error {
 		}
 	}
 
-	return fmt.Errorf("value is not allowed")
+	return errors.New("value is not allowed")
 }
 
 // validateValue ensures that value matches the expected type of option.
 // It does not create a copy of the value!
-func validateValue(option *Option, value interface{}) (*valueCache, error) { //nolint:gocyclo
+func validateValue(option *Option, value interface{}) (*valueCache, *ValidationError) { //nolint:gocyclo
 	if option.OptType != OptTypeStringArray {
 		if err := isAllowedPossibleValue(option, value); err != nil {
-			return nil, fmt.Errorf("validation of option %s failed for %v: %w", option.Key, value, err)
+			return nil, &ValidationError{
+				Option: option.copyOrNil(),
+				Err:    err,
+			}
 		}
 	}
 
 	reflect.TypeOf(value).ConvertibleTo(reflect.TypeOf(""))
 
+	var validated *valueCache
 	switch v := value.(type) {
 	case string:
 		if option.OptType != OptTypeString {
-			return nil, fmt.Errorf("expected type %s for option %s, got type %T", getTypeName(option.OptType), option.Key, v)
+			return nil, invalid(option, "expected type %s, got type %T", getTypeName(option.OptType), v)
 		}
 		if option.compiledRegex != nil {
 			if !option.compiledRegex.MatchString(v) {
-				return nil, fmt.Errorf("validation of option %s failed: string \"%s\" did not match validation regex for option", option.Key, v)
+				return nil, invalid(option, "did not match validation regex")
 			}
 		}
-		return &valueCache{stringVal: v}, nil
+		validated = &valueCache{stringVal: v}
 	case []interface{}:
 		vConverted := make([]string, len(v))
 		for pos, entry := range v {
 			s, ok := entry.(string)
 			if !ok {
-				return nil, fmt.Errorf("validation of option %s failed: element %+v at index %d is not a string", option.Key, entry, pos)
+				return nil, invalid(option, "entry #%d is not a string", pos+1)
 			}
 			vConverted[pos] = s
 		}
-		// continue to next case
+		// Call validation function again with converted value.
-		return validateValue(option, vConverted)
+		var vErr *ValidationError
+		validated, vErr = validateValue(option, vConverted)
+		if vErr != nil {
+			return nil, vErr
+		}
 	case []string:
 		if option.OptType != OptTypeStringArray {
-			return nil, fmt.Errorf("expected type %s for option %s, got type %T", getTypeName(option.OptType), option.Key, v)
+			return nil, invalid(option, "expected type %s, got type %T", getTypeName(option.OptType), v)
 		}
 		if option.compiledRegex != nil {
 			for pos, entry := range v {
 				if !option.compiledRegex.MatchString(entry) {
-					return nil, fmt.Errorf("validation of option %s failed: string \"%s\" at index %d did not match validation regex", option.Key, entry, pos)
+					return nil, invalid(option, "entry #%d did not match validation regex", pos+1)
 				}
 
 				if err := isAllowedPossibleValue(option, entry); err != nil {
-					return nil, fmt.Errorf("validation of option %s failed: string %q at index %d is not allowed", option.Key, entry, pos)
+					return nil, invalid(option, "entry #%d is not allowed", pos+1)
 				}
 			}
 		}
-		return &valueCache{stringArrayVal: v}, nil
+		validated = &valueCache{stringArrayVal: v}
 	case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, float32, float64:
 		// uint64 is omitted, as it does not fit in a int64
 		if option.OptType != OptTypeInt {
-			return nil, fmt.Errorf("expected type %s for option %s, got type %T", getTypeName(option.OptType), option.Key, v)
+			return nil, invalid(option, "expected type %s, got type %T", getTypeName(option.OptType), v)
 		}
 		if option.compiledRegex != nil {
 			// we need to use %v here so we handle float and int correctly.
 			if !option.compiledRegex.MatchString(fmt.Sprintf("%v", v)) {
-				return nil, fmt.Errorf("validation of option %s failed: number \"%d\" did not match validation regex", option.Key, v)
+				return nil, invalid(option, "did not match validation regex")
 			}
 		}
 		switch v := value.(type) {
 		case int:
-			return &valueCache{intVal: int64(v)}, nil
+			validated = &valueCache{intVal: int64(v)}
 		case int8:
-			return &valueCache{intVal: int64(v)}, nil
+			validated = &valueCache{intVal: int64(v)}
 		case int16:
-			return &valueCache{intVal: int64(v)}, nil
+			validated = &valueCache{intVal: int64(v)}
 		case int32:
-			return &valueCache{intVal: int64(v)}, nil
+			validated = &valueCache{intVal: int64(v)}
 		case int64:
-			return &valueCache{intVal: v}, nil
+			validated = &valueCache{intVal: v}
 		case uint:
-			return &valueCache{intVal: int64(v)}, nil
+			validated = &valueCache{intVal: int64(v)}
 		case uint8:
-			return &valueCache{intVal: int64(v)}, nil
+			validated = &valueCache{intVal: int64(v)}
 		case uint16:
-			return &valueCache{intVal: int64(v)}, nil
+			validated = &valueCache{intVal: int64(v)}
 		case uint32:
-			return &valueCache{intVal: int64(v)}, nil
+			validated = &valueCache{intVal: int64(v)}
 		case float32:
 			// convert if float has no decimals
 			if math.Remainder(float64(v), 1) == 0 {
-				return &valueCache{intVal: int64(v)}, nil
+				validated = &valueCache{intVal: int64(v)}
+			} else {
+				return nil, invalid(option, "failed to convert float32 to int64")
 			}
-			return nil, fmt.Errorf("failed to convert float32 to int64 for option %s, got value %+v", option.Key, v)
 		case float64:
 			// convert if float has no decimals
 			if math.Remainder(v, 1) == 0 {
-				return &valueCache{intVal: int64(v)}, nil
+				validated = &valueCache{intVal: int64(v)}
+			} else {
+				return nil, invalid(option, "failed to convert float64 to int64")
 			}
-			return nil, fmt.Errorf("failed to convert float64 to int64 for option %s, got value %+v", option.Key, v)
 		default:
-			return nil, errors.New("internal error")
+			return nil, invalid(option, "internal error")
 		}
 	case bool:
 		if option.OptType != OptTypeBool {
-			return nil, fmt.Errorf("expected type %s for option %s, got type %T", getTypeName(option.OptType), option.Key, v)
+			return nil, invalid(option, "expected type %s, got type %T", getTypeName(option.OptType), v)
 		}
-		return &valueCache{boolVal: v}, nil
+		validated = &valueCache{boolVal: v}
 	default:
-		return nil, fmt.Errorf("invalid option value type for option %s: %T", option.Key, value)
+		return nil, invalid(option, "invalid option value type: %T", value)
+	}
+
+	// Check if there is an additional function to validate the value.
+	if option.ValidationFunc != nil {
+		var err error
+		switch option.OptType {
+		case optTypeAny:
+			err = errors.New("internal error")
+		case OptTypeString:
+			err = option.ValidationFunc(validated.stringVal)
+		case OptTypeStringArray:
+			err = option.ValidationFunc(validated.stringArrayVal)
+		case OptTypeInt:
+			err = option.ValidationFunc(validated.intVal)
+		case OptTypeBool:
+			err = option.ValidationFunc(validated.boolVal)
+		}
+		if err != nil {
+			return nil, &ValidationError{
+				Option: option.copyOrNil(),
+				Err:    err,
+			}
+		}
+	}
+
+	return validated, nil
+}
+
+// ValidationError error holds details about a config option value validation error.
+type ValidationError struct {
+	Option *Option
+	Err    error
+}
+
+// Error returns the formatted error.
+func (ve *ValidationError) Error() string {
+	return fmt.Sprintf("validation of %s failed: %s", ve.Option.Key, ve.Err)
+}
+
+// Unwrap returns the wrapped error.
+func (ve *ValidationError) Unwrap() error {
+	return ve.Err
+}
+
+func invalid(option *Option, format string, a ...interface{}) *ValidationError {
+	return &ValidationError{
+		Option: option.copyOrNil(),
+		Err:    fmt.Errorf(format, a...),
 	}
 }

config/validity.go

@@ -10,9 +10,11 @@ type ValidityFlag struct {
 }
 
 // NewValidityFlag returns a flag that signifies if the configuration has been changed.
+// It always starts out as invalid. Refresh to start with the current value.
 func NewValidityFlag() *ValidityFlag {
-	vf := &ValidityFlag{}
+	vf := &ValidityFlag{
-	vf.Refresh()
+		flag: abool.New(),
+	}
 	return vf
 }
 

go.mod

@@ -20,6 +20,7 @@ require (
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hashicorp/go-version v1.4.0
+	github.com/mitchellh/copystructure v1.2.0 // indirect
 	github.com/seehuhn/fortuna v1.0.1
 	github.com/shirou/gopsutil v3.21.11+incompatible
 	github.com/stretchr/testify v1.6.1

go.sum

@@ -67,8 +67,12 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
+github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw=
+github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
 github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
+github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ=
+github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
 github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=

notifications/module.go

@@ -1,15 +1,17 @@
 package notifications
 
 import (
+	"fmt"
 	"time"
 
+	"github.com/safing/portbase/config"
 	"github.com/safing/portbase/modules"
 )
 
 var module *modules.Module
 
 func init() {
-	module = modules.Register("notifications", prep, start, nil, "database", "base")
+	module = modules.Register("notifications", prep, start, nil, "database", "config", "base")
 }
 
 func prep() error {
@@ -22,6 +24,43 @@ func start() error {
 		return err
 	}
 
+	showConfigLoadingErrors()
+
 	go module.StartServiceWorker("cleaner", 1*time.Second, cleaner)
 	return nil
 }
+
+func showConfigLoadingErrors() {
+	validationErrors := config.GetLoadedConfigValidationErrors()
+	if len(validationErrors) == 0 {
+		return
+	}
+
+	// Trigger a module error for more awareness.
+	module.Error(
+		"config:validation-errors-on-load",
+		"Invalid Settings",
+		"Some current settings are invalid. Please update them and restart the Portmaster.",
+	)
+
+	// Send one notification per invalid setting.
+	for _, validationError := range config.GetLoadedConfigValidationErrors() {
+		NotifyError(
+			fmt.Sprintf("config:validation-error:%s", validationError.Option.Key),
+			fmt.Sprintf("Invalid Setting for %s", validationError.Option.Name),
+			fmt.Sprintf(`Your current setting for %s is invalid: %s
+
+Please update the setting and restart the Portmaster, until then the default value is used.`,
+				validationError.Option.Name,
+				validationError.Err.Error(),
+			),
+			Action{
+				Text: "Change",
+				Type: ActionTypeOpenSetting,
+				Payload: &ActionTypeOpenSettingPayload{
+					Key: validationError.Option.Key,
+				},
+			},
+		)
+	}
+}