From 09e2423a7eb736b951307ea946441e4a079e7318 Mon Sep 17 00:00:00 2001 From: Jorge Guerra Date: Mon, 20 Feb 2023 20:44:16 -0600 Subject: [PATCH] Add string check and isNil Panic protection --- remoteconfig.go | 23 ++++++++++++++-- remoteconfig_test.go | 65 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/remoteconfig.go b/remoteconfig.go index 031429a..22374ed 100644 --- a/remoteconfig.go +++ b/remoteconfig.go @@ -51,6 +51,15 @@ func ReadJSONValidate(cfgReader io.Reader, configStruct interface{}) error { return nil } +func isNilFixed(v reflect.Value) bool { + switch v.Kind() { + case reflect.Ptr, reflect.Map, reflect.Array, reflect.Chan, reflect.Slice: + //use of IsNil method + return v.IsNil() + } + return false +} + // Validates a configuration struct. // Uses reflection to determine and call the correct Validation methods for each type. func validateConfigWithReflection(c interface{}) error { @@ -78,9 +87,9 @@ func validateConfigWithReflection(c interface{}) error { continue } - if valueField.IsNil() && !optional { + if isNilFixed(valueField) && !optional { return fmt.Errorf("Field: %s, not set", typeField.Name) - } else if valueField.IsNil() && optional { + } else if isNilFixed(valueField) && optional { continue } @@ -115,7 +124,7 @@ func validateConfigWithReflection(c interface{}) error { continue } - // If this is a string field, check that it isn't empty (unless optional) + // If this is a string pointer field, check that it isn't empty (unless optional) if s, ok := valueField.Interface().(*string); ok { if *s == "" { return fmt.Errorf("String Field: %s, contains an empty string", typeField.Name) @@ -123,6 +132,14 @@ func validateConfigWithReflection(c interface{}) error { continue } + // If this is a string field, check that it isn't empty (unless optional) + if s, ok := valueField.Interface().(string); ok { + if s == "" { + return fmt.Errorf("String Field: %s, contains an empty string", typeField.Name) + } + continue + } + // If the Validater interface is implemented, call the Validate method if typeField.Type.Implements(validaterType) { if err := valueField.Interface().(Validater).Validate(); err != nil { diff --git a/remoteconfig_test.go b/remoteconfig_test.go index 3796ccc..b768661 100644 --- a/remoteconfig_test.go +++ b/remoteconfig_test.go @@ -50,7 +50,8 @@ type SampleConfig struct { SQSClient *SQSClientConfig `json:"sqs_client,omitempty"` DynamoDBTable *DynamoDBTableConfig `json:"dynamodb_table,omitempty"` DynamoDBClient *DynamoDBClientConfig `json:"dynamodb_client,omitempty"` - Str *string `json:"str,omitempty"` + Str string `json:"str,omitempty"` + StrPointer *string `json:"str_pointer,omitempty"` StorageConfig *StorageConfig `json:"storage_config,omitempty"` StorageConfigSlice []*StorageConfig `json:"storage_config_slice,omitempty"` StorageConfigMap map[string]*StorageConfig `json:"storage_config_map,omitempty"` @@ -79,6 +80,7 @@ var validConfigJSON = ` "table_name" : "testTable" }, "str" : "testStr", + "str_pointer" : "testStr", "storage_config" : { "provider" : "aws", "location" : "us-west-2" @@ -151,7 +153,8 @@ func (s *RemoteConfigSuite) TestValidateConfigWithReflectionWithOptional() { SQSClient: sqsClient, DynamoDBTable: dynamodbTable, DynamoDBClient: dynamodbClient, - Str: &str, + Str: str, + StrPointer: &str, StorageConfig: storageConfig, StorageConfigSlice: []*StorageConfig{storageConfig}, StorageConfigMap: map[string]*StorageConfig{"one": storageConfig}, @@ -348,12 +351,52 @@ func (s *RemoteConfigSuite) TestValidateConfigWithReflectionErrorStrNotSet() { SQSClient: sqsClient, DynamoDBTable: dynamodbTable, DynamoDBClient: dynamodbClient, - Str: nil, + Str: "testString", + StrPointer: nil, } err := validateConfigWithReflection(c) assert.NotNil(s.T(), err) - assert.Equal(s.T(), errors.New("Field: Str, not set"), err) + assert.Equal(s.T(), errors.New("Field: StrPointer, not set"), err) +} + +func (s *RemoteConfigSuite) TestValidateConfigWithReflectionErrorStrPointerEmpty() { + sqsRegion := VALID_REMOTE_CONFIG_SQS_REGION + sqsAWSAccountID := VALID_REMOTE_CONFIG_SQS_AWS_ACCOUNT_ID + sqsQueueName := VALID_REMOTE_CONFIG_SQS_QUEUE_NAME + sqsQueue := &SQSQueueConfig{ + Region: &sqsRegion, + AWSAccountID: &sqsAWSAccountID, + QueueName: &sqsQueueName, + } + sqsClient := &SQSClientConfig{ + Region: &sqsRegion, + } + + dynamodbTableName := VALID_REMOTE_CONFIG_DYNAMODB_TABLE_NAME + dynamodbTable := &DynamoDBTableConfig{ + TableName: &dynamodbTableName, + } + + dynamodbClientRegion := VALID_REMOTE_CONFIG_DYNAMODB_CLIENT_REGION + dynamodbClient := &DynamoDBClientConfig{ + Region: &dynamodbClientRegion, + } + + str := "" + + c := &SampleConfig{ + SQSQueue: sqsQueue, + SQSClient: sqsClient, + DynamoDBTable: dynamodbTable, + DynamoDBClient: dynamodbClient, + Str: "testString", + StrPointer: &str, + } + + err := validateConfigWithReflection(c) + assert.NotNil(s.T(), err) + assert.Equal(s.T(), errors.New("String Field: StrPointer, contains an empty string"), err) } func (s *RemoteConfigSuite) TestValidateConfigWithReflectionErrorStrEmpty() { @@ -386,7 +429,7 @@ func (s *RemoteConfigSuite) TestValidateConfigWithReflectionErrorStrEmpty() { SQSClient: sqsClient, DynamoDBTable: dynamodbTable, DynamoDBClient: dynamodbClient, - Str: &str, + Str: str, } err := validateConfigWithReflection(c) @@ -424,7 +467,8 @@ func (s *RemoteConfigSuite) TestValidateConfigWithReflectionErrorStorageConfigNo SQSClient: sqsClient, DynamoDBTable: dynamodbTable, DynamoDBClient: dynamodbClient, - Str: &str, + Str: str, + StrPointer: &str, StorageConfig: nil, } @@ -470,7 +514,8 @@ func (s *RemoteConfigSuite) TestValidateConfigWithReflectionErrorStorageConfigSl SQSClient: sqsClient, DynamoDBTable: dynamodbTable, DynamoDBClient: dynamodbClient, - Str: &str, + Str: str, + StrPointer: &str, StorageConfig: storageConfig, StorageConfigSlice: nil, } @@ -549,7 +594,8 @@ func (s *RemoteConfigSuite) TestValidateConfigWithReflectionErrorStorageConfigSl SQSClient: sqsClient, DynamoDBTable: dynamodbTable, DynamoDBClient: dynamodbClient, - Str: &str, + Str: str, + StrPointer: &str, StorageConfig: storageConfig, StorageConfigSlice: []*StorageConfig{}, } @@ -691,7 +737,8 @@ func (s *RemoteConfigSuite) buildValidSampleConfig() *SampleConfig { SQSClient: sqsClient, DynamoDBTable: dynamodbTable, DynamoDBClient: dynamodbClient, - Str: &str, + Str: str, + StrPointer: &str, StorageConfig: storageConfig, StorageConfigSlice: []*StorageConfig{storageConfig}, StorageConfigMap: map[string]*StorageConfig{"one": storageConfig},