From d8f266eec634bad5f681cb5d0ad6bd60137f5c04 Mon Sep 17 00:00:00 2001 From: Javier David Carnelli Date: Tue, 15 Sep 2026 05:23:18 -0300 Subject: [PATCH 1/4] fix: DB2 native DSN brace matching can misattribute a later value's closing brace splitDB2DSN decided whether a '{' opened a quoted ODBC value by checking whether any '}' existed anywhere later in the string, with no concept of brace pairing. An earlier, unterminated '{' could steal the closing '}' of a later, legitimately-braced value, swallowing the key in between and causing DSNDatabase to silently return an empty database name. Replace the lookahead with LIFO stack-based brace matching (matchBraces) so each '{' pairs with the '}' that actually closes it; an unmatched brace stays literal instead of consuming unrelated content. Co-Authored-By: Claude Sonnet 5 --- pkg/database/db2/dsn.go | 53 ++++++++++++++++++++++++++---------- pkg/database/db2/dsn_test.go | 10 +++++++ 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/pkg/database/db2/dsn.go b/pkg/database/db2/dsn.go index 278abc7e..e0f0d5fa 100644 --- a/pkg/database/db2/dsn.go +++ b/pkg/database/db2/dsn.go @@ -57,33 +57,56 @@ func DSNDatabase(dsn string) string { return database } +// matchBraces pairs each '{' with the '}' that actually closes it via LIFO stack +// matching, so an earlier unterminated '{' can't steal a later value's closing '}'. +// Unmatched braces have no entry in the returned map. +func matchBraces(s string) map[int]int { + pairs := make(map[int]int) + var stack []int + for i := 0; i < len(s); i++ { + switch s[i] { + case '{': + stack = append(stack, i) + case '}': + if n := len(stack); n > 0 { + open := stack[n-1] + stack = stack[:n-1] + pairs[open] = i + } + } + } + return pairs +} + // 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. +// opens a value and has a genuine matching '}' (per matchBraces). func splitDB2DSN(dsn string) []string { + 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 { + 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. diff --git a/pkg/database/db2/dsn_test.go b/pkg/database/db2/dsn_test.go index a3c0b776..ad508f1b 100644 --- a/pkg/database/db2/dsn_test.go +++ b/pkg/database/db2/dsn_test.go @@ -134,6 +134,8 @@ func TestIsNativeDSN(t *testing.T) { // 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}, + // HOSTNAME stays visible since the later '{' pairs with DATABASE's '}', not PWD's. + {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,6 +162,14 @@ 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}"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 5ac6635960c308ad118cc484771ff7aa9aad4175 Mon Sep 17 00:00:00 2001 From: Javier David Carnelli Date: Tue, 15 Sep 2026 05:39:45 -0300 Subject: [PATCH 2/4] fix: reject DB2 native DSNs where a brace swallows a later bare field matchBraces fixed the case of an unterminated '{' stealing a later value's own closing '}', but a matched brace pair can still be wrong when the "closing" '}' isn't part of any real quoted value: an earlier unterminated '{' can pair with a stray '}' at the end of a later, plain KEYWORD=value field (e.g. "PWD={oops;DATABASE=TESTDB}"), swallowing that field and causing DSNDatabase to silently return "". Detect this by checking whether a matched brace span's interior looks like it contains a later "KEYWORD=" field. When it does, reject the DSN with ErrAmbiguousDSN instead of guessing: DSNs carry credentials, so fail loud rather than silently drop a field. ParseNativeDSN now returns an error; callers in database.go propagate it instead of discarding it. Co-Authored-By: Claude Sonnet 5 --- pkg/database/database.go | 11 ++++-- pkg/database/db2/dsn.go | 53 ++++++++++++++++++++++------- pkg/database/db2/dsn_test.go | 18 ++++++++++ pkg/database/native_db2_dsn_test.go | 5 +++ 4 files changed, 72 insertions(+), 15 deletions(-) diff --git a/pkg/database/database.go b/pkg/database/database.go index 0c540075..f090a777 100644 --- a/pkg/database/database.go +++ b/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 } diff --git a/pkg/database/db2/dsn.go b/pkg/database/db2/dsn.go index e0f0d5fa..3cc71de9 100644 --- a/pkg/database/db2/dsn.go +++ b/pkg/database/db2/dsn.go @@ -1,6 +1,7 @@ package db2 import ( + "errors" "fmt" "net/url" "regexp" @@ -13,17 +14,27 @@ import ( // being misread as a URL. var urlSchemeRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*://`) +// ErrAmbiguousDSN is returned when a brace-quoted value in a native DB2 DSN appears to have +// swallowed a later, unrelated KEYWORD=value field. Rather than guess which interpretation +// is correct, the DSN is rejected: credential-adjacent parsing should fail loud, not silently +// drop a field. +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) { +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,18 +53,21 @@ 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 +// (see ErrAmbiguousDSN) is treated as not native, so callers fail loudly downstream instead +// of routing a DSN whose fields could not be reliably split. 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 +// if the DSN is ambiguous (see ErrAmbiguousDSN). func DSNDatabase(dsn string) string { - database, _ := ParseNativeDSN(dsn) + database, _, _ := ParseNativeDSN(dsn) return database } @@ -78,9 +92,15 @@ func matchBraces(s string) map[int]int { return pairs } +// bareFieldPattern matches ";KEYWORD=" inside a brace-quoted span: a sign that the span has +// swallowed a separate KEYWORD=value field rather than deliberately quoting one value. +var bareFieldPattern = regexp.MustCompile(`;\s*[A-Za-z][A-Za-z0-9 _]*=`) + // splitDB2DSN splits a native DB2 DSN on ';', treating '{' as ODBC quoting only when it -// opens a value and has a genuine matching '}' (per matchBraces). -func splitDB2DSN(dsn string) []string { +// opens a value and has a genuine matching '}' (per matchBraces). If that quoted span itself +// looks like it swallowed a later KEYWORD=value field (per bareFieldPattern), the DSN is +// rejected with ErrAmbiguousDSN instead of silently dropping that field. +func splitDB2DSN(dsn string) ([]string, error) { pairs := matchBraces(dsn) var parts []string start := 0 @@ -98,6 +118,9 @@ func splitDB2DSN(dsn string) []string { case '{': if atValueStart { if end, ok := pairs[i]; ok { + if bareFieldPattern.MatchString(dsn[i+1 : end]) { + return nil, fmt.Errorf("%w: %q", ErrAmbiguousDSN, dsn[i:end+1]) + } braceEnd = end } } @@ -114,7 +137,7 @@ func splitDB2DSN(dsn string) []string { 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. @@ -145,7 +168,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 } diff --git a/pkg/database/db2/dsn_test.go b/pkg/database/db2/dsn_test.go index ad508f1b..b38781b2 100644 --- a/pkg/database/db2/dsn_test.go +++ b/pkg/database/db2/dsn_test.go @@ -177,3 +177,21 @@ func TestDSNDatabase(t *testing.T) { }) } } + +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 DATABASE=TESTDB into PWD's value. + {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) + }) + } +} diff --git a/pkg/database/native_db2_dsn_test.go b/pkg/database/native_db2_dsn_test.go index b5afda9c..a13261ac 100644 --- a/pkg/database/native_db2_dsn_test.go +++ b/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 { From e36d7dac432c0a79563b46c588d8f2583e0993da Mon Sep 17 00:00:00 2001 From: Javier David Carnelli Date: Thu, 17 Sep 2026 10:55:57 -0300 Subject: [PATCH 3/4] fix: address ambiguous-DSN review findings and trim comments - Never interpolate DSN values into ErrAmbiguousDSN; report only the owning and swallowed keyword names, since values may be credentials. - Only treat a brace-quoted span as ambiguous when the field name it appears to swallow is a reserved DSN keyword, so a password merely containing ";word=" text is no longer rejected. - Make matchBraces value-start aware so a literal '{' inside an already-open value isn't mistaken for a new opener; ODBC values don't nest, and this was truncating values like "a{b;c" to "{a{b". - Add regression tests for all three, and restore coverage for the non-erroring brace-aware split path lost in an earlier fix. - Trim comments across dsn.go and dsn_test.go to one or two sentences. Co-Authored-By: Claude Sonnet 5 --- pkg/database/db2/dsn.go | 75 ++++++++++++++++++++++-------------- pkg/database/db2/dsn_test.go | 17 ++++++-- 2 files changed, 60 insertions(+), 32 deletions(-) diff --git a/pkg/database/db2/dsn.go b/pkg/database/db2/dsn.go index 3cc71de9..53b717f2 100644 --- a/pkg/database/db2/dsn.go +++ b/pkg/database/db2/dsn.go @@ -9,21 +9,16 @@ import ( "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 is returned when a brace-quoted value in a native DB2 DSN appears to have -// swallowed a later, unrelated KEYWORD=value field. Rather than guess which interpretation -// is correct, the DSN is rejected: credential-adjacent parsing should fail loud, not silently -// drop a field. +// 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. +// 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, nil @@ -57,49 +52,71 @@ func ParseNativeDSN(dsn string) (string, bool, error) { } // IsNativeDSN reports whether dsn is DB2's native ODBC keyword=value form. An ambiguous DSN -// (see ErrAmbiguousDSN) is treated as not native, so callers fail loudly downstream instead -// of routing a DSN whose fields could not be reliably split. +// is treated as not native. func IsNativeDSN(dsn string) bool { _, native, err := ParseNativeDSN(dsn) return err == nil && native } -// DSNDatabase returns the DATABASE keyword value from a native DB2 DSN, or "" if absent or -// if the DSN is ambiguous (see ErrAmbiguousDSN). +// DSNDatabase returns the DATABASE keyword value from a native DB2 DSN, or "" if absent or ambiguous. func DSNDatabase(dsn string) string { database, _, _ := ParseNativeDSN(dsn) return database } -// matchBraces pairs each '{' with the '}' that actually closes it via LIFO stack -// matching, so an earlier unterminated '{' can't steal a later value's closing '}'. -// Unmatched braces have no entry in the returned map. +// matchBraces pairs each value-opening '{' with the '}' that closes it via LIFO stack +// matching, so an earlier unterminated '{' can't steal a later value's closing '}'. Only a +// '{' at a value-start position is pushed, since ODBC values don't nest and ambiguous braces +// have no entry in the returned map. func matchBraces(s string) map[int]int { pairs := make(map[int]int) var stack []int + atValueStart := false for i := 0; i < len(s); i++ { switch s[i] { case '{': - stack = append(stack, i) + if atValueStart { + 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 '=': + atValueStart = true + case ';': + atValueStart = false + case ' ', '\t': + // keep atValueStart across whitespace before a brace. + default: + atValueStart = false } } return pairs } -// bareFieldPattern matches ";KEYWORD=" inside a brace-quoted span: a sign that the span has -// swallowed a separate KEYWORD=value field rather than deliberately quoting one value. -var bareFieldPattern = regexp.MustCompile(`;\s*[A-Za-z][A-Za-z0-9 _]*=`) +// 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 and has a genuine matching '}' (per matchBraces). If that quoted span itself -// looks like it swallowed a later KEYWORD=value field (per bareFieldPattern), the DSN is -// rejected with ErrAmbiguousDSN instead of silently dropping that field. +// 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 @@ -118,8 +135,10 @@ func splitDB2DSN(dsn string) ([]string, error) { case '{': if atValueStart { if end, ok := pairs[i]; ok { - if bareFieldPattern.MatchString(dsn[i+1 : end]) { - return nil, fmt.Errorf("%w: %q", ErrAmbiguousDSN, dsn[i:end+1]) + 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 } @@ -132,7 +151,7 @@ func splitDB2DSN(dsn string) ([]string, error) { 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 } diff --git a/pkg/database/db2/dsn_test.go b/pkg/database/db2/dsn_test.go index b38781b2..e54ca615 100644 --- a/pkg/database/db2/dsn_test.go +++ b/pkg/database/db2/dsn_test.go @@ -128,13 +128,15 @@ 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}, - // HOSTNAME stays visible since the later '{' pairs with DATABASE's '}', not PWD's. + // 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 { @@ -170,6 +172,10 @@ func TestDSNDatabase(t *testing.T) { {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"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -184,7 +190,7 @@ func TestParseNativeDSN_Ambiguous(t *testing.T) { dsn string }{ // An earlier unterminated '{' pairs with a bare field's stray trailing '}' instead of - // its own value, swallowing DATABASE=TESTDB into PWD's value. + // 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}"}, } @@ -192,6 +198,9 @@ func TestParseNativeDSN_Ambiguous(t *testing.T) { 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") }) } } From 632856dd327cd49bfc60154cdfbcd37ce00c35b1 Mon Sep 17 00:00:00 2001 From: Javier David Carnelli Date: Thu, 17 Sep 2026 11:53:38 -0300 Subject: [PATCH 4/4] fix: stop a nested '=' from opening a new candidate brace matchBraces set atValueStart on every '=', including one appearing inside an already-open braced value. A non-keyword field there (e.g. "x=" in "DATABASE={db;x={y}") could push a second candidate opener that then stole the real closing brace, truncating the value. Only push a '{' when the identifier preceding its '=' is a reserved DSN keyword, so an '=' inside an open value can no longer be mistaken for a new field's opener. The original ticket's fix (two reserved keywords racing for one close) is unaffected. Co-Authored-By: Claude Sonnet 5 --- pkg/database/db2/dsn.go | 14 +++++++++----- pkg/database/db2/dsn_test.go | 3 +++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/pkg/database/db2/dsn.go b/pkg/database/db2/dsn.go index 53b717f2..7a9f27c4 100644 --- a/pkg/database/db2/dsn.go +++ b/pkg/database/db2/dsn.go @@ -64,18 +64,20 @@ func DSNDatabase(dsn string) string { return database } -// matchBraces pairs each value-opening '{' with the '}' that closes it via LIFO stack -// matching, so an earlier unterminated '{' can't steal a later value's closing '}'. Only a -// '{' at a value-start position is pushed, since ODBC values don't nest and ambiguous braces -// have no entry in the returned map. +// 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 { + if atValueStart && eqPos >= 0 && reservedDSNKeywords[strings.ToUpper(strings.TrimSpace(s[wordStart:eqPos]))] { stack = append(stack, i) } atValueStart = false @@ -87,8 +89,10 @@ func matchBraces(s string) map[int]int { } atValueStart = false case '=': + eqPos = i atValueStart = true case ';': + wordStart = i + 1 atValueStart = false case ' ', '\t': // keep atValueStart across whitespace before a brace. diff --git a/pkg/database/db2/dsn_test.go b/pkg/database/db2/dsn_test.go index e54ca615..c79f1217 100644 --- a/pkg/database/db2/dsn_test.go +++ b/pkg/database/db2/dsn_test.go @@ -176,6 +176,9 @@ func TestDSNDatabase(t *testing.T) { {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) {