Skip to content

Conversation

@pawannn
Copy link

@pawannn pawannn commented Nov 1, 2025

Description

This PR fixes a bug in framework.TypeKVPairs where comma-separated key-value pair strings are incorrectly parsed as a single pair instead of multiple pairs.

Input: "A=a,B=b,C=c"
Expected Output: map[A:a B:b C:c] (3 pairs)
Actual Output: map[A:a,B=b,C=c] (1 pair)

Problem

The TypeKVPairs field type only parses the first key-value pair from a comma-separated string, treating everything after the first = sign as a single value. This contradicts the type name and the official documentation in field_type.go which states:

TypeKVPairs allows you to represent the data as a map or a list of equal sign delimited key pairs

The phrase "a list of" (plural) clearly indicates support for multiple key-value pairs, not just a single pair.

Related

Fixes #31621

Reproduction

Consider the following test case

func TestTypeKVPairs(t *testing.T) {
	fields := map[string]*framework.FieldSchema{
		"metadata": {
			Type: framework.TypeKVPairs,
		},
	}

	testCases := []struct {
		name          string
		input         string
		expectedCount int
		expectedPairs map[string]string
	}{
		{
			name:          "Single pair",
			input:         "key1=value1",
			expectedCount: 1,
			expectedPairs: map[string]string{"key1": "value1"},
		},
		{
			name:          "Multiple pairs",
			input:         "key1=value1,key2=value2,key3=value3",
			expectedCount: 3,
			expectedPairs: map[string]string{
				"key1": "value1",
				"key2": "value2",
				"key3": "value3",
			},
		},
		{
			name:          "Two pairs",
			input:         "key1=value1,key2=value2",
			expectedCount: 2,
			expectedPairs: map[string]string{
				"key1": "value1",
				"key2": "value2",
			},
		},
	}

	for _, tc := range testCases {
		t.Run(tc.name, func(t *testing.T) {

			rawData := map[string]interface{}{
				"metadata": tc.input,
			}

			fieldData := &framework.FieldData{
				Raw:    rawData,
				Schema: fields,
			}

			result := fieldData.Get("metadata")
			kvMap, ok := result.(map[string]string)

			if !ok {
				t.Fatal("Failed to convert to map[string]string")
			}

			if len(kvMap) != tc.expectedCount {
				t.Errorf("Expected %d pairs, got %d", tc.expectedCount, len(kvMap))
				t.Logf("Actual map: %v", kvMap)
			}

			for expectedKey, expectedValue := range tc.expectedPairs {
				actualValue, exists := kvMap[expectedKey]
				if !exists {
					t.Errorf("Expected key '%s' not found in result", expectedKey)
				} else if actualValue != expectedValue {
					t.Errorf("For key '%s': expected value '%s', got '%s'",
						expectedKey, expectedValue, actualValue)

				}
			}
		})
	}
}

Output Before Fix

Screenshot 2025-11-02 at 2 02 19 AM

Root Cause

In sdk/framework/field_data.go, when parsing TypeKVPairs, the code uses mapstructure.WeakDecode to decode the raw input as a string slice. When a single comma-separated string is provided, mapstructure treats it as a single-element slice:

var listResult []string
mapstructure.WeakDecode(raw, &listResult)
// Input: "A=a,B=b,C=c"
// Result: listResult = []string{"A=a,B=b,C=c"}  // One element!

The subsequent loop only processes this single element, splitting it at the first =:

for _, keyPair := range listResult {  // Runs only once
    keyPairSlice := strings.SplitN(keyPair, "=", 2)
    // Splits "A=a,B=b,C=c" into ["A", "a,B=b,C=c"]
    result["A"] = "a,B=b,C=c"  // Bug: entire remainder becomes the value
}

Solution

Add logic to detect when listResult contains a single comma-separated string and split it into individual key-value pairs before processing:

// If we have a single element containing commas, split it
if len(listResult) == 1 && strings.Contains(listResult[0], ",") {
    commaSplit := strings.Split(listResult[0], ",")
    if len(commaSplit) > 1 {
        listResult = commaSplit  // Now ["A=a", "B=b", "C=c"]
    }
}

Additionally, trim whitespace from each pair to handle formats like "A=a, B=b".

Changes

  • sdk/framework/field_data.go: Added comma-splitting logic for single-element string slices in the TypeKVPairs case, plus whitespace trimming in the parsing loop
  • sdk/framework/field_data_test.go: Added comprehensive test cases.

Output after the Fix

Screenshot 2025-11-02 at 2 04 39 AM

map[key1:value1 key2:value2 key3:value3] // 3 separate pairs

Breaking Changes

None. This fix maintains full backward compatibility:

  • Single key-value pairs still work: "A=a"{"A": "a"}
  • Map inputs still work: {"A": "a", "B": "b"}{"A": "a", "B": "b"}
  • String array inputs still work: ["A=a", "B=b"]{"A": "a", "B": "b"}
  • New functionality: Comma-separated strings now parse correctly

The fix only applies comma-splitting when a single-element list contains commas, ensuring existing behavior is preserved.

Revert plan: This change can be safely reverted by reverting this PR. No data migration, configuration changes, or additional steps required.

Security impact: No changes to security controls. This is a pure parsing bug fix in the SDK framework that does not affect access control, logging, authentication, or any security mechanisms. The change only affects how string input is parsed into key-value pairs.

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.
  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
  • If applicable, I've documented the impact of any changes to security controls.

Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

@pawannn pawannn requested a review from a team as a code owner November 1, 2025 20:56
@pawannn pawannn requested a review from vidparmar November 1, 2025 20:56
@vercel
Copy link

vercel bot commented Nov 1, 2025

@pawannn is attempting to deploy a commit to the HashiCorp Team on Vercel.

A member of the Team first needs to authorize it.

@hashicorp-cla-app
Copy link

hashicorp-cla-app bot commented Nov 1, 2025

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SDK: framework.TypeKVPairs only handles one KV pair.

2 participants