[confmap] Return error when decoding negative values into uints

.chloggen/fix_negative_uint_config.yaml

@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: confmap
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fix decoding negative configuration values into uints
+
+# One or more tracking issues or pull requests related to the change
+issues: [9060]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

confmap/confmap.go

@@ -164,6 +164,7 @@ func decodeConfig(m *Conf, result any, errorUnused bool) error {
 			// we unmarshal the embedded structs if present to merge with the result:
 			unmarshalerEmbeddedStructsHookFunc(),
 			zeroSliceHookFunc(),
+			negativeUintHookFunc(),
 		),
 	}
 	decoder, err := mapstructure.NewDecoder(dc)
@@ -418,6 +419,19 @@ 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
 }

confmap/confmap_test.go

@@ -5,6 +5,7 @@ package confmap
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -213,6 +214,74 @@ func TestMapKeyStringToMapKeyTextUnmarshalerHookFunc(t *testing.T) {
 	assert.Equal(t, map[TestID]string{"string": "this is a string"}, cfg.Map)
 }
 
+type UintConfig struct {
+	UintTest uint32 `mapstructure:"uint_test"`
+}
+
+func TestUintUnmarshalerSuccess(t *testing.T) {
+	tests := []struct {
+		name      string
+		testValue int
+	}{
+		{
+			name:      "Test convert 0 to uint",
+			testValue: 0,
+		},
+		{
+			name:      "Test positive uint conversion",
+			testValue: 1000,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+
+		t.Run(tt.name, func(t *testing.T) {
+			stringMap := map[string]any{
+				"uint_test": tt.testValue,
+			}
+			conf := NewFromStringMap(stringMap)
+			cfg := &UintConfig{}
+			err := conf.Unmarshal(cfg)
+
+			assert.NoError(t, err)
+			assert.Equal(t, cfg.UintTest, uint32(tt.testValue))
+		})
+	}
+}
+
+func TestUint64Unmarshaler(t *testing.T) {
+	negativeInt := -1000
+	testValue := uint64(negativeInt)
+
+	type Uint64Config struct {
+		UintTest uint64 `mapstructure:"uint_test"`
+	}
+	stringMap := map[string]any{
+		"uint_test": testValue,
+	}
+
+	conf := NewFromStringMap(stringMap)
+	cfg := &Uint64Config{}
+	err := conf.Unmarshal(cfg)
+
+	assert.NoError(t, err)
+	assert.Equal(t, cfg.UintTest, testValue)
+}
+
+func TestUintUnmarshalerFailure(t *testing.T) {
+	testValue := -1000
+	stringMap := map[string]any{
+		"uint_test": testValue,
+	}
+	conf := NewFromStringMap(stringMap)
+	cfg := &UintConfig{}
+	err := conf.Unmarshal(cfg)
+
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), fmt.Sprintf("* error decoding 'uint_test': cannot convert negative value %v to an unsigned integer", testValue))
+}
+
 func TestMapKeyStringToMapKeyTextUnmarshalerHookFuncDuplicateID(t *testing.T) {
 	stringMap := map[string]any{
 		"bool": true,

internal/memorylimiter/config_test.go

@@ -93,3 +93,13 @@ func TestConfigValidate(t *testing.T) {
 		})
 	}
 }
+
+func TestUnmarshalInvalidConfig(t *testing.T) {
+	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "negative_unsigned_limits_config.yaml"))
+	require.NoError(t, err)
+	cfg := &Config{}
+	err = component.UnmarshalConfig(cm, 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")
+}

internal/memorylimiter/testdata/negative_unsigned_limits_config.yaml

@@ -0,0 +1,13 @@
+# check_interval is the time between measurements of memory usage for the
+# purposes of avoiding going over the limits. Defaults to zero, so no
+# checks will be performed. Values below 1 second are not recommended since
+# it can result in unnecessary CPU consumption.
+check_interval: 5s
+
+# Maximum amount of memory, in MiB, targeted to be allocated by the process heap.
+# Note that typically the total memory usage of process will be about 50MiB higher
+# than this value.
+limit_mib: -2000
+
+# The maximum, in MiB, spike expected between the measurements of memory usage.
+spike_limit_mib: -2300