fix: TypeKVPairs to handle multiple comma-separated pairs
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
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 theTypeKVPairscase, plus whitespace trimming in the parsing loop -
sdk/framework/field_data_test.go: Added comprehensive test cases.
Output after the Fix
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
- [x] I have documented a clear reason for, and description of, the change I am making.
- [x] If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
- [x] 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 is attempting to deploy a commit to the HashiCorp Team on Vercel.
A member of the Team first needs to authorize it.
Hey, @vidparmar @heatherezell Please check this in your free time.