CXH-2458 Support brace matching on config string

pkg/database/database.go

@@ -533,7 +533,11 @@ func nativeDB2DSN(opts ConnectOptions) (string, string, bool, error) {
 	if err != nil {
 		return "", "", false, err
 	}
-	if _, native := db2.ParseNativeDSN(dsn); !native {
+	_, native, err := db2.ParseNativeDSN(dsn)
+	if err != nil {
+		return "", "", false, fmt.Errorf("invalid native DB2 DSN: %w", err)
+	}
+	if !native {
 		return "", "", false, nil
 	}
 	// Confirmed native: re-expand with keyword-injection validation. The expansion above
@@ -542,7 +546,10 @@ func nativeDB2DSN(opts ConnectOptions) (string, string, bool, error) {
 	if err != nil {
 		return "", "", false, err
 	}
-	database, _ := db2.ParseNativeDSN(safeDSN)
+	database, _, err := db2.ParseNativeDSN(safeDSN)
+	if err != nil {
+		return "", "", false, fmt.Errorf("invalid native DB2 DSN: %w", err)
+	}
 	return safeDSN, database, true, nil
 }
 

pkg/database/db2/dsn.go

@@ -1,29 +1,35 @@
 package db2
 
 import (
+	"errors"
 	"fmt"
 	"net/url"
 	"regexp"
 	"sort"
 	"strings"
 )
 
-// urlSchemeRegex matches a DSN that begins with a URL scheme (e.g. "db2://"); anchoring
-// to the start keeps a native DSN whose value contains "://" (e.g. PWD=my://secret) from
-// being misread as a URL.
+// urlSchemeRegex matches a DSN that begins with a URL scheme, so a native DSN whose value
+// contains "://" isn't misread as a URL.
 var urlSchemeRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*://`)
 
+// ErrAmbiguousDSN means a brace-quoted value appears to contain a later KEYWORD=value field.
+// The DSN is rejected rather than guessed at, since it may carry credentials.
+var ErrAmbiguousDSN = errors.New("ambiguous DB2 DSN: a brace-quoted value appears to contain a later field")
+
 // ParseNativeDSN reports whether dsn is DB2's native ODBC keyword=value form (not a URL),
-// returning its DATABASE value if present. HOSTNAME, not DATABASE alone, is the native
-// marker, since other engines' ODBC/ADO strings also carry DATABASE; every caller shares
-// this one detector to avoid drift.
-func ParseNativeDSN(dsn string) (string, bool) {
+// returning its DATABASE value if present. HOSTNAME, not DATABASE alone, is the native marker.
+func ParseNativeDSN(dsn string) (string, bool, error) {
 	if urlSchemeRegex.MatchString(dsn) {
-		return "", false
+		return "", false, nil
+	}
+	parts, err := splitDB2DSN(dsn)
+	if err != nil {
+		return "", false, err
 	}
 	var database string
 	native, haveDB := false, false
-	for _, part := range splitDB2DSN(dsn) {
+	for _, part := range parts {
 		keyword, value, found := strings.Cut(part, "=")
 		if !found {
 			continue
@@ -42,56 +48,119 @@ func ParseNativeDSN(dsn string) (string, bool) {
 			}
 		}
 	}
-	return database, native
+	return database, native, nil
 }
 
-// IsNativeDSN reports whether dsn is DB2's native ODBC keyword=value form.
+// IsNativeDSN reports whether dsn is DB2's native ODBC keyword=value form. An ambiguous DSN
+// is treated as not native.
 func IsNativeDSN(dsn string) bool {
-	_, native := ParseNativeDSN(dsn)
-	return native
+	_, native, err := ParseNativeDSN(dsn)
+	return err == nil && native
 }
 
-// DSNDatabase returns the DATABASE keyword value from a native DB2 DSN, or "" if absent.
+// DSNDatabase returns the DATABASE keyword value from a native DB2 DSN, or "" if absent or ambiguous.
 func DSNDatabase(dsn string) string {
-	database, _ := ParseNativeDSN(dsn)
+	database, _, _ := ParseNativeDSN(dsn)
 	return database
 }
 
-// splitDB2DSN splits a native DB2 DSN on ';', treating '{' as ODBC quoting only when it
-// opens a value and is later closed by '}'; an unterminated or misplaced '{' is literal,
-// so HOSTNAME/DATABASE markers stay visible instead of being silently swallowed.
-func splitDB2DSN(dsn string) []string {
+// matchBraces pairs each reserved-keyword's opening '{' with the '}' that closes it via LIFO
+// stack matching, so an earlier unterminated '{' can't steal a later value's closing '}'. Only
+// a '{' right after a reserved keyword's '=' is pushed, so a '=' occurring inside an
+// already-open value (ODBC values don't nest) can't be mistaken for a new field's opener.
+// Ambiguous braces have no entry in the returned map.
+func matchBraces(s string) map[int]int {
+	pairs := make(map[int]int)
+	var stack []int
+	wordStart, eqPos := 0, -1
+	atValueStart := false
+	for i := 0; i < len(s); i++ {
+		switch s[i] {
+		case '{':
+			if atValueStart && eqPos >= 0 && reservedDSNKeywords[strings.ToUpper(strings.TrimSpace(s[wordStart:eqPos]))] {
+				stack = append(stack, i)
+			}
+			atValueStart = false
+		case '}':
+			if n := len(stack); n > 0 {
+				open := stack[n-1]
+				stack = stack[:n-1]
+				pairs[open] = i
+			}
+			atValueStart = false
+		case '=':
+			eqPos = i
+			atValueStart = true
+		case ';':
+			wordStart = i + 1
+			atValueStart = false
+		case ' ', '\t':
+			// keep atValueStart across whitespace before a brace.
+		default:
+			atValueStart = false
+		}
+	}
+	return pairs
+}
+
+// bareFieldPattern captures a "KEYWORD=" immediately after a ';' inside a brace-quoted span.
+var bareFieldPattern = regexp.MustCompile(`;\s*([A-Za-z][A-Za-z0-9_]*)=`)
+
+// swallowedReservedField reports whether span contains what looks like a later reserved
+// KEYWORD=value field, and returns that keyword. Only reserved keywords count, so a value
+// that merely contains ";word=" text isn't misclassified as ambiguous.
+func swallowedReservedField(span string) (string, bool) {
+	for _, m := range bareFieldPattern.FindAllStringSubmatch(span, -1) {
+		if keyword := strings.ToUpper(m[1]); reservedDSNKeywords[keyword] {
+			return keyword, true
+		}
+	}
+	return "", false
+}
+
+// splitDB2DSN splits a native DB2 DSN on ';', treating '{' as ODBC quoting only when it opens
+// a value with a genuine matching '}'. A span that appears to swallow a later reserved field
+// makes the DSN ambiguous; the resulting error reports only keyword names, never values.
+func splitDB2DSN(dsn string) ([]string, error) {
+	pairs := matchBraces(dsn)
 	var parts []string
 	start := 0
-	braced := false       // inside a {...} quoted value
+	braceEnd := -1        // index of the '}' that closes the current quoted value, or -1
 	atValueStart := false // at a value position (right after '=', across whitespace) outside braces
 	for i := 0; i < len(dsn); i++ {
+		if braceEnd != -1 {
+			if i == braceEnd {
+				braceEnd = -1
+				atValueStart = false
+			}
+			continue
+		}
 		switch dsn[i] {
-		case '}':
-			braced = false
-			atValueStart = false
 		case '{':
-			if atValueStart && strings.IndexByte(dsn[i:], '}') != -1 {
-				braced = true
+			if atValueStart {
+				if end, ok := pairs[i]; ok {
+					if swallowed, ambiguous := swallowedReservedField(dsn[i+1 : end]); ambiguous {
+						owner, _, _ := strings.Cut(dsn[start:i], "=")
+						return nil, fmt.Errorf("%w (keyword %q appears to swallow a later %q field)",
+							ErrAmbiguousDSN, strings.TrimSpace(owner), swallowed)
+					}
+					braceEnd = end
+				}
 			}
 			atValueStart = false
 		case '=':
-			if !braced {
-				atValueStart = true
-			}
+			atValueStart = true
 		case ';':
-			if !braced {
-				parts = append(parts, dsn[start:i])
-				start = i + 1
-			}
+			parts = append(parts, dsn[start:i])
+			start = i + 1
 			atValueStart = false
 		case ' ', '\t':
-			// keep atValueStart so "DATABASE= {my;db}" still brace-detects.
+			// keep atValueStart across whitespace before a brace.
 		default:
 			atValueStart = false
 		}
 	}
-	return append(parts, dsn[start:])
+	return append(parts, dsn[start:]), nil
 }
 
 // Keywords derived from the URL itself; query parameters may not override them.
@@ -122,7 +191,11 @@ func quoteDB2Value(v string) (string, error) {
 func convertToDB2DSN(dsn string) (string, error) {
 	// If it's already in DB2's native keyword=value format, return as-is.
 	// URL-format DSNs are exempt so those markers may appear in credentials.
-	if IsNativeDSN(dsn) {
+	_, native, err := ParseNativeDSN(dsn)
+	if err != nil {
+		return "", fmt.Errorf("invalid native DB2 DSN: %w", err)
+	}
+	if native {
 		return dsn, nil
 	}
 

pkg/database/db2/dsn_test.go

@@ -128,12 +128,16 @@ func TestIsNativeDSN(t *testing.T) {
 		{name: "space before the =", dsn: "HOSTNAME = h;DATABASE=X", want: true},
 		// DATABASE without HOSTNAME is a generic ODBC/ADO shape (e.g. MSSQL), not native DB2.
 		{name: "database without hostname is not native", dsn: "Server=x;Database=y;User Id=u", want: false},
-		// HOSTNAME appears only inside a braced PWD value, so the brace-aware split keeps it
-		// as one PWD part: not a native marker.
+		// A reserved keyword found after a ';' inside a braced value is rejected as ambiguous.
 		{name: "hostname marker only inside braced value", dsn: "UID=u;PWD={x;HOSTNAME=y}", want: false},
+		// A reserved keyword with no leading ';' can't look like a separate field, so it's kept
+		// buried without erroring.
+		{name: "hostname marker directly inside braced value does not error", dsn: "UID=u;PWD={HOSTNAME=y}", want: false},
 		// Unterminated '{' is literal, so the ';' still splits and HOSTNAME= stays visible;
 		// the malformed value then reaches the driver instead of silently misrouting.
 		{name: "unterminated brace keeps marker visible", dsn: "PWD={oops;HOSTNAME=h", want: true},
+		// The later '{' pairs with DATABASE's '}', not PWD's, so HOSTNAME stays visible.
+		{name: "hostname visible despite later legitimately-braced value", dsn: "HOSTNAME=h;PWD={oops;DATABASE={REAL}", want: true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -160,10 +164,46 @@ func TestDSNDatabase(t *testing.T) {
 		// A literal '{' mid-value (not ODBC quoting) must not swallow the following ';'.
 		{name: "unquoted brace in earlier value", dsn: "HOSTNAME=h;PWD=p{q;DATABASE=TESTDB", want: "TESTDB"},
 		{name: "absent", dsn: "HOSTNAME=h;UID=u", want: ""},
+		// An earlier unterminated '{' must not steal DATABASE's closing '}'.
+		{name: "unterminated brace does not steal a later value's closing brace", dsn: "HOSTNAME=h;PWD={oops;DATABASE={REAL}", want: "REAL"},
+		{name: "two unterminated braces before the real one", dsn: "HOSTNAME=h;A={one;B={two;DATABASE={REAL}", want: "REAL"},
+		{name: "three unterminated braces before the real one", dsn: "HOSTNAME=h;A={one;B={two;C={three;DATABASE={REAL}", want: "REAL"},
+		// A later unterminated brace must not retroactively corrupt an earlier, already-closed value.
+		{name: "real value first, unterminated brace after", dsn: "HOSTNAME=h;DATABASE={REAL};PWD={oops", want: "REAL"},
+		// A stray '}' with no preceding '{' has nothing to pair with and stays literal.
+		{name: "stray closing brace with no opener", dsn: "HOSTNAME=h;DATABASE=TESTDB};UID=u", want: "TESTDB}"},
+		// Only a reserved keyword after the ';' makes a braced value ambiguous.
+		{name: "braced password with semicolon and non-reserved word is not ambiguous", dsn: "HOSTNAME=h;DATABASE=db;UID=u;PWD={pa;ss=word}", want: "db"},
+		// A literal '{' inside a braced value isn't a new opener, since ODBC values don't nest.
+		{name: "literal brace inside a braced value does not truncate it", dsn: "HOSTNAME=h;DATABASE={a{b;c};UID=u", want: "a{b;c"},
+		// A non-reserved keyword's '=' inside an already-open value must not open a new
+		// candidate brace, or it steals the real closing brace and truncates the value.
+		{name: "non-reserved keyword inside a braced value does not steal its closing brace", dsn: "HOSTNAME=h;DATABASE={db;x={y};UID=u", want: "db;x={y"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			require.Equal(t, tt.want, DSNDatabase(tt.dsn))
 		})
 	}
 }
+
+func TestParseNativeDSN_Ambiguous(t *testing.T) {
+	tests := []struct {
+		name string
+		dsn  string
+	}{
+		// An earlier unterminated '{' pairs with a bare field's stray trailing '}' instead of
+		// its own value, swallowing that field.
+		{name: "unterminated brace swallows a later bare field", dsn: "HOSTNAME=h;PWD={oops;DATABASE=TESTDB}"},
+		{name: "unterminated brace swallows multiple later bare fields", dsn: "HOSTNAME=h;PWD={oops;UID=u;DATABASE=TESTDB}"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, _, err := ParseNativeDSN(tt.dsn)
+			require.ErrorIs(t, err, ErrAmbiguousDSN)
+			// The error must never echo any part of the DSN's values, which may be credentials.
+			require.NotContains(t, err.Error(), "oops")
+			require.NotContains(t, err.Error(), "TESTDB")
+		})
+	}
+}

pkg/database/native_db2_dsn_test.go

@@ -68,6 +68,11 @@ func TestNativeDB2DSN(t *testing.T) {
 			opts:    ConnectOptions{DSN: "HOSTNAME=${MISSING};DATABASE=d", Lookup: lookup(map[string]string{})},
 			wantErr: "MISSING",
 		},
+		{
+			name:    "ambiguous brace swallowing a later field errors",
+			opts:    ConnectOptions{DSN: "HOSTNAME=h;PWD={oops;DATABASE=TESTDB}"},
+			wantErr: "ambiguous DB2 DSN",
+		},
 	}
 
 	for _, tt := range tests {