handle edgecases for mergetworanges

Author: jess-lowe

vulnfeeds/conversion/common.go

@@ -284,10 +284,16 @@ func BuildVersionRange(intro string, lastAff string, fixed string) *osvschema.Ra
 	return &versionRange
 }
 
-func MergeTwoRanges(range1, range2 *osvschema.Range) *osvschema.Range {
+// MergeTwoRanges combines two osvschema.Range objects into a single range.
+// It merges the events and the DatabaseSpecific fields. If the ranges are
+// not for the same repository or are of different types, it returns an error.
+// When merging DatabaseSpecific fields, it handles lists, maps, and simple
+// strings. If there are mismatching types for the same key, it returns an error.
+func MergeTwoRanges(range1, range2 *osvschema.Range) (*osvschema.Range, error) {
 	// check if the ranges are the same
 	if range1.GetRepo() != range2.GetRepo() || range1.GetType() != range2.GetType() {
-		return nil
+		// return an error if not the case
+		return nil, fmt.Errorf("ranges are not the same repo or type")
 	}
 
 	mergedRange := &osvschema.Range{
@@ -300,7 +306,7 @@ func MergeTwoRanges(range1, range2 *osvschema.Range) *osvschema.Range {
 	db2 := range2.GetDatabaseSpecific()
 
 	if db1 == nil && db2 == nil {
-		return mergedRange
+		return mergedRange, nil
 	}
 
 	mergedMap := make(map[string]any)
@@ -313,17 +319,16 @@ func MergeTwoRanges(range1, range2 *osvschema.Range) *osvschema.Range {
 
 	if db2 != nil {
 		for k, v := range db2.GetFields() {
+			val2 := v.AsInterface()
 			if existing, ok := mergedMap[k]; ok {
-				// If both are lists, append them
-				if list1, ok := existing.([]any); ok {
-					if list2, ok := v.AsInterface().([]any); ok {
-						mergedMap[k] = append(list1, list2...)
-						continue
-					}
+				mergedVal, err := mergeDatabaseSpecificValues(existing, val2)
+				if err != nil {
+					return nil, fmt.Errorf("failed to merge database specific key %q: %w", k, err)
 				}
+				mergedMap[k] = mergedVal
+			} else {
+				mergedMap[k] = val2
 			}
-			// Otherwise overwrite or add new
-			mergedMap[k] = v.AsInterface()
 		}
 	}
 
@@ -335,5 +340,55 @@ func MergeTwoRanges(range1, range2 *osvschema.Range) *osvschema.Range {
 		}
 	}
 
-	return mergedRange
+	return mergedRange, nil
 }
+
+// mergeDatabaseSpecificValues is a helper function that recursively merges two
+// values from a DatabaseSpecific field. It handles lists (by appending), maps
+// (by recursively merging keys), and simple strings (by creating a list if they
+// differ). It returns an error if the types of the two values do not match.
+func mergeDatabaseSpecificValues(val1, val2 any) (any, error) {
+	switch v1 := val1.(type) {
+	case []any:
+		if v2, ok := val2.([]any); ok {
+			return append(v1, v2...), nil
+		}
+		return nil, fmt.Errorf("mismatching types: %T and %T", val1, val2)
+	case map[string]any:
+		if v2, ok := val2.(map[string]any); ok {
+			merged := make(map[string]any)
+			for k, v := range v1 {
+				merged[k] = v
+			}
+			for k, v := range v2 {
+				if existing, ok := merged[k]; ok {
+					mergedVal, err := mergeDatabaseSpecificValues(existing, v)
+					if err != nil {
+						return nil, err
+					}
+					merged[k] = mergedVal
+				} else {
+					merged[k] = v
+				}
+			}
+			return merged, nil
+		}
+		return nil, fmt.Errorf("mismatching types: %T and %T", val1, val2)
+	case string:
+		if v2, ok := val2.(string); ok {
+			if v1 == v2 {
+				return v1, nil
+			}
+			return []any{v1, v2}, nil
+		}
+		return nil, fmt.Errorf("mismatching types: %T and %T", val1, val2)
+	default:
+		if fmt.Sprintf("%T", val1) != fmt.Sprintf("%T", val2) {
+			return nil, fmt.Errorf("mismatching types: %T and %T", val1, val2)
+		}
+		if val1 == val2 {
+			return val1, nil
+		}
+		return []any{val1, val2}, nil
+	}
+}

vulnfeeds/conversion/common_test.go

@@ -70,10 +70,11 @@ func TestBuildVersionRange(t *testing.T) {
 
 func TestMergeTwoRanges(t *testing.T) {
 	tests := []struct {
-		name   string
-		range1 *osvschema.Range
-		range2 *osvschema.Range
-		want   *osvschema.Range
+		name    string
+		range1  *osvschema.Range
+		range2  *osvschema.Range
+		want    *osvschema.Range
+		wantErr bool
 	}{
 		{
 			name: "Merge identical ranges",
@@ -101,7 +102,7 @@ func TestMergeTwoRanges(t *testing.T) {
 			},
 		},
 		{
-			name: "Different repos should return nil",
+			name: "Different repos should return nil and error",
 			range1: &osvschema.Range{
 				Type: osvschema.Range_GIT,
 				Repo: "https://github.com/example/repo1",
@@ -110,10 +111,11 @@ func TestMergeTwoRanges(t *testing.T) {
 				Type: osvschema.Range_GIT,
 				Repo: "https://github.com/example/repo2",
 			},
-			want: nil,
+			want:    nil,
+			wantErr: true,
 		},
 		{
-			name: "Different types should return nil",
+			name: "Different types should return nil and error",
 			range1: &osvschema.Range{
 				Type: osvschema.Range_GIT,
 				Repo: "https://github.com/example/repo",
@@ -122,7 +124,8 @@ func TestMergeTwoRanges(t *testing.T) {
 				Type: osvschema.Range_ECOSYSTEM,
 				Repo: "https://github.com/example/repo",
 			},
-			want: nil,
+			want:    nil,
+			wantErr: true,
 		},
 		{
 			name: "Merge with DatabaseSpecific",
@@ -198,10 +201,117 @@ func TestMergeTwoRanges(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := MergeTwoRanges(tt.range1, tt.range2)
+			got, err := MergeTwoRanges(tt.range1, tt.range2)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("MergeTwoRanges() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
 			if diff := cmp.Diff(tt.want, got, protocmp.Transform()); diff != "" {
 				t.Errorf("mergeTwoRanges() mismatch (-want +got):\n%s", diff)
 			}
 		})
 	}
 }
+
+func TestMergeDatabaseSpecificValues(t *testing.T) {
+	tests := []struct {
+		name    string
+		val1    any
+		val2    any
+		want    any
+		wantErr bool
+	}{
+		{
+			name: "Merge lists",
+			val1: []any{"a", "b"},
+			val2: []any{"c", "d"},
+			want: []any{"a", "b", "c", "d"},
+		},
+		{
+			name:    "List and string mismatch",
+			val1:    []any{"a", "b"},
+			val2:    "c",
+			wantErr: true,
+		},
+		{
+			name: "Merge maps",
+			val1: map[string]any{"key1": "value1"},
+			val2: map[string]any{"key2": "value2"},
+			want: map[string]any{"key1": "value1", "key2": "value2"},
+		},
+		{
+			name: "Merge nested maps",
+			val1: map[string]any{
+				"nested": map[string]any{
+					"key1": "value1",
+				},
+			},
+			val2: map[string]any{
+				"nested": map[string]any{
+					"key2": "value2",
+				},
+			},
+			want: map[string]any{
+				"nested": map[string]any{
+					"key1": "value1",
+					"key2": "value2",
+				},
+			},
+		},
+		{
+			name:    "Map and string mismatch",
+			val1:    map[string]any{"key1": "value1"},
+			val2:    "string",
+			wantErr: true,
+		},
+		{
+			name: "Merge same strings",
+			val1: "value1",
+			val2: "value1",
+			want: "value1",
+		},
+		{
+			name: "Merge different strings",
+			val1: "value1",
+			val2: "value2",
+			want: []any{"value1", "value2"},
+		},
+		{
+			name:    "String and int mismatch",
+			val1:    "value1",
+			val2:    123,
+			wantErr: true,
+		},
+		{
+			name: "Merge same ints",
+			val1: 123,
+			val2: 123,
+			want: 123,
+		},
+		{
+			name: "Merge different ints",
+			val1: 123,
+			val2: 456,
+			want: []any{123, 456},
+		},
+		{
+			name:    "Int and float64 mismatch",
+			val1:    123,
+			val2:    456.0,
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := mergeDatabaseSpecificValues(tt.val1, tt.val2)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("mergeDatabaseSpecificValues() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !tt.wantErr && !cmp.Equal(got, tt.want) {
+				t.Errorf("mergeDatabaseSpecificValues() mismatch (-want +got):\n%s", cmp.Diff(tt.want, got))
+			}
+		})
+	}
+}

vulnfeeds/conversion/nvd/converter.go

@@ -357,7 +357,11 @@ func MergeRangesAndCreateAffected(resolvedRanges []*osvschema.Range, unresolvedR
 					if mergedRange == nil {
 						mergedRange = vr
 					} else {
-						mergedRange = conversion.MergeTwoRanges(mergedRange, vr)
+						var err error
+						mergedRange, err = conversion.MergeTwoRanges(mergedRange, vr)
+						if err != nil {
+							metrics.AddNote("Failed to merge ranges: %v", err)
+						}
 					}
 				}
 			}