From f976451d19aea6db42d08ff8cf6e760176f94acb Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Wed, 22 May 2019 14:32:12 -0600 Subject: [PATCH] Disallow unknown fields (strict unmarshal) when loading modules This makes it faster and easier to detect broken configurations, but is a slight performance hit on config loads since we have to re-encode the decoded struct back into JSON without the module name's key --- context.go | 4 ++-- modules.go | 32 +++++++++++++++++++++++++++----- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/context.go b/context.go index bdf9f605..dab603e8 100644 --- a/context.go +++ b/context.go @@ -84,7 +84,7 @@ func (ctx Context) LoadModule(name string, rawMsg json.RawMessage) (interface{}, // fill in its config only if there is a config to fill in if len(rawMsg) > 0 { - err := json.Unmarshal(rawMsg, &val) + err := strictUnmarshalJSON(rawMsg, &val) if err != nil { return nil, fmt.Errorf("decoding module config: %s: %v", mod.Name, err) } @@ -127,7 +127,7 @@ func (ctx Context) LoadModule(name string, rawMsg json.RawMessage) (interface{}, // containing the module name is treated special/separate from all // the other keys. func (ctx Context) LoadModuleInline(moduleNameKey, moduleScope string, raw json.RawMessage) (interface{}, error) { - moduleName, err := getModuleNameInline(moduleNameKey, raw) + moduleName, raw, err := getModuleNameInline(moduleNameKey, raw) if err != nil { return nil, err } diff --git a/modules.go b/modules.go index 19d203da..9ee38358 100644 --- a/modules.go +++ b/modules.go @@ -1,6 +1,7 @@ package caddy2 import ( + "bytes" "encoding/json" "fmt" "sort" @@ -120,20 +121,31 @@ func Modules() []string { } // getModuleNameInline loads the string value from raw of moduleNameKey, -// where raw must be a JSON encoding of a map. -func getModuleNameInline(moduleNameKey string, raw json.RawMessage) (string, error) { +// where raw must be a JSON encoding of a map. It returns that value, +// along with the result of removing that key from raw. +func getModuleNameInline(moduleNameKey string, raw json.RawMessage) (string, json.RawMessage, error) { var tmp map[string]interface{} err := json.Unmarshal(raw, &tmp) if err != nil { - return "", err + return "", nil, err } moduleName, ok := tmp[moduleNameKey].(string) if !ok || moduleName == "" { - return "", fmt.Errorf("module name not specified with key '%s' in %+v", moduleNameKey, tmp) + return "", nil, fmt.Errorf("module name not specified with key '%s' in %+v", moduleNameKey, tmp) } - return moduleName, nil + // remove key from the object, otherwise decoding it later + // will yield an error because the struct won't recognize it + // (this is only needed because we strictly enforce that + // all keys are recognized when loading modules) + delete(tmp, moduleNameKey) + result, err := json.Marshal(tmp) + if err != nil { + return "", nil, fmt.Errorf("re-encoding module configuration: %v", err) + } + + return moduleName, result, nil } // Provisioner is implemented by modules which may need to perform @@ -166,6 +178,16 @@ type CleanerUpper interface { Cleanup() error } +// strictUnmarshalJSON is like json.Unmarshal but returns an error +// if any of the fields are unrecognized. Useful when decoding +// module configurations, where you want to be more sure they're +// correct. +func strictUnmarshalJSON(data []byte, v interface{}) error { + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + return dec.Decode(v) +} + var ( modules = make(map[string]Module) modulesMu sync.Mutex