diff --git a/cmd/admin-handlers.go b/cmd/admin-handlers.go index fb0641077..c1703b680 100644 --- a/cmd/admin-handlers.go +++ b/cmd/admin-handlers.go @@ -33,6 +33,7 @@ import ( "github.com/minio/minio/pkg/auth" "github.com/minio/minio/pkg/handlers" "github.com/minio/minio/pkg/madmin" + "github.com/minio/minio/pkg/quick" ) const ( @@ -693,7 +694,7 @@ func (a adminAPIHandlers) SetConfigHandler(w http.ResponseWriter, r *http.Reques // Validate JSON provided in the request body: check the // client has not sent JSON objects with duplicate keys. - if err = checkDupJSONKeys(string(configBytes)); err != nil { + if err = quick.CheckDuplicateKeys(string(configBytes)); err != nil { logger.LogIf(ctx, err) writeErrorResponse(w, ErrAdminConfigBadJSON, r.URL) return diff --git a/cmd/config-current.go b/cmd/config-current.go index e52d2e904..8490b3de5 100644 --- a/cmd/config-current.go +++ b/cmd/config-current.go @@ -19,7 +19,6 @@ package cmd import ( "errors" "fmt" - "io/ioutil" "reflect" "sync" @@ -27,7 +26,6 @@ import ( "github.com/minio/minio/pkg/event" "github.com/minio/minio/pkg/event/target" "github.com/minio/minio/pkg/quick" - "github.com/tidwall/gjson" ) // Steps to move from version N to version N+1 @@ -254,57 +252,6 @@ func newConfig() error { return globalServerConfig.Save() } -// doCheckDupJSONKeys recursively detects duplicate json keys -func doCheckDupJSONKeys(key, value gjson.Result) error { - // Key occurrences map of the current scope to count - // if there is any duplicated json key. - keysOcc := make(map[string]int) - - // Holds the found error - var checkErr error - - // Iterate over keys in the current json scope - value.ForEach(func(k, v gjson.Result) bool { - // If current key is not null, check if its - // value contains some duplicated keys. - if k.Type != gjson.Null { - keysOcc[k.String()]++ - checkErr = doCheckDupJSONKeys(k, v) - } - return checkErr == nil - }) - - // Check found err - if checkErr != nil { - return errors.New(key.String() + " => " + checkErr.Error()) - } - - // Check for duplicated keys - for k, v := range keysOcc { - if v > 1 { - return errors.New(key.String() + " => `" + k + "` entry is duplicated") - } - } - - return nil -} - -// Check recursively if a key is duplicated in the same json scope -// e.g.: -// `{ "key" : { "key" ..` is accepted -// `{ "key" : { "subkey" : "val1", "subkey": "val2" ..` throws subkey duplicated error -func checkDupJSONKeys(json string) error { - // Parse config with gjson library - config := gjson.Parse(json) - - // Create a fake rootKey since root json doesn't seem to have representation - // in gjson library. - rootKey := gjson.Result{Type: gjson.String, Str: minioConfigFile} - - // Check if loaded json contains any duplicated keys - return doCheckDupJSONKeys(rootKey, config) -} - // getValidConfig - returns valid server configuration func getValidConfig() (*serverConfig, error) { srvCfg := &serverConfig{ @@ -312,8 +259,7 @@ func getValidConfig() (*serverConfig, error) { Browser: true, } - configFile := getConfigFile() - if _, err := quick.Load(configFile, srvCfg); err != nil { + if _, err := quick.Load(getConfigFile(), srvCfg); err != nil { return nil, err } @@ -321,21 +267,11 @@ func getValidConfig() (*serverConfig, error) { return nil, fmt.Errorf("configuration version mismatch. Expected: ‘%s’, Got: ‘%s’", serverConfigVersion, srvCfg.Version) } - // Load config file json and check for duplication json keys - jsonBytes, err := ioutil.ReadFile(configFile) - if err != nil { - return nil, err - } - if err = checkDupJSONKeys(string(jsonBytes)); err != nil { - return nil, err - } - // Validate credential fields only when // they are not set via the environment - // Error out if global is env credential is not set and config has invalid credential if !globalIsEnvCreds && !srvCfg.Credential.IsValid() { - return nil, errors.New("invalid credential in config file " + configFile) + return nil, errors.New("invalid credential in config file " + getConfigFile()) } return srvCfg, nil diff --git a/cmd/config-current_test.go b/cmd/config-current_test.go index cab4d08f6..f797008c6 100644 --- a/cmd/config-current_test.go +++ b/cmd/config-current_test.go @@ -24,7 +24,6 @@ import ( "github.com/minio/minio/pkg/auth" "github.com/minio/minio/pkg/event/target" - "github.com/tidwall/gjson" ) func TestServerConfig(t *testing.T) { @@ -126,34 +125,6 @@ func TestServerConfigWithEnvs(t *testing.T) { } } -func TestCheckDupJSONKeys(t *testing.T) { - testCases := []struct { - json string - shouldPass bool - }{ - {`{}`, true}, - {`{"version" : "13"}`, true}, - {`{"version" : "13", "version": "14"}`, false}, - {`{"version" : "13", "credential": {"accessKey": "12345"}}`, true}, - {`{"version" : "13", "credential": {"accessKey": "12345", "accessKey":"12345"}}`, false}, - {`{"version" : "13", "notify": {"amqp": {"1"}, "webhook":{"3"}}}`, true}, - {`{"version" : "13", "notify": {"amqp": {"1"}, "amqp":{"3"}}}`, false}, - {`{"version" : "13", "notify": {"amqp": {"1":{}, "2":{}}}}`, true}, - {`{"version" : "13", "notify": {"amqp": {"1":{}, "1":{}}}}`, false}, - } - - for i, testCase := range testCases { - err := doCheckDupJSONKeys(gjson.Result{}, gjson.Parse(testCase.json)) - if testCase.shouldPass && err != nil { - t.Errorf("Test %d, should pass but it failed with err = %v", i+1, err) - } - if !testCase.shouldPass && err == nil { - t.Errorf("Test %d, should fail but it succeed.", i+1) - } - } - -} - // Tests config validator.. func TestValidateConfig(t *testing.T) { rootPath, err := newTestConfig(globalMinioDefaultRegion) diff --git a/pkg/quick/encoding.go b/pkg/quick/encoding.go index c59f98347..c79a7eaa9 100644 --- a/pkg/quick/encoding.go +++ b/pkg/quick/encoding.go @@ -136,6 +136,11 @@ func loadFileConfig(filename string, v interface{}) error { if runtime.GOOS == "windows" { fileData = []byte(strings.Replace(string(fileData), "\r\n", "\n", -1)) } + + if err = checkDupJSONKeys(string(fileData)); err != nil { + return err + } + // Unmarshal file's content return toUnmarshaller(filepath.Ext(filename))(fileData, v) } diff --git a/pkg/quick/errorutil.go b/pkg/quick/errorutil.go index 2c0bc9759..f20b981b2 100644 --- a/pkg/quick/errorutil.go +++ b/pkg/quick/errorutil.go @@ -21,10 +21,12 @@ package quick import ( "bufio" "bytes" + "errors" "fmt" "io" "github.com/cheggaaa/pb" + "github.com/tidwall/gjson" ) const errorFmt = "%5d: %s <<<<" @@ -81,3 +83,59 @@ func FormatJSONSyntaxError(data io.Reader, offset int64) (highlight string) { return fmt.Sprintf(errorFmt, errLine, readLine.String()[idx:]) } + +// doCheckDupJSONKeys recursively detects duplicate json keys +func doCheckDupJSONKeys(key, value gjson.Result) error { + // Key occurrences map of the current scope to count + // if there is any duplicated json key. + keysOcc := make(map[string]int) + + // Holds the found error + var checkErr error + + // Iterate over keys in the current json scope + value.ForEach(func(k, v gjson.Result) bool { + // If current key is not null, check if its + // value contains some duplicated keys. + if k.Type != gjson.Null { + keysOcc[k.String()]++ + checkErr = doCheckDupJSONKeys(k, v) + } + return checkErr == nil + }) + + // Check found err + if checkErr != nil { + return errors.New(key.String() + " => " + checkErr.Error()) + } + + // Check for duplicated keys + for k, v := range keysOcc { + if v > 1 { + return errors.New(key.String() + " => `" + k + "` entry is duplicated") + } + } + + return nil +} + +// Check recursively if a key is duplicated in the same json scope +// e.g.: +// `{ "key" : { "key" ..` is accepted +// `{ "key" : { "subkey" : "val1", "subkey": "val2" ..` throws subkey duplicated error +func checkDupJSONKeys(json string) error { + // Parse config with gjson library + config := gjson.Parse(json) + + // Create a fake rootKey since root json doesn't seem to have representation + // in gjson library. + rootKey := gjson.Result{Type: gjson.String, Str: "config.json"} + + // Check if loaded json contains any duplicated keys + return doCheckDupJSONKeys(rootKey, config) +} + +// CheckDuplicateKeys - checks for duplicate entries in a JSON file +func CheckDuplicateKeys(json string) error { + return checkDupJSONKeys(json) +} diff --git a/pkg/quick/quick_test.go b/pkg/quick/quick_test.go index 3eec610c7..5fbd5a389 100644 --- a/pkg/quick/quick_test.go +++ b/pkg/quick/quick_test.go @@ -27,6 +27,8 @@ import ( "runtime" "strings" "testing" + + "github.com/tidwall/gjson" ) func TestReadVersion(t *testing.T) { @@ -504,3 +506,31 @@ func TestDeepDiff(t *testing.T) { // fmt.Printf("DeepDiff[%d]: %s=%v\n", i, field.Name(), field.Value()) // } } + +func TestCheckDupJSONKeys(t *testing.T) { + testCases := []struct { + json string + shouldPass bool + }{ + {`{}`, true}, + {`{"version" : "13"}`, true}, + {`{"version" : "13", "version": "14"}`, false}, + {`{"version" : "13", "credential": {"accessKey": "12345"}}`, true}, + {`{"version" : "13", "credential": {"accessKey": "12345", "accessKey":"12345"}}`, false}, + {`{"version" : "13", "notify": {"amqp": {"1"}, "webhook":{"3"}}}`, true}, + {`{"version" : "13", "notify": {"amqp": {"1"}, "amqp":{"3"}}}`, false}, + {`{"version" : "13", "notify": {"amqp": {"1":{}, "2":{}}}}`, true}, + {`{"version" : "13", "notify": {"amqp": {"1":{}, "1":{}}}}`, false}, + } + + for i, testCase := range testCases { + err := doCheckDupJSONKeys(gjson.Result{}, gjson.Parse(testCase.json)) + if testCase.shouldPass && err != nil { + t.Errorf("Test %d, should pass but it failed with err = %v", i+1, err) + } + if !testCase.shouldPass && err == nil { + t.Errorf("Test %d, should fail but it succeed.", i+1) + } + } + +}