From 34453d69893b9d1115d5403acd17d3dd5ebcd7fe Mon Sep 17 00:00:00 2001 From: Patrick Pacher Date: Mon, 5 Sep 2022 15:02:12 +0200 Subject: [PATCH 1/3] Fix handling of unmapped configuration options --- config/registry.go | 26 ++++++++++++++++++++++++++ config/set.go | 14 +++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/config/registry.go b/config/registry.go index 04b0dbf6..01a7d722 100644 --- a/config/registry.go +++ b/config/registry.go @@ -11,6 +11,12 @@ import ( var ( optionsLock sync.RWMutex options = make(map[string]*Option) + + // unmappedValues holds a list of configuration values that have been + // read from the persistence layer but no option has been defined yet. + // This is mainly to support the plugin system of the Portmaster. + unmappedValuesLock sync.Mutex + unmappedValues map[string]interface{} ) // ForEachOption calls fn for each defined option. If fn returns @@ -98,9 +104,29 @@ func Register(option *Option) error { return fmt.Errorf("config: invalid default value: %w", vErr) } + if err := loadUnmappedValue(option); err != nil { + return err + } + optionsLock.Lock() defer optionsLock.Unlock() options[option.Key] = option return nil } + +func loadUnmappedValue(option *Option) error { + unmappedValuesLock.Lock() + defer unmappedValuesLock.Unlock() + if value, ok := unmappedValues[option.Key]; ok { + delete(unmappedValues, option.Key) + + var vErr *ValidationError + option.activeValue, vErr = validateValue(option, value) + if vErr != nil { + return fmt.Errorf("config: invalid value: %w", vErr) + } + } + + return nil +} diff --git a/config/set.go b/config/set.go index dfa2f15c..25f01769 100644 --- a/config/set.go +++ b/config/set.go @@ -41,6 +41,11 @@ func signalChanges() { func replaceConfig(newValues map[string]interface{}) []*ValidationError { var validationErrors []*ValidationError + valuesCopy := make(map[string]interface{}, len(newValues)) + for key, value := range newValues { + valuesCopy[key] = value + } + // RLock the options because we are not adding or removing // options from the registration but rather only update the // options value which is guarded by the option's lock itself @@ -48,7 +53,9 @@ func replaceConfig(newValues map[string]interface{}) []*ValidationError { defer optionsLock.RUnlock() for key, option := range options { - newValue, ok := newValues[key] + newValue, ok := valuesCopy[key] + + delete(valuesCopy, key) option.Lock() option.activeValue = nil @@ -66,6 +73,11 @@ func replaceConfig(newValues map[string]interface{}) []*ValidationError { option.Unlock() } + unmappedValuesLock.Lock() + defer unmappedValuesLock.Unlock() + + unmappedValues = valuesCopy + signalChanges() return validationErrors From f423570475f1f30ab365a56347ea54428e785adb Mon Sep 17 00:00:00 2001 From: Patrick Pacher Date: Tue, 23 May 2023 10:56:32 +0200 Subject: [PATCH 2/3] Allow errors from loadUnmappedValue to be "soft" errors. Signal config changes when loading unmapped values --- config/registry.go | 26 ++++++++++++++++++++------ config/validate.go | 5 +++-- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/config/registry.go b/config/registry.go index 01a7d722..e0128ad8 100644 --- a/config/registry.go +++ b/config/registry.go @@ -104,29 +104,43 @@ func Register(option *Option) error { return fmt.Errorf("config: invalid default value: %w", vErr) } - if err := loadUnmappedValue(option); err != nil { - return err + hasUnmappedValue, vErr := loadUnmappedValue(option) + if vErr != nil && !vErr.SoftError { + return fmt.Errorf("config: invalid value: %w", vErr) } optionsLock.Lock() defer optionsLock.Unlock() options[option.Key] = option - return nil + if hasUnmappedValue { + signalChanges() + } + + // return the validation-error from loadUnmappedValue here + return vErr } -func loadUnmappedValue(option *Option) error { +func loadUnmappedValue(option *Option) (bool, *ValidationError) { unmappedValuesLock.Lock() defer unmappedValuesLock.Unlock() + if value, ok := unmappedValues[option.Key]; ok { delete(unmappedValues, option.Key) var vErr *ValidationError option.activeValue, vErr = validateValue(option, value) if vErr != nil { - return fmt.Errorf("config: invalid value: %w", vErr) + // we consider this error as a "soft" error so lazily registered + // options don't fail the hard way. + option.activeValue = option.activeFallbackValue + vErr.SoftError = true + + return true, vErr } + + return true, nil } - return nil + return false, nil } diff --git a/config/validate.go b/config/validate.go index 4120d2e2..67d73525 100644 --- a/config/validate.go +++ b/config/validate.go @@ -205,8 +205,9 @@ func validateValue(option *Option, value interface{}) (*valueCache, *ValidationE // ValidationError error holds details about a config option value validation error. type ValidationError struct { - Option *Option - Err error + Option *Option + Err error + SoftError bool } // Error returns the formatted error. From f67ad49a7b91519abf0de1c33d75366996671bb3 Mon Sep 17 00:00:00 2001 From: Patrick Pacher Date: Tue, 23 May 2023 10:58:50 +0200 Subject: [PATCH 3/3] Fix panic when returning a nil ValidationError --- config/registry.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/config/registry.go b/config/registry.go index e0128ad8..55a44abc 100644 --- a/config/registry.go +++ b/config/registry.go @@ -118,7 +118,11 @@ func Register(option *Option) error { } // return the validation-error from loadUnmappedValue here - return vErr + if vErr != nil { + return vErr + } + + return nil } func loadUnmappedValue(option *Option) (bool, *ValidationError) {