Skip to content

[chore] rely on strict input parsing to error out on invalid unsigned int values #10609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 20, 2024
Merged

[chore] rely on strict input parsing to error out on invalid unsigned int values #10609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca47f3a
[chore] rely on strict input parsing to error out on invalid unsigned…
atoulme Jul 14, 2024
ec43229
Merge branch 'main' into remove_todo_confmap
atoulme Jul 15, 2024
496b39a
Merge remote-tracking branch 'origin/main' into remove_todo_confmap
mx-psi Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions confmap/confmap.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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")
}
Loading