config: fix panic on nil value in headers name/value pair

Found a bug during testing of v0.3.0 support with the Collector where the value passed in to the headers name/value pair list could be nil, causing a dereferencing error. This fixes that bug. Signed-off-by: Alex Boten <[email protected]>
codeboten · Dec 11, 2024 · 978a51f · 978a51f
1 parent 8e19210
commit 978a51f
Show file tree

Hide file tree

Showing 5 changed files with 98 additions and 2 deletions.
diff --git a/config/testdata/invalid_nil_value.json b/config/testdata/invalid_nil_value.json
@@ -0,0 +1,49 @@
+{
+    "file_format": "0.3",
+    "disabled": false,
+    "logger_provider": {
+        "processors": [
+            {
+                "batch": {
+                    "schedule_delay": 5000,
+                    "export_timeout": 30000,
+                    "max_queue_size": 2048,
+                    "max_export_batch_size": 512,
+                    "exporter": {
+                        "otlp": {
+                            "protocol": "http/protobuf",
+                            "endpoint": "http://localhost:4318/v1/logs",
+                            "certificate": "/app/cert.pem",
+                            "client_key": "/app/cert.pem",
+                            "client_certificate": "/app/cert.pem",
+                            "headers": [
+                                {
+                                    "name": "api-key",
+                                    "value": "1234"
+                                },
+                                {
+                                    "name": "nil-value"
+                                }
+                            ],
+                            "headers_list": "api-key=1234",
+                            "compression": "gzip",
+                            "timeout": 10000,
+                            "insecure": false
+                        }
+                    }
+                }
+            },
+            {
+                "simple": {
+                    "exporter": {
+                        "console": {}
+                    }
+                }
+            }
+        ],
+        "limits": {
+            "attribute_value_length_limit": 4096,
+            "attribute_count_limit": 128
+        }
+    }
+}
diff --git a/config/testdata/invalid_nil_value.yaml b/config/testdata/invalid_nil_value.yaml
@@ -0,0 +1,13 @@
+file_format: "0.3"
+disabled: false
+logger_provider:
+  processors:
+    - batch:
+        exporter:
+          otlp:
+            protocol: http/protobuf
+            endpoint: http://localhost:4318/v1/logs
+            headers:
+              - name: api-key
+                value: "1234"
+              - name: nil-value
diff --git a/config/v0.3.0/config_json.go b/config/v0.3.0/config_json.go
@@ -113,10 +113,10 @@ func (j *NameStringValuePair) UnmarshalJSON(b []byte) error {
 		return err
 	}
 	if _, ok := raw["name"]; raw != nil && !ok {
-		return errors.New("field name in NameStringValuePair: required")
+		return errors.New("json: cannot unmarshal field name in NameStringValuePair required")
 	}
 	if _, ok := raw["value"]; raw != nil && !ok {
-		return errors.New("field value in NameStringValuePair: required")
+		return errors.New("json: cannot unmarshal field value in NameStringValuePair required")
 	}
 	type Plain NameStringValuePair
 	var plain Plain

diff --git a/config/v0.3.0/config_test.go b/config/v0.3.0/config_test.go
@@ -405,6 +405,11 @@ func TestParseYAML(t *testing.T) {
 			wantErr: errors.New(`yaml: unmarshal errors:
   line 2: cannot unmarshal !!str ` + "`notabool`" + ` into bool`),
 		},
+		{
+			name:    "invalid nil value",
+			input:   "invalid_nil_value.yaml",
+			wantErr: errors.New(`yaml: cannot unmarshal field value in NameStringValuePair required`),
+		},
 		{
 			name:  "valid v0.2 config",
 			input: "v0.2.yaml",
@@ -429,6 +434,7 @@ func TestParseYAML(t *testing.T) {
 
 			got, err := ParseYAML(b)
 			if tt.wantErr != nil {
+				require.Error(t, err)
 				require.Equal(t, tt.wantErr.Error(), err.Error())
 			} else {
 				require.NoError(t, err)
@@ -459,6 +465,11 @@ func TestSerializeJSON(t *testing.T) {
 			input:   "invalid_bool.json",
 			wantErr: errors.New(`json: cannot unmarshal string into Go struct field Plain.disabled of type bool`),
 		},
+		{
+			name:    "invalid nil value",
+			input:   "invalid_nil_value.json",
+			wantErr: errors.New(`json: cannot unmarshal field value in NameStringValuePair required`),
+		},
 		{
 			name:    "valid v0.2 config",
 			input:   "v0.2.json",
@@ -480,6 +491,7 @@ func TestSerializeJSON(t *testing.T) {
 			err = json.Unmarshal(b, &got)
 
 			if tt.wantErr != nil {
+				require.Error(t, err)
 				require.Equal(t, tt.wantErr.Error(), err.Error())
 			} else {
 				require.NoError(t, err)

diff --git a/config/v0.3.0/config_yaml.go b/config/v0.3.0/config_yaml.go
@@ -4,6 +4,7 @@
 package config // import "go.opentelemetry.io/contrib/config/v0.3.0"
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 )
@@ -30,6 +31,27 @@ func (j *AttributeNameValueType) UnmarshalYAML(unmarshal func(interface{}) error
 	return nil
 }
 
+// UnmarshalYAML implements json.Unmarshaler.
+func (j *NameStringValuePair) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	var raw map[string]interface{}
+	if err := unmarshal(&raw); err != nil {
+		return err
+	}
+	if _, ok := raw["name"]; raw != nil && !ok {
+		return errors.New("yaml: cannot unmarshal field name in NameStringValuePair required")
+	}
+	if _, ok := raw["value"]; raw != nil && !ok {
+		return errors.New("yaml: cannot unmarshal field value in NameStringValuePair required")
+	}
+	type Plain NameStringValuePair
+	var plain Plain
+	if err := unmarshal(&plain); err != nil {
+		return err
+	}
+	*j = NameStringValuePair(plain)
+	return nil
+}
+
 // UnmarshalYAML implements yaml.Unmarshaler.
 func (j *LanguageSpecificInstrumentation) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	var raw map[string]interface{}