Repository: elastic/go-ucfg
Pull request #197 (2024-01-18 19:56:21)

Add exported IgnoreCommas option to fix parser issues
Fixes https://github.com/elastic/go-ucfg/issues/196


This is a rather cautious fix for the above issue, as the issue itself (Parsing a config with `VarExp` causes us to also treat any string with a `,` as an array) is so odd that I'm not sure if there are any secret, undocumented use cases that I might break. Therefore this is an opt-in fix, requiring code to send the `IgnoreCommas` option to `Unpack` in order for us to not trigger this bug.

This also includes a number of tests that demonstrate the issue.

pierrehilbert: @blakerouse could you please have a look now that you are back?
pierrehilbert: /test

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 9e2b41d..024f434 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -1,58 +1,58 @@
 ---
-  name: ci
-
-  on:
-    workflow_dispatch:
-    push:
-      branches:
-        - main
-    pull_request: ~
-
-  permissions:
-    contents: read
-
-  ## Concurrency only allowed in the main branch.
-  ## So old builds running for old commits within the same Pull Request are cancelled
-  concurrency:
-    group: ${{ github.workflow }}-${{ github.ref }}
-    cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
-
-  jobs:
-    check:
-      runs-on: ubuntu-latest
-      steps:
-        - uses: actions/checkout@v3
-
-        - uses: actions/setup-go@v4
-          with:
-            go-version: '1.17'
-
-        - name: Check
-          run: .ci/check.sh
-
-    test:
-      strategy:
-        fail-fast: false
-        matrix:
-          go-version: ['1.17', '1.18']
-      runs-on: ubuntu-latest
-      steps:
-        - uses: actions/checkout@v3
-
-        - uses: actions/setup-go@v4
-          with:
-            go-version: ${{ matrix.go-version }}
-            cache: true
-            cache-dependency-path: '**/go.sum'
-
-        - name: Test
-          run: .ci/test.sh
-          env:
-            GO_VERSION: ${{ matrix.go-version }}
-
-        - name: Store test results
-          if: success() || failure()
-          uses: actions/upload-artifact@v3
-          with:
-            name: test-results
-            path: build/junit*.xml
+name: ci
+
+on:
+  workflow_dispatch:
+  push:
+    branches:
+      - main
+  pull_request: ~
+
+permissions:
+  contents: read
+
+## Concurrency only allowed in the main branch.
+## So old builds running for old commits within the same Pull Request are cancelled
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
+
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: actions/setup-go@v4
+        with:
+          go-version: '1.17'
+
+      - name: Check
+        run: .ci/check.sh
+
+  test:
+    strategy:
+      fail-fast: false
+      matrix:
+        go-version: ['1.17', '1.18']
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: actions/setup-go@v4
+        with:
+          go-version: ${{ matrix.go-version }}
+          cache: true
+          cache-dependency-path: '**/go.sum'
+
+      - name: Test
+        run: .ci/test.sh
+        env:
+          GO_VERSION: ${{ matrix.go-version }}
+
+      - name: Store test results
+        if: success() || failure()
+        uses: actions/upload-artifact@v3
+        with:
+          name: test-results
+          path: build/junit*.xml
diff --git a/NOTICE.txt b/NOTICE.txt
index 05c57a7..5c81acb 100644
--- a/NOTICE.txt
+++ b/NOTICE.txt
@@ -1,4 +1,4 @@
-Copyright 2016-2023 Elasticsearch BV
+Copyright 2016-2024 Elasticsearch BV
 
 This product includes software developed by The Apache Software
 Foundation (http://www.apache.org/).
diff --git a/getset.go b/getset.go
index a0a71f9..b6b456d 100644
--- a/getset.go
+++ b/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 {
diff --git a/merge.go b/merge.go
index 11b5252..1b9b10a 100644
--- a/merge.go
+++ b/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)
 	}
diff --git a/opts.go b/opts.go
index f6957cc..7dae85e 100644
--- a/opts.go
+++ b/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
 
@@ -47,6 +54,8 @@ type options struct {
 	parsed valueCache
 
 	activeFields *fieldSet
+
+	ignoreCommas bool
 }
 
 type valueCache map[string]spliceValue
@@ -75,6 +84,12 @@ func StructTag(tag string) Option {
 	}
 }
 
+var IgnoreCommas = doIgnoreCommas
+
+func doIgnoreCommas(o *options) {
+	o.ignoreCommas = true
+}
+
 // ValidatorTag option sets the struct tag name used to set validators
 // on struct fields in `Unpack`.
 // The default struct tag in `validate`.
@@ -118,6 +133,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 +269,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)
diff --git a/path.go b/path.go
index ff7e630..a89bb47 100644
--- a/path.go
+++ b/path.go
@@ -43,15 +43,15 @@ 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})
 	}
@@ -59,25 +59,42 @@ func parsePathIdx(in, sep string, idx int) cfgPath {
 	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}
 }
diff --git a/reify.go b/reify.go
index 29c0e5d..ee415f1 100644
--- a/reify.go
+++ b/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 {
diff --git a/types.go b/types.go
index b0b8e92..949f910 100644
--- a/types.go
+++ b/types.go
@@ -561,6 +561,11 @@ func parseValue(p *cfgPrimitive, opts *options, str string, parseCfg parse.Confi
 		return nil, raiseNoParse(p.ctx, p.meta())
 	}
 
+	// only set IgnoreCommas if the default has been changed.
+	if opts.ignoreCommas {
+		parseCfg.IgnoreCommas = opts.ignoreCommas
+	}
+
 	ifc, err := parse.ValueWithConfig(str, parseCfg)
 	if err != nil {
 		return nil, err
diff --git a/ucfg.go b/ucfg.go
index f84cf90..b38f363 100644
--- a/ucfg.go
+++ b/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)
 }
 
diff --git a/variables.go b/variables.go
index 791bee3..b559d6e 100644
--- a/variables.go
+++ b/variables.go
@@ -232,7 +232,7 @@ func (e *expansionSingle) eval(cfg *Config, opts *options) (string, error) {
 		return "", err
 	}
 
-	ref := newReference(parsePath(path, e.pathSep))
+	ref := newReference(parsePathWithOpts(path, opts))
 	return ref.eval(cfg, opts)
 }
 
@@ -241,7 +241,7 @@ func (e *expansionDefault) eval(cfg *Config, opts *options) (string, error) {
 	if err != nil || path == "" {
 		return e.right.eval(cfg, opts)
 	}
-	ref := newReference(parsePath(path, e.pathSep))
+	ref := newReference(parsePath(path, e.pathSep, opts.maxIdx, opts.enableNumKeys))
 	v, err := ref.eval(cfg, opts)
 	if err != nil || v == "" {
 		return e.right.eval(cfg, opts)
@@ -255,7 +255,7 @@ func (e *expansionAlt) eval(cfg *Config, opts *options) (string, error) {
 		return "", nil
 	}
 
-	ref := newReference(parsePath(path, e.pathSep))
+	ref := newReference(parsePath(path, e.pathSep, opts.maxIdx, opts.enableNumKeys))
 	tmp, err := ref.resolve(cfg, opts)
 	if err != nil || tmp == nil {
 		return "", nil
@@ -267,7 +267,7 @@ func (e *expansionAlt) eval(cfg *Config, opts *options) (string, error) {
 func (e *expansionErr) eval(cfg *Config, opts *options) (string, error) {
 	path, err := e.left.eval(cfg, opts)
 	if err == nil && path != "" {
-		ref := newReference(parsePath(path, e.pathSep))
+		ref := newReference(parsePath(path, e.pathSep, opts.maxIdx, opts.enableNumKeys))
 		str, err := ref.eval(cfg, opts)
 		if err == nil && str != "" {
 			return str, nil
@@ -281,7 +281,7 @@ func (e *expansionErr) eval(cfg *Config, opts *options) (string, error) {
 	return "", errors.New(errStr)
 }
 
-func (st parseState) finalize(pathSep string) (varEvaler, error) {
+func (st parseState) finalize(pathSep string, maxIdx int64, enableNumKeys bool) (varEvaler, error) {
 	if !st.isvar {
 		return nil, errors.New("fatal: processing non-variable state")
 	}
@@ -298,7 +298,7 @@ func (st parseState) finalize(pathSep string) (varEvaler, error) {
 
 		if len(pieces) == 1 {
 			if str, ok := pieces[0].(constExp); ok {
-				return newReference(parsePath(string(str), pathSep)), nil
+				return newReference(parsePath(string(str), pathSep, maxIdx, enableNumKeys)), nil
 			}
 		}
 
@@ -334,7 +334,7 @@ func makeOpExpansion(l, r varEvaler, op, pathSep string) varEvaler {
 	panic(fmt.Sprintf("Unknown operator: %v", op))
 }
 
-func parseSplice(in, pathSep string) (varEvaler, error) {
+func parseSplice(in, pathSep string, maxIdx int64, enableNumKeys bool) (varEvaler, error) {
 	lex, errs := lexer(in)
 	drainLex := func() {
 		for range lex {
@@ -344,7 +344,7 @@ func parseSplice(in, pathSep string) (varEvaler, error) {
 	// drain lexer on return so go-routine won't leak
 	defer drainLex()
 
-	pieces, perr := parseVarExp(lex, pathSep)
+	pieces, perr := parseVarExp(lex, pathSep, maxIdx, enableNumKeys)
 	if perr != nil {
 		return nil, perr
 	}
@@ -448,7 +448,7 @@ func lexer(in string) (<-chan token, <-chan error) {
 	return lex, errors
 }
 
-func parseVarExp(lex <-chan token, pathSep string) (varEvaler, error) {
+func parseVarExp(lex <-chan token, pathSep string, maxIdx int64, enableNumKeys bool) (varEvaler, error) {
 	stack := []parseState{{st: stLeft}}
 
 	// parser loop
@@ -458,7 +458,7 @@ func parseVarExp(lex <-chan token, pathSep string) (varEvaler, error) {
 			stack = append(stack, parseState{st: stLeft, isvar: true})
 		case tokClose:
 			// finalize and pop state
-			piece, err := stack[len(stack)-1].finalize(pathSep)
+			piece, err := stack[len(stack)-1].finalize(pathSep, maxIdx, enableNumKeys)
 			stack = stack[:len(stack)-1]
 			if err != nil {
 				return nil, err


Test patch:
diff --git a/reify_test.go b/reify_test.go
index cd12c6a..9d85f09 100644
--- a/reify_test.go
+++ b/reify_test.go
@@ -22,8 +22,46 @@ import (
 
 	"github.com/elastic/go-ucfg/parse"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+var exampleInputWithExpVarAndComma = map[string]interface{}{
+	"condition": "startsWith('${host.name}','motmot')",
+	"data_stream": map[string]interface{}{
+		"dataset": "docker.container_logs",
+	},
+}
+
+func TestValueVarExpIgnoreCommas(t *testing.T) {
+	extendedCfg, err := NewFrom(exampleInputWithExpVarAndComma, VarExp)
+	require.NoError(t, err)
+
+	var test map[string]interface{}
+	err = extendedCfg.Unpack(&test, ResolveNOOP, IgnoreCommas) // IgnoreCommmas is what makes this not fail
+	require.NoError(t, err)
+	require.Equal(t, "startsWith('${host.name}','motmot')", test["condition"])
+}
+
+func TestValueUnpackVarExpFail(t *testing.T) {
+	extendedCfg, err := NewFrom(exampleInputWithExpVarAndComma, VarExp)
+	require.NoError(t, err)
+
+	var test map[string]interface{}
+	err = extendedCfg.Unpack(&test, ResolveNOOP) // will fail without IgnoreCommas
+	require.Error(t, err)
+}
+
+func TestValueUnpackNoExpVar(t *testing.T) {
+	extendedCfg, err := NewFrom(exampleInputWithExpVarAndComma) // will not fail, since no VarExp
+	require.NoError(t, err)
+
+	var test map[string]interface{}
+	err = extendedCfg.Unpack(&test, ResolveNOOP)
+	require.NoError(t, err)
+
+	require.Equal(t, "startsWith('${host.name}','motmot')", test["condition"])
+}
+
 func TestUnpackPrimitiveValues(t *testing.T) {
 	tests := []interface{}{
 		New(),
diff --git a/ucfg_test.go b/ucfg_test.go
index cb43a2b..c84e03b 100644
--- a/ucfg_test.go
+++ b/ucfg_test.go
@@ -489,3 +489,93 @@ func TestRemove(t *testing.T) {
 		})
 	}
 }
+
+func TestNewFromWithMaxIdxAndEnableNumKeys(t *testing.T) {
+	baseOptions := []Option{
+		PathSep("."),
+		ResolveEnv,
+		VarExp,
+	}
+
+	tests := []struct {
+		name     string
+		opts     []Option
+		in, want map[string]interface{}
+	}{
+		{
+			name: "original",
+			in: map[string]interface{}{
+				"a": map[string]interface{}{"4": map[string]interface{}{"foo": "bar"}},
+			},
+			want: map[string]interface{}{"a": []interface{}{nil, nil, nil, nil, map[string]interface{}{"foo": "bar"}}}, // original behavour
+		},
+		{
+			name: "original with num keys",
+			opts: []Option{EnableNumKeys(true)},
+			in: map[string]interface{}{
+				"a": map[string]interface{}{"4": map[string]interface{}{"foo": "bar"}},
+			},
+			want: map[string]interface{}{
+				"a": map[string]interface{}{"4": map[string]interface{}{"foo": "bar"}},
+			},
+		},
+		{
+			name: "max idx",
+			opts: []Option{MaxIdx(3)}, // This should preserve numeric key since the max smaller that
+			in: map[string]interface{}{
+				"a": map[string]interface{}{"4": map[string]interface{}{"foo": "bar"}},
+			},
+			want: map[string]interface{}{
+				"a": map[string]interface{}{"4": map[string]interface{}{"foo": "bar"}},
+			},
+		},
+		{
+			name: "max idx with num keys",
+			opts: []Option{MaxIdx(3), EnableNumKeys(true)},
+			in: map[string]interface{}{
+				"a": map[string]interface{}{"4": map[string]interface{}{"foo": "bar"}},
+			},
+			want: map[string]interface{}{
+				"a": map[string]interface{}{"4": map[string]interface{}{"foo": "bar"}},
+			},
+		},
+		{
+			name: "max idx equals",
+			opts: []Option{MaxIdx(4)}, // This should fallback on the old behavour
+			in: map[string]interface{}{
+				"a": map[string]interface{}{"4": map[string]interface{}{"foo": "bar"}},
+			},
+			want: map[string]interface{}{"a": []interface{}{nil, nil, nil, nil, map[string]interface{}{"foo": "bar"}}},
+		},
+		{
+			name: "max idx equals with num keys",
+			opts: []Option{MaxIdx(4), EnableNumKeys(true)}, // This should fallback on the old behavour
+			in: map[string]interface{}{
+				"a": map[string]interface{}{"4": map[string]interface{}{"foo": "bar"}},
+			},
+			want: map[string]interface{}{
+				"a": map[string]interface{}{"4": map[string]interface{}{"foo": "bar"}},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		// Combine base opts with test opts
+		opts := append(baseOptions, tc.opts...)
+
+		// Create config from test map
+		cfg, err := NewFrom(tc.in, opts...)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// Unpack config into map
+		var m map[string]interface{}
+		err = cfg.Unpack(&m, opts...)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		assert.Equal(t, tc.want, m)
+	}
+}
diff --git a/variables_test.go b/variables_test.go
index dcd966b..6f54004 100644
--- a/variables_test.go
+++ b/variables_test.go
@@ -26,7 +26,7 @@ import (
 
 func TestVarExpParserSuccess(t *testing.T) {
 	str := func(s string) varEvaler { return constExp(s) }
-	ref := func(s string) *reference { return newReference(parsePath(s, ".")) }
+	ref := func(s string) *reference { return newReference(parsePath(s, ".", defaultMaxIdx, false)) }
 	cat := func(e ...varEvaler) *splice { return &splice{e} }
 	nested := func(n ...varEvaler) varEvaler {
 		return &expansionSingle{&splice{n}, "."}
@@ -73,7 +73,7 @@ func TestVarExpParserSuccess(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(fmt.Sprintf("%s %s", test.title, test.exp), func(t *testing.T) {
-			actual, err := parseSplice(test.exp, ".")
+			actual, err := parseSplice(test.exp, ".", defaultMaxIdx, false)
 			if assert.NoError(t, err) {
 				assert.Equal(t, test.expected, actual)
 			}
@@ -89,7 +89,7 @@ func TestVarExpParseErrors(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(fmt.Sprintf("test %v: %v", test.title, test.exp), func(t *testing.T) {
-			res, err := parseSplice(test.exp, ".")
+			res, err := parseSplice(test.exp, ".", defaultMaxIdx, false)
 			assert.True(t, err != nil)
 			assert.Error(t, err, fmt.Sprintf("result: %v, error: %v", res, err))
 		})