Skip to content

Commit

Permalink
[chore] rely on strict input parsing to error out on invalid unsigned…
Browse files Browse the repository at this point in the history
… int values (#10609)

This removes a hook for confmap that was used to handle invalid uint
values, now that
#9532 is
fixed.

Co-authored-by: Pablo Baeyens <pablo.baeyens@datadoghq.com>
  • Loading branch information
atoulme and mx-psi authored Aug 20, 2024
1 parent e477c3a commit fce1d46
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 17 deletions.
14 changes: 0 additions & 14 deletions confmap/confmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ func decodeConfig(m *Conf, result any, errorUnused bool, skipTopLevelUnmarshaler
// we unmarshal the embedded structs if present to merge with the result:
unmarshalerEmbeddedStructsHookFunc(),
zeroSliceHookFunc(),
negativeUintHookFunc(),
),
}
decoder, err := mapstructure.NewDecoder(dc)
Expand Down Expand Up @@ -499,19 +498,6 @@ func zeroSliceHookFunc() mapstructure.DecodeHookFuncValue {
}
}

// This hook is used to solve the issue: https://github.com/open-telemetry/opentelemetry-collector/issues/9060
// Decoding should fail when converting a negative integer to any type of unsigned integer. This prevents
// negative values being decoded as large uint values.
// TODO: This should be removed as a part of https://github.com/open-telemetry/opentelemetry-collector/issues/9532
func negativeUintHookFunc() mapstructure.DecodeHookFuncValue {
return func(from reflect.Value, to reflect.Value) (interface{}, error) {
if from.CanInt() && from.Int() < 0 && to.CanUint() {
return nil, fmt.Errorf("cannot convert negative value %v to an unsigned integer", from.Int())
}
return from.Interface(), nil
}
}

type moduleFactory[T any, S any] interface {
Create(s S) T
}
Expand Down
2 changes: 1 addition & 1 deletion confmap/confmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func TestUintUnmarshalerFailure(t *testing.T) {
err := conf.Unmarshal(cfg)

assert.Error(t, err)
assert.Contains(t, err.Error(), fmt.Sprintf("decoding failed due to the following error(s):\n\nerror decoding 'uint_test': cannot convert negative value %v to an unsigned integer", testValue))
assert.Contains(t, err.Error(), fmt.Sprintf("decoding failed due to the following error(s):\n\ncannot parse 'uint_test', %d overflows uint", testValue))
}

func TestMapKeyStringToMapKeyTextUnmarshalerHookFuncDuplicateID(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions internal/memorylimiter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,6 @@ func TestUnmarshalInvalidConfig(t *testing.T) {
cfg := &Config{}
err = cm.Unmarshal(&cfg)
require.Error(t, err)
require.Contains(t, err.Error(), "error decoding 'limit_mib': cannot convert negative value -2000 to an unsigned integer")
require.Contains(t, err.Error(), "error decoding 'spike_limit_mib': cannot convert negative value -2300 to an unsigned integer")
require.Contains(t, err.Error(), "cannot parse 'limit_mib', -2000 overflows uint")
require.Contains(t, err.Error(), "cannot parse 'spike_limit_mib', -2300 overflows uint")
}

0 comments on commit fce1d46

Please sign in to comment.