Fix numeric configuration keys handling (#198)

* Fix numeric configuration keys handling

* Update NOTICE.txt

getset.go

@@ -252,7 +252,7 @@ func (c *Config) SetChild(name string, idx int, value *Config, opts ...Option) e
 
 // getField supports the options: PathSep, Env, Resolve, ResolveEnv
 func (c *Config) getField(name string, idx int, opts *options) (value, Error) {
-	p := parsePathIdx(name, opts.pathSep, idx)
+	p := parsePathIdx(name, idx, opts)
 	v, err := p.GetValue(c, opts)
 	if err != nil {
 		return v, err
@@ -267,7 +267,7 @@ func (c *Config) getField(name string, idx int, opts *options) (value, Error) {
 // setField supports the options: PathSep, MetaData
 func (c *Config) setField(name string, idx int, v value, options []Option) Error {
 	opts := makeOptions(options)
-	p := parsePathIdx(name, opts.pathSep, idx)
+	p := parsePathIdx(name, idx, opts)
 
 	err := p.SetValue(c, opts, v)
 	if err != nil {

merge.go

@@ -34,34 +34,33 @@ import (
 // Merge supports the options: PathSep, MetaData, StructTag, VarExp, ReplaceValues, AppendValues, PrependValues
 //
 // Merge uses the type-dependent default encodings:
-//  - Boolean values are encoded as booleans.
-//  - Integer are encoded as int64 values, unsigned integer values as uint64 and
-//    floats as float64 values.
-//  - Strings are copied into string values.
-//    If the VarExp is set, string fields will be parsed into
-//    variable expansion expressions. The expression can reference any
-//    other setting by absolute name.
-//  - Array and slices are copied into new Config objects with index accessors only.
-//  - Struct values and maps with key type string are encoded as Config objects with
-//    named field accessors.
-//  - Config objects will be copied and added to the current hierarchy.
+//   - Boolean values are encoded as booleans.
+//   - Integer are encoded as int64 values, unsigned integer values as uint64 and
+//     floats as float64 values.
+//   - Strings are copied into string values.
+//     If the VarExp is set, string fields will be parsed into
+//     variable expansion expressions. The expression can reference any
+//     other setting by absolute name.
+//   - Array and slices are copied into new Config objects with index accessors only.
+//   - Struct values and maps with key type string are encoded as Config objects with
+//     named field accessors.
+//   - Config objects will be copied and added to the current hierarchy.
 //
 // The `config` struct tag (configurable via StructTag option) can be used to
 // set the field name and enable additional merging settings per field:
 //
-//  // field appears in Config as key "myName"
-//  Field int `config:"myName"`
+//	// field appears in Config as key "myName"
+//	Field int `config:"myName"`
 //
-//  // field appears in sub-Config "mySub" as key "myName" (requires PathSep("."))
-//  Field int `config:"mySub.myName"`
+//	// field appears in sub-Config "mySub" as key "myName" (requires PathSep("."))
+//	Field int `config:"mySub.myName"`
 //
-//  // field is processed as if keys are part of outer struct (type can be a
-//  // struct, a slice, an array, a map or of type *Config)
-//  Field map[string]interface{} `config:",inline"`
-//
-//  // field is ignored by Merge
-//  Field string `config:",ignore"`
+//	// field is processed as if keys are part of outer struct (type can be a
+//	// struct, a slice, an array, a map or of type *Config)
+//	Field map[string]interface{} `config:",inline"`
 //
+//	// field is ignored by Merge
+//	Field string `config:",ignore"`
 //
 // Returns an error if merging fails to normalize and validate the from value.
 // If duplicate setting names are detected in the input, merging fails as well.
@@ -379,7 +378,7 @@ func normalizeSetField(
 		return err
 	}
 
-	p := parsePath(name, opts.pathSep)
+	p := parsePathWithOpts(name, opts)
 	old, err := p.GetValue(cfg, opts)
 	if err != nil {
 		if err.Reason() != ErrMissing {
@@ -515,7 +514,7 @@ func normalizeString(ctx context, opts *options, str string) (value, Error) {
 		return newString(ctx, opts.meta, str), nil
 	}
 
-	varexp, err := parseSplice(str, opts.pathSep)
+	varexp, err := parseSplice(str, opts.pathSep, opts.maxIdx, opts.enableNumKeys)
 	if err != nil {
 		return nil, raiseParseSplice(ctx, opts.meta, err)
 	}

opts.go

@@ -25,6 +25,10 @@ import (
 	"github.com/elastic/go-ucfg/parse"
 )
 
+// Some sane value for the index fields such as input.0.foo
+// in order to protect against cases where user specifies input.9223372036854.foo for the key
+const defaultMaxIdx = 1024
+
 // Option type implementing additional options to be passed
 // to go-ucfg library functions.
 type Option func(*options)
@@ -39,6 +43,9 @@ type options struct {
 	varexp       bool
 	noParse      bool
 
+	maxIdx        int64 // Max index field value allowed
+	enableNumKeys bool  // Enables numeric keys, example "123"
+
 	configValueHandling configHandling
 	fieldHandlingTree   *fieldHandlingTree
 
@@ -118,6 +125,24 @@ func Resolve(fn func(name string) (string, parse.Config, error)) Option {
 	}
 }
 
+// MaxIdx overwrites max index field value allowed.
+// By default it is limited to defaultMaxIdx value.
+func MaxIdx(maxIdx int64) Option {
+	return func(o *options) {
+		o.maxIdx = maxIdx
+	}
+}
+
+// EnableNumKeys enables numeric keys, such as "1234" in the configuration.
+// The numeric key values are converted to array's index otherwise by default.
+// This feature is disabled by default for backwards compatibility.
+// This is useful when it's needed to support and preserve the configuration numeric string keys.
+func EnableNumKeys(enableNumKeys bool) Option {
+	return func(o *options) {
+		o.enableNumKeys = enableNumKeys
+	}
+}
+
 // ResolveEnv option adds a look up callback looking up values in the available
 // OS environment variables.
 var ResolveEnv Option = doResolveEnv
@@ -236,6 +261,7 @@ func makeOptions(opts []Option) *options {
 		pathSep:      "", // no separator by default
 		parsed:       map[string]spliceValue{},
 		activeFields: newFieldSet(nil),
+		maxIdx:       defaultMaxIdx,
 	}
 	for _, opt := range opts {
 		opt(&o)

path.go

@@ -43,41 +43,58 @@ type idxField struct {
 	i int
 }
 
-func parsePathIdx(in, sep string, idx int) cfgPath {
+func parsePathIdx(in string, idx int, opts *options) cfgPath {
 	if in == "" {
 		return cfgPath{
-			sep:    sep,
+			sep:    opts.pathSep,
 			fields: []field{idxField{idx}},
 		}
 	}
 
-	p := parsePath(in, sep)
+	p := parsePathWithOpts(in, opts)
 	if idx >= 0 {
 		p.fields = append(p.fields, idxField{idx})
 	}
 
 	return p
 }
 
-func parsePath(in, sep string) cfgPath {
+func parsePath(in, sep string, maxIdx int64, enableNumKeys bool) cfgPath {
 	if sep == "" {
 		return cfgPath{
 			sep:    sep,
-			fields: []field{parseField(in)},
+			fields: []field{parseField(in, maxIdx, enableNumKeys)},
 		}
 	}
 
 	elems := strings.Split(in, sep)
 	fields := make([]field, 0, len(elems))
+	// If property is the name with separators, for example "inputs.0.i"
+	// fall back to original implementation
+	if len(elems) > 1 {
+		enableNumKeys = false
+	}
 	for _, elem := range elems {
-		fields = append(fields, parseField(elem))
+		fields = append(fields, parseField(elem, maxIdx, enableNumKeys))
 	}
 	return cfgPath{fields: fields, sep: sep}
 }
 
-func parseField(in string) field {
-	if idx, err := strconv.ParseInt(in, 0, 64); err == nil {
-		return idxField{int(idx)}
+func parsePathWithOpts(in string, opts *options) cfgPath {
+	return parsePath(in, opts.pathSep, opts.maxIdx, opts.enableNumKeys)
+}
+
+func parseField(in string, maxIdx int64, enableNumKeys bool) field {
+	// If numeric keys are not enabled, fallback to the original implementation
+	if !enableNumKeys {
+		idx, err := strconv.ParseInt(in, 0, 64)
+		// Limit index value to the configurable max.
+		// If the idx > opts.maxIdx treat it as a regular named field.
+		// This preserves the current behavour for small index fields values (<= opts.maxIdx)
+		// and prevents large memory allocations or OOM if the string is large numeric value
+		if err == nil && idx <= int64(maxIdx) {
+			return idxField{int(idx)}
+		}
 	}
 	return namedField{in}
 }

reify.go

@@ -33,55 +33,55 @@ import (
 // value implements the Unpacker interface. Otherwise, Unpack tries to convert
 // the internal value into the target type:
 //
-//  # Primitive types
+//	# Primitive types
 //
-//  bool: requires setting of type bool or string which parses into a
-//     boolean value (true, false, on, off)
-//  int(8, 16, 32, 64): requires any number type convertible to int or a string
-//      parsing to int. Fails if the target value would overflow.
-//  uint(8, 16, 32, 64): requires any number type convertible to int or a string
-//       parsing to int. Fails if the target value is negative or would overflow.
-//  float(32, 64): requires any number type convertible to float or a string
-//       parsing to float. Fails if the target value is negative or would overflow.
-//  string: requires any primitive value which is serialized into a string.
+//	bool: requires setting of type bool or string which parses into a
+//	   boolean value (true, false, on, off)
+//	int(8, 16, 32, 64): requires any number type convertible to int or a string
+//	    parsing to int. Fails if the target value would overflow.
+//	uint(8, 16, 32, 64): requires any number type convertible to int or a string
+//	     parsing to int. Fails if the target value is negative or would overflow.
+//	float(32, 64): requires any number type convertible to float or a string
+//	     parsing to float. Fails if the target value is negative or would overflow.
+//	string: requires any primitive value which is serialized into a string.
 //
-//  # Special types:
+//	# Special types:
 //
-//  time.Duration: requires a number setting converted to seconds or a string
-//       parsed into time.Duration via time.ParseDuration.
-//  *regexp.Regexp: requires a string being compiled into a regular expression
-//       using regexp.Compile.
-//  *Config: requires a Config object to be stored by pointer into the target
-//       value. Can be used to capture a sub-Config without interpreting
-//       the settings yet.
+//	time.Duration: requires a number setting converted to seconds or a string
+//	     parsed into time.Duration via time.ParseDuration.
+//	*regexp.Regexp: requires a string being compiled into a regular expression
+//	     using regexp.Compile.
+//	*Config: requires a Config object to be stored by pointer into the target
+//	     value. Can be used to capture a sub-Config without interpreting
+//	     the settings yet.
 //
-//   # Arrays/Slices:
+//	 # Arrays/Slices:
 //
-//  Requires a Config object with indexed entries. Named entries will not be
-//  unpacked into the Array/Slice. Primitive values will be handled like arrays
-//  of length 1.
+//	Requires a Config object with indexed entries. Named entries will not be
+//	unpacked into the Array/Slice. Primitive values will be handled like arrays
+//	of length 1.
 //
-//  # Map
+//	# Map
 //
-//  Requires a Config object with all named top-level entries being unpacked into
-//  the map.
+//	Requires a Config object with all named top-level entries being unpacked into
+//	the map.
 //
-//  # Struct
+//	# Struct
 //
-//  Requires a Config object. All named values in the Config object will be unpacked
-//  into the struct its fields, if the name is available in the struct.
-//  A field its name is set using the `config` struct tag (configured by StructTag)
-//  If tag is missing or no field name is configured in the tag, the field name
-//  itself will be used.
-//  If the tag sets the `,ignore` flag, the field will not be overwritten.
-//  If the tag sets the `,inline` or `,squash` flag, Unpack will apply the current
-//  configuration namespace to the fields.
-//  If the tag option `replace` is configured, arrays and *ucfg.Config
-//  convertible fields are replaced by the new values.
-//  If the tag options `append` or `prepend` is used, arrays will be merged by
-//  appending/prepending the new array contents.
-//  The struct tag options `replace`, `append`, and `prepend` overwrites the
-//  global value merging strategy (e.g. ReplaceValues, AppendValues, ...) for all sub-fields.
+//	Requires a Config object. All named values in the Config object will be unpacked
+//	into the struct its fields, if the name is available in the struct.
+//	A field its name is set using the `config` struct tag (configured by StructTag)
+//	If tag is missing or no field name is configured in the tag, the field name
+//	itself will be used.
+//	If the tag sets the `,ignore` flag, the field will not be overwritten.
+//	If the tag sets the `,inline` or `,squash` flag, Unpack will apply the current
+//	configuration namespace to the fields.
+//	If the tag option `replace` is configured, arrays and *ucfg.Config
+//	convertible fields are replaced by the new values.
+//	If the tag options `append` or `prepend` is used, arrays will be merged by
+//	appending/prepending the new array contents.
+//	The struct tag options `replace`, `append`, and `prepend` overwrites the
+//	global value merging strategy (e.g. ReplaceValues, AppendValues, ...) for all sub-fields.
 //
 // When unpacking into a map, primitive, or struct Unpack will call InitDefaults if
 // the type implements the Initializer interface. The Initializer interface is not supported
@@ -109,13 +109,13 @@ import (
 // Struct field validators are set using the `validate` tag (configurable by
 // ValidatorTag). Default validators options are:
 //
-//  required: check value is set and not empty
-//  nonzero: check numeric value != 0 or string/slice not being empty
-//  positive: check numeric value >= 0
-//  min=<value>: check numeric value >= <value>. If target type is time.Duration,
-//       <value> can be a duration.
-//  max=<value>: check numeric value <= <value>. If target type is time.Duration,
-//     <value> can be a duration.
+//	required: check value is set and not empty
+//	nonzero: check numeric value != 0 or string/slice not being empty
+//	positive: check numeric value >= 0
+//	min=<value>: check numeric value >= <value>. If target type is time.Duration,
+//	     <value> can be a duration.
+//	max=<value>: check numeric value <= <value>. If target type is time.Duration,
+//	   <value> can be a duration.
 //
 // If a config value is not the convertible to the target type, or overflows the
 // target type, Unpack will abort immediately and return the appropriate error.
@@ -126,14 +126,14 @@ import (
 // When unpacking into an interface{} value, Unpack will store a value of one of
 // these types in the value:
 //
-//   bool for boolean values
-//   int64 for signed integer values
-//   uint64 for unsigned integer values
-//   float64 for floating point values
-//   string for string values
-//   []interface{} for list-only Config objects
-//   map[string]interface{} for Config objects
-//   nil for pointers if key has a nil value
+//	bool for boolean values
+//	int64 for signed integer values
+//	uint64 for unsigned integer values
+//	float64 for floating point values
+//	string for string values
+//	[]interface{} for list-only Config objects
+//	map[string]interface{} for Config objects
+//	nil for pointers if key has a nil value
 func (c *Config) Unpack(to interface{}, options ...Option) error {
 	opts := makeOptions(options)
 
@@ -311,7 +311,7 @@ func reifyGetField(
 	to reflect.Value,
 	fieldType reflect.Type,
 ) Error {
-	p := parsePath(name, opts.opts.pathSep)
+	p := parsePathWithOpts(name, opts.opts)
 	value, err := p.GetValue(cfg, opts.opts)
 	if err != nil {
 		if err.Reason() != ErrMissing {

ucfg.go

@@ -132,7 +132,7 @@ func (c *Config) GetFields() []string {
 // value is found in the middle of the traversal.
 func (c *Config) Has(name string, idx int, options ...Option) (bool, error) {
 	opts := makeOptions(options)
-	p := parsePathIdx(name, opts.pathSep, idx)
+	p := parsePathIdx(name, idx, opts)
 	return p.Has(c, opts)
 }
 
@@ -167,7 +167,7 @@ func (c *Config) Remove(name string, idx int, options ...Option) (bool, error) {
 	opts.resolvers = nil
 	opts.noParse = true
 
-	p := parsePathIdx(name, opts.pathSep, idx)
+	p := parsePathIdx(name, idx, opts)
 	return p.Remove(c, opts)
 }