From c4683b026a1c35dc4bf0cac649cff16996cea88b Mon Sep 17 00:00:00 2001 From: amwps290 Date: Wed, 5 Aug 2026 11:37:57 +0800 Subject: [PATCH] fix(kingbase): filter unsupported DSN parameters safely --- agents/drivers/kingbase-go/main.go | 337 ++++++++++++++++++++++-- agents/drivers/kingbase-go/main_test.go | 261 +++++++++++++++++- 2 files changed, 570 insertions(+), 28 deletions(-) diff --git a/agents/drivers/kingbase-go/main.go b/agents/drivers/kingbase-go/main.go index 924a654dd..510d62f9a 100644 --- a/agents/drivers/kingbase-go/main.go +++ b/agents/drivers/kingbase-go/main.go @@ -910,7 +910,6 @@ func buildDSNWithSSLMode(cp connectParams, sslMode string) string { "password=" + quoteDSNValue(cp.Password), "dbname=" + quoteDSNValue(cp.Database), "sslmode=" + sslMode, - "connect_timeout=15", } if cp.CACertPath != "" { parts = append(parts, "sslrootcert="+quoteDSNValue(cp.CACertPath)) @@ -921,11 +920,15 @@ func buildDSNWithSSLMode(cp connectParams, sslMode string) string { if cp.ClientKeyPath != "" { parts = append(parts, "sslkey="+quoteDSNValue(cp.ClientKeyPath)) } - for _, pair := range strings.FieldsFunc(cp.URLParams, func(r rune) bool { return r == '&' || r == ';' }) { - key, value, ok := strings.Cut(pair, "=") - if ok && isSafeParamKey(key) && !strings.EqualFold(strings.TrimSpace(key), "sslmode") { - parts = append(parts, strings.TrimSpace(key)+"="+quoteDSNValue(strings.TrimSpace(value))) - } + // Classify and de-duplicate the app-supplied url_params. The connect_timeout + // default is only applied when the user did not provide one (natively or via + // the connectTimeout alias), so the parameter is never emitted twice. + urlParams := normalizeURLParams(cp.URLParams) + if !hasDSNParam(urlParams, "connect_timeout") { + parts = append(parts, "connect_timeout=15") + } + for _, p := range urlParams { + parts = append(parts, p.key+"="+quoteDSNValue(p.value)) } return strings.Join(parts, " ") } @@ -1000,19 +1003,39 @@ func rewriteNativeConnectionStringSSLMode(value, sslMode string) string { if strings.HasPrefix(strings.ToLower(value), "kingbase://") { baseAndQuery, fragment, hasFragment := strings.Cut(value, "#") base, query, hasQuery := strings.Cut(baseAndQuery, "?") - pairs := make([]string, 0) + params := make([]dsnParam, 0) if hasQuery { for _, pair := range strings.Split(query, "&") { - key, _, _ := strings.Cut(pair, "=") - decodedKey, err := url.QueryUnescape(key) - if err == nil && strings.EqualFold(decodedKey, "sslmode") { + if pair == "" { continue } - if pair != "" { - pairs = append(pairs, pair) + rawKey, rawValue, _ := strings.Cut(pair, "=") + decodedKey, err := url.QueryUnescape(rawKey) + if err != nil { + decodedKey = rawKey } + if strings.EqualFold(strings.TrimSpace(decodedKey), "sslmode") { + continue + } + decodedValue, err := url.QueryUnescape(rawValue) + if err != nil { + decodedValue = rawValue + } + nativeKey, keep := classifyDSNParam(decodedKey, decodedValue) + if !keep { + continue + } + params = append(params, dsnParam{ + key: nativeKey, + value: rawValue, // preserve the original percent-encoding + fromAlias: !strings.EqualFold(strings.TrimSpace(decodedKey), nativeKey), + }) } } + pairs := make([]string, 0, len(params)+1) + for _, p := range mergeDSNParams(params) { + pairs = append(pairs, url.QueryEscape(p.key)+"="+p.value) + } pairs = append(pairs, "sslmode="+url.QueryEscape(sslMode)) result := base + "?" + strings.Join(pairs, "&") if hasFragment { @@ -1022,14 +1045,32 @@ func rewriteNativeConnectionStringSSLMode(value, sslMode string) string { } fields := splitNativeDSNFields(value) - result := make([]string, 0, len(fields)+1) + params := make([]dsnParam, 0, len(fields)) + passthrough := make([]string, 0) for _, field := range fields { - key, _, ok := strings.Cut(field, "=") - if ok && strings.EqualFold(strings.TrimSpace(key), "sslmode") { + key, rawValue, ok := strings.Cut(field, "=") + if !ok { + passthrough = append(passthrough, field) continue } - result = append(result, field) + if strings.EqualFold(strings.TrimSpace(key), "sslmode") { + continue + } + nativeKey, keep := classifyDSNParam(key, unquoteNativeDSNValue(rawValue)) + if !keep { + continue + } + params = append(params, dsnParam{ + key: nativeKey, + value: rawValue, // preserve the original quoting + fromAlias: !strings.EqualFold(strings.TrimSpace(key), nativeKey), + }) } + result := make([]string, 0, len(params)+len(passthrough)+1) + for _, p := range mergeDSNParams(params) { + result = append(result, p.key+"="+p.value) + } + result = append(result, passthrough...) result = append(result, "sslmode="+sslMode) return strings.Join(result, " ") } @@ -1102,6 +1143,87 @@ func quoteDSNValue(value string) string { return "'" + strings.ReplaceAll(strings.ReplaceAll(value, `\`, `\\`), "'", `\'`) + "'" } +// supportedDSNParams is the curated set of parameters known to be understood by +// the gokb driver or the Kingbase server. It is no longer a strict allow-list: +// classifyDSNParam also forwards unknown lower_snake_case names to the server as +// run-time parameters, because gokb passes every non-driver-setting to the +// startup packet (conn.go startup()). This set is what classifyDSNParam treats +// as definitely native, which short-circuits the camelCase JDBC heuristic so +// CamelCase GUCs such as DateStyle/TimeZone are still forwarded rather than +// dropped. +// +// The list mirrors the driver's own surface: +// - gokb conn.go isDriverSetting(): host, port, password, sslmode, sslcert, +// sslkey, sslrootcert, fallback_application_name, connect_timeout, +// disable_prepared_binary_result, binary_parameters, krbsrvname, krbspn; +// - the standard startup keywords user and dbname; +// - connector.go special handling: client_encoding (must be UTF8), +// datestyle, extra_float_digits; +// - common Kingbase/PostgreSQL run-time parameters that can be set in the +// startup packet: application_name, options, search_path, +// statement_timeout, work_mem, timezone and friends. +var supportedDSNParams = map[string]struct{}{ + // gokb driver settings (conn.go isDriverSetting) and startup keywords + "host": {}, + "port": {}, + "user": {}, + "password": {}, + "dbname": {}, + "sslmode": {}, + "sslcert": {}, + "sslkey": {}, + "sslrootcert": {}, + "fallback_application_name": {}, + "connect_timeout": {}, + "disable_prepared_binary_result": {}, + "binary_parameters": {}, + "krbsrvname": {}, + "krbspn": {}, + + // connector.go special handling + "client_encoding": {}, + "datestyle": {}, + "extra_float_digits": {}, + + // Common run-time parameters the Kingbase server accepts in the startup + // packet (PostgreSQL-compatible GUCs). + "application_name": {}, + "options": {}, + "search_path": {}, + "statement_timeout": {}, + "lock_timeout": {}, + "idle_in_transaction_session_timeout": {}, + "idle_session_timeout": {}, + "work_mem": {}, + "maintenance_work_mem": {}, + "temp_buffers": {}, + "effective_cache_size": {}, + "timezone": {}, + "intervalstyle": {}, + "lc_messages": {}, + "lc_monetary": {}, + "lc_numeric": {}, + "lc_time": {}, + "default_transaction_isolation": {}, + "default_transaction_read_only": {}, + "default_transaction_deferrable": {}, + "synchronous_commit": {}, + "client_min_messages": {}, + "standard_conforming_strings": {}, + "xmloption": {}, + "role": {}, + "session_replication_role": {}, + "default_tablespace": {}, + "temp_tablespaces": {}, + "default_table_access_method": {}, + "max_parallel_workers_per_gather": {}, +} + +func isSupportedDSNParam(key string) bool { + _, ok := supportedDSNParams[strings.ToLower(strings.TrimSpace(key))] + return ok +} + func isSafeParamKey(value string) bool { value = strings.TrimSpace(value) if value == "" { @@ -1115,6 +1237,189 @@ func isSafeParamKey(value string) bool { return true } +// dsnParam is a single normalized connection parameter ready to be emitted into +// a DSN. value carries the surface-specific text (single-quoted for keyword +// DSNs, percent-encoded for kingbase:// URLs, raw for url_params) so callers can +// preserve the original quoting/encoding when only the key was rewritten. +type dsnParam struct { + key string + value string + fromAlias bool +} + +// jdbcAliasParams maps a lowercased JDBC property to the native gokb/server +// parameter with equivalent semantics. clientEncoding is handled separately in +// classifyDSNParam because it also has to validate the value. +var jdbcAliasParams = map[string]string{ + "connecttimeout": "connect_timeout", // both measured in seconds + "currentschema": "search_path", // both accept a comma-separated list + "applicationname": "application_name", +} + +// jdbcOnlyParams lists client-side JDBC/driver properties that have no meaning to +// the Kingbase server. gokb forwards every non-driver-setting to the startup +// packet, so a value the server does not recognize fails the whole connection +// with "unrecognized configuration parameter". camelCase names are also caught by +// the heuristic in classifyDSNParam; this set additionally covers the lowercase +// JDBC properties the heuristic cannot detect and documents intent for the common +// MySQL/JDBC-style names. +var jdbcOnlyParams = map[string]struct{}{ + "usessl": {}, + "autoreconnect": {}, + "characterencoding": {}, + "servertimezone": {}, + "rewritebatchedstatements": {}, + "useserverprepstmts": {}, + "sockettimeout": {}, + "usecompression": {}, + "zerodatetimebehavior": {}, + "useaffectedrows": {}, + "usecursorfetch": {}, + "defaultfetchsize": {}, + "allowmultiqueries": {}, + "useunicode": {}, + // Lowercase PgJDBC/Kingbase-JDBC client properties the camelCase heuristic + // would otherwise forward and break the connection. + "ssl": {}, + "sslfactory": {}, + "stringtype": {}, + "gsslib": {}, + "sspiservicename": {}, + "protocolversion": {}, + "loglevel": {}, +} + +// classifyDSNParam decides how one connection parameter should be treated and +// returns the native parameter name to emit plus whether to keep it. sslmode is +// handled separately by the callers and must not be passed here. decodedValue is +// the already-unquoted/decoded value, used only for the client_encoding check. +func classifyDSNParam(key, decodedValue string) (nativeKey string, keep bool) { + trimmed := strings.TrimSpace(key) + if !isSafeParamKey(trimmed) { + return "", false + } + lower := strings.ToLower(trimmed) + + // client_encoding (native, or via the clientEncoding alias): gokb only + // accepts UTF-8, so map compatible values and drop everything else — a + // non-UTF8 value would otherwise fail the whole connection. + if lower == "client_encoding" || lower == "clientencoding" { + if isUTF8Encoding(decodedValue) { + return "client_encoding", true + } + return "", false + } + + // JDBC properties with a direct native equivalent. + if native, ok := jdbcAliasParams[lower]; ok { + return native, true + } + + // Curated native/server parameters are always forwarded. Matching here also + // keeps CamelCase GUCs such as DateStyle/TimeZone from being mistaken for JDBC + // camelCase properties by the heuristic below. + if isSupportedDSNParam(lower) { + return lower, true + } + + // Known JDBC-only client properties never reach the server. + if _, ok := jdbcOnlyParams[lower]; ok { + return "", false + } + + // Unknown parameter. Server GUCs are conventionally lower_snake_case while + // JDBC properties are camelCase, so forward snake_case names as run-time + // parameters (gokb passes them to the startup packet) and drop names carrying + // an uppercase letter as presumed client-side JDBC settings. + if hasUpperASCII(trimmed) { + return "", false + } + return lower, true +} + +// mergeDSNParams applies duplicate-parameter precedence: an explicit native +// parameter beats a JDBC alias for the same key, and within the same class the +// first occurrence wins to preserve gokb's existing DSN behavior. Output order +// follows each key's first appearance. +func mergeDSNParams(params []dsnParam) []dsnParam { + result := make([]dsnParam, 0, len(params)) + pos := make(map[string]int, len(params)) + for _, p := range params { + if i, ok := pos[p.key]; ok { + // A later explicit native parameter may replace an earlier alias, but + // same-class duplicates keep the first value just as gokb does. + if result[i].fromAlias && !p.fromAlias { + result[i] = p + } + continue + } + pos[p.key] = len(result) + result = append(result, p) + } + return result +} + +// normalizeURLParams classifies and de-duplicates the app-supplied url_params +// blob (a &/;-separated key=value list), excluding sslmode which is handled +// separately. Values are kept raw for later single-quoting. +func normalizeURLParams(raw string) []dsnParam { + params := make([]dsnParam, 0) + for _, pair := range strings.FieldsFunc(raw, func(r rune) bool { return r == '&' || r == ';' }) { + key, value, ok := strings.Cut(pair, "=") + if !ok { + continue + } + if strings.EqualFold(strings.TrimSpace(key), "sslmode") { + continue + } + val := strings.TrimSpace(value) + nativeKey, keep := classifyDSNParam(key, val) + if !keep { + continue + } + params = append(params, dsnParam{ + key: nativeKey, + value: val, + fromAlias: !strings.EqualFold(strings.TrimSpace(key), nativeKey), + }) + } + return mergeDSNParams(params) +} + +func hasDSNParam(params []dsnParam, key string) bool { + for _, p := range params { + if p.key == key { + return true + } + } + return false +} + +func hasUpperASCII(value string) bool { + for i := 0; i < len(value); i++ { + if value[i] >= 'A' && value[i] <= 'Z' { + return true + } + } + return false +} + +// isUTF8Encoding mirrors gokb's isUTF8: it recognizes fuzzy variants of "UTF-8" +// (dropping non-alphanumerics, case-insensitively) as well as "unicode". +func isUTF8Encoding(name string) bool { + var b strings.Builder + for _, ch := range name { + switch { + case ch >= 'A' && ch <= 'Z': + b.WriteRune(ch + ('a' - 'A')) + case ch >= 'a' && ch <= 'z', ch >= '0' && ch <= '9': + b.WriteRune(ch) + } + } + s := b.String() + return s == "utf8" || s == "unicode" +} + func normalizeValue(value any) any { switch typed := value.(type) { case nil: diff --git a/agents/drivers/kingbase-go/main_test.go b/agents/drivers/kingbase-go/main_test.go index 878dbafd4..ed0f31c25 100644 --- a/agents/drivers/kingbase-go/main_test.go +++ b/agents/drivers/kingbase-go/main_test.go @@ -397,24 +397,241 @@ func TestHandshakeAdvertisesMultiSession(t *testing.T) { } } -func TestBuildDSNQuotesCredentialsAndFiltersKeys(t *testing.T) { +// dsnContainsParam reports whether dsn contains key= as a real parameter pair +// (not as a substring of another parameter name such as fallback_application_name). +func dsnContainsParam(dsn, key string) bool { + lowerDSN := strings.ToLower(dsn) + needle := strings.ToLower(strings.TrimSpace(key)) + "=" + if strings.HasPrefix(lowerDSN, needle) { + return true + } + for _, boundary := range []string{" ", "?", "&"} { + if strings.Contains(lowerDSN, boundary+needle) { + return true + } + } + return false +} + +func TestBuildDSNQuotesCredentialsAndFiltersUnsafeKeys(t *testing.T) { dsn := buildDSN(connectParams{ Host: "db host", Port: 54321, Database: "test'db", Username: "system", Password: `p'ass\\word`, - URLParams: "application_name=dbx&bad-key=ignored", + URLParams: "application_name=dbx&fallback_application_name=dbx&useSSL=false&bad-key=ignored", }) for _, expected := range []string{ - `host='db host'`, `dbname='test\'db'`, `password='p\'ass\\\\word'`, `application_name='dbx'`, + `host='db host'`, `dbname='test\'db'`, `password='p\'ass\\\\word'`, `application_name='dbx'`, `fallback_application_name='dbx'`, } { if !strings.Contains(dsn, expected) { t.Fatalf("DSN missing %q: %s", expected, dsn) } } - if strings.Contains(dsn, "bad-key") { - t.Fatalf("unsafe parameter key was accepted: %s", dsn) + for _, skipped := range []string{"useSSL", "bad-key"} { + if dsnContainsParam(dsn, skipped) { + t.Fatalf("unsupported parameter %q was not skipped: %s", skipped, dsn) + } + } +} + +func TestBuildDSNKeepsOnlySupportedURLParams(t *testing.T) { + cp := connectParams{ + Host: "127.0.0.1", + Port: 54321, + Database: "test", + Username: "system", + Password: "secret", + URLParams: "fallback_application_name=dbx&connect_timeout=30&sslcert=cert.pem&sslkey=key.pem&sslrootcert=root.pem" + + "&disable_prepared_binary_result=yes&binary_parameters=yes&krbsrvname=kingbase&krbspn=kingbase/db.example.com" + + "&application_name=dbx&options=-csearch_path=public&client_encoding=UTF8&search_path=public&statement_timeout=1000&work_mem=64MB" + + "&timezone=Asia/Shanghai&default_transaction_read_only=off&synchronous_commit=on" + + "&useSSL=false&autoReconnect=true&characterEncoding=UTF-8&serverTimezone=Asia/Shanghai&rewriteBatchedStatements=true" + + "&useServerPrepStmts=true&connectTimeout=10&socketTimeout=30&useCompression=true&zeroDateTimeBehavior=convertToNull" + + "&useAffectedRows=true&useCursorFetch=true&defaultFetchSize=100&allowMultiQueries=true&useUnicode=true¤tSchema=public", + } + dsn := buildDSN(cp) + for _, expected := range []string{ + `fallback_application_name='dbx'`, `connect_timeout='30'`, `sslcert='cert.pem'`, `sslkey='key.pem'`, `sslrootcert='root.pem'`, + `disable_prepared_binary_result='yes'`, `binary_parameters='yes'`, `krbsrvname='kingbase'`, `krbspn='kingbase/db.example.com'`, + `application_name='dbx'`, `options='-csearch_path=public'`, `client_encoding='UTF8'`, `search_path='public'`, `statement_timeout='1000'`, `work_mem='64MB'`, + `timezone='Asia/Shanghai'`, `default_transaction_read_only='off'`, `synchronous_commit='on'`, + } { + if !strings.Contains(dsn, expected) { + t.Fatalf("DSN missing supported parameter %q: %s", expected, dsn) + } + } + for _, skipped := range []string{ + "useSSL", "autoReconnect", "characterEncoding", "serverTimezone", "rewriteBatchedStatements", "useServerPrepStmts", + "connectTimeout", "socketTimeout", "useCompression", "zeroDateTimeBehavior", "useAffectedRows", "useCursorFetch", + "defaultFetchSize", "allowMultiQueries", "useUnicode", "currentSchema", + } { + if dsnContainsParam(dsn, skipped) { + t.Fatalf("unsupported parameter %q was not skipped: %s", skipped, dsn) + } + } +} + +func TestBuildDSNKeepsOnlySupportedNativeConnectionStringParameters(t *testing.T) { + for _, test := range []struct { + name string + connectionString string + preservedFragments []string + }{ + { + name: "keyword DSN", + connectionString: "host=db.example.com port=54321 user=system password=secret dbname=test connect_timeout=30 fallback_application_name='dbx' application_name='dbx app' options='-c search_path=public' client_encoding=UTF8 disable_prepared_binary_result=yes binary_parameters=yes krbsrvname=kingbase krbspn='kingbase/db.example.com' statement_timeout=1000 useSSL=false serverTimezone=Asia/Shanghai currentSchema=public", + preservedFragments: []string{ + "host=db.example.com", "port=54321", "user=system", "password=secret", "dbname=test", "connect_timeout=30", + "fallback_application_name='dbx'", "application_name='dbx app'", "options='-c search_path=public'", "client_encoding=UTF8", + "disable_prepared_binary_result=yes", "binary_parameters=yes", "krbsrvname=kingbase", "krbspn='kingbase/db.example.com'", + "statement_timeout=1000", + }, + }, + { + name: "Kingbase URL", + connectionString: "kingbase://system:secret@db.example.com:54321/test?connect_timeout=30&fallback_application_name=dbx&application_name=dbx&options=-c%20search_path%3Dpublic&disable_prepared_binary_result=yes&binary_parameters=yes&krbsrvname=kingbase&statement_timeout=1000&useSSL=false&serverTimezone=Asia%2FShanghai¤tSchema=public", + preservedFragments: []string{ + "connect_timeout=30", "fallback_application_name=dbx", "application_name=dbx", "options=-c%20search_path%3Dpublic", + "disable_prepared_binary_result=yes", "binary_parameters=yes", "krbsrvname=kingbase", "statement_timeout=1000", + }, + }, + } { + t.Run(test.name, func(t *testing.T) { + dsn := buildDSN(connectParams{ConnectionString: test.connectionString}) + for _, expected := range test.preservedFragments { + if !strings.Contains(dsn, expected) { + t.Fatalf("native DSN missing supported parameter %q: %s", expected, dsn) + } + } + for _, skipped := range []string{"useSSL", "serverTimezone", "currentSchema"} { + if dsnContainsParam(dsn, skipped) { + t.Fatalf("native DSN kept unsupported parameter %q: %s", skipped, dsn) + } + } + }) + } +} + +// kingbaseParamSurfaces expands a &-separated parameter list into the three +// connection-input surfaces the driver must treat consistently: app-supplied +// url_params, a native keyword DSN, and a kingbase:// URL. Values must not +// contain spaces so they survive the keyword-DSN join. +func kingbaseParamSurfaces(params string) map[string]connectParams { + pairs := strings.Split(params, "&") + keyword := "host=db.example.com user=system password=secret dbname=test " + strings.Join(pairs, " ") + kurl := "kingbase://system:secret@db.example.com:54321/test?" + params + return map[string]connectParams{ + "url_params": {Host: "db.example.com", Port: 54321, Database: "test", Username: "system", Password: "secret", URLParams: params}, + "keyword_dsn": {ConnectionString: keyword}, + "kingbase_url": {ConnectionString: kurl}, + } +} + +// TestBuildDSNForwardsUnknownServerParameters locks in the review's central +// requirement: gokb forwards every non-driver-setting to the server startup +// packet (conn.go startup()), so user/session GUCs that are not in the curated +// native list must still be passed through rather than silently dropped. +func TestBuildDSNForwardsUnknownServerParameters(t *testing.T) { + for surface, cp := range kingbaseParamSurfaces("plan_cache_mode=force_generic_plan&row_security=off&bytea_output=hex") { + t.Run(surface, func(t *testing.T) { + dsn := buildDSN(cp) + for _, key := range []string{"plan_cache_mode", "row_security", "bytea_output"} { + if !dsnContainsParam(dsn, key) { + t.Fatalf("expected server GUC %q to be forwarded: %s", key, dsn) + } + } + }) + } +} + +// TestBuildDSNNormalizesJDBCAliases verifies JDBC properties with a direct native +// equivalent are rewritten to the gokb/server name instead of being discarded. +func TestBuildDSNNormalizesJDBCAliases(t *testing.T) { + for surface, cp := range kingbaseParamSurfaces("connectTimeout=20¤tSchema=public&ApplicationName=dbx&clientEncoding=UTF-8") { + t.Run(surface, func(t *testing.T) { + dsn := buildDSN(cp) + for _, native := range []string{"connect_timeout", "search_path", "application_name", "client_encoding"} { + if !dsnContainsParam(dsn, native) { + t.Fatalf("expected JDBC alias to normalize to %q: %s", native, dsn) + } + } + for _, jdbc := range []string{"connectTimeout", "currentSchema", "ApplicationName", "clientEncoding"} { + if dsnContainsParam(dsn, jdbc) { + t.Fatalf("JDBC alias %q must not be forwarded verbatim: %s", jdbc, dsn) + } + } + }) + } +} + +// TestBuildDSNDropsNonUTF8ClientEncoding checks that a non-UTF-8 clientEncoding +// is dropped: gokb rejects any client_encoding other than UTF-8, so forwarding +// or renaming a GBK value would fail the whole connection. +func TestBuildDSNDropsNonUTF8ClientEncoding(t *testing.T) { + for surface, cp := range kingbaseParamSurfaces("clientEncoding=GBK") { + t.Run(surface, func(t *testing.T) { + dsn := buildDSN(cp) + if dsnContainsParam(dsn, "client_encoding") || strings.Contains(strings.ToLower(dsn), "gbk") { + t.Fatalf("non-UTF8 clientEncoding must be dropped: %s", dsn) + } + }) + } +} + +// TestBuildDSNDropsUnknownCamelCaseJDBCProperties guards the heuristic: unknown +// camelCase names are treated as client-side JDBC properties and dropped, since +// forwarding them would make the server reject the startup packet. +func TestBuildDSNDropsUnknownCamelCaseJDBCProperties(t *testing.T) { + for surface, cp := range kingbaseParamSurfaces("tinyInt1isBit=true&someFutureJdbcFlag=1") { + t.Run(surface, func(t *testing.T) { + dsn := buildDSN(cp) + for _, jdbc := range []string{"tinyInt1isBit", "someFutureJdbcFlag"} { + if dsnContainsParam(dsn, jdbc) { + t.Fatalf("unknown camelCase JDBC property %q must be dropped: %s", jdbc, dsn) + } + } + }) + } +} + +// TestBuildDSNParameterPrecedenceNativeBeatsAlias verifies duplicate-parameter +// precedence: an explicit native parameter wins over its JDBC alias and the +// parameter is emitted exactly once (no duplicate for gokb to resolve). +func TestBuildDSNParameterPrecedenceNativeBeatsAlias(t *testing.T) { + for _, params := range []string{"connect_timeout=30&connectTimeout=10", "connectTimeout=10&connect_timeout=30"} { + for surface, cp := range kingbaseParamSurfaces(params) { + t.Run(surface+"/"+params, func(t *testing.T) { + dsn := buildDSN(cp) + if got := strings.Count(strings.ToLower(dsn), "connect_timeout="); got != 1 { + t.Fatalf("connect_timeout must appear exactly once, saw %d: %s", got, dsn) + } + unquoted := strings.ReplaceAll(dsn, "'", "") + if !strings.Contains(unquoted, "connect_timeout=30") { + t.Fatalf("native connect_timeout=30 must win over alias: %s", dsn) + } + if strings.Contains(unquoted, "connect_timeout=10") { + t.Fatalf("alias connectTimeout=10 must not win: %s", dsn) + } + }) + } + } +} + +func TestBuildDSNPreservesFirstDuplicateWithinSameParameterClass(t *testing.T) { + for _, params := range []string{"application_name=first&application_name=second", "ApplicationName=first&applicationName=second"} { + for surface, cp := range kingbaseParamSurfaces(params) { + t.Run(surface+"/"+params, func(t *testing.T) { + dsn := strings.ReplaceAll(buildDSN(cp), "'", "") + if !strings.Contains(dsn, "application_name=first") { + t.Fatalf("first duplicate value must be preserved: %s", dsn) + } + if strings.Contains(dsn, "application_name=second") { + t.Fatalf("later duplicate value must not replace the first: %s", dsn) + } + }) + } } } @@ -440,7 +657,7 @@ func TestBuildDSNNormalizesPreferWithoutPassingLiteralMode(t *testing.T) { Database: "test", Username: "system", Password: "secret", - URLParams: "SSLMODE=disable&sslmode=prefer&application_name=dbx", + URLParams: "SSLMODE=disable&sslmode=prefer&application_name=dbx&fallback_application_name=dbx&useSSL=false", } if mode := effectiveSSLMode(cp); mode != "prefer" { t.Fatalf("unexpected effective SSL mode: %q", mode) @@ -452,8 +669,13 @@ func TestBuildDSNNormalizesPreferWithoutPassingLiteralMode(t *testing.T) { if !strings.Contains(dsn, "sslmode=require") || strings.Contains(strings.ToLower(dsn), "sslmode=prefer") { t.Fatalf("prefer must be converted to the first require attempt: %s", dsn) } - if !strings.Contains(dsn, "application_name='dbx'") { - t.Fatalf("unrelated URL parameters must be preserved: %s", dsn) + for _, expected := range []string{`application_name='dbx'`, `fallback_application_name='dbx'`} { + if !strings.Contains(dsn, expected) { + t.Fatalf("unrelated URL parameters must be preserved, missing %q: %s", expected, dsn) + } + } + if dsnContainsParam(dsn, "useSSL") { + t.Fatalf("unsupported parameter was not skipped: %s", dsn) } } @@ -462,24 +684,32 @@ func TestBuildDSNOverridesPreferInNativeConnectionStrings(t *testing.T) { name string connectionString string preservedFragments []string + droppedFragments []string }{ { name: "keyword DSN", - connectionString: "host=db.example.com application_name='dbx app' sslmode = 'prefer' options='-c search_path=public tenant'", + connectionString: "host=db.example.com application_name='dbx app' sslmode = 'prefer' options='-c search_path=public tenant' useSSL=false", preservedFragments: []string{ "host=db.example.com", "application_name='dbx app'", "options='-c search_path=public tenant'", }, + droppedFragments: []string{ + "useSSL", + }, }, { name: "Kingbase URL", - connectionString: "kingbase://system:secret@db.example.com/test?application_name=dbx&SSLMODE=prefer#section", + connectionString: "kingbase://system:secret@db.example.com/test?application_name=dbx&options=-c%20search_path%3Dpublic&useSSL=false&SSLMODE=prefer#section", preservedFragments: []string{ "kingbase://system:secret@db.example.com/test?", "application_name=dbx", + "options=-c%20search_path%3Dpublic", "#section", }, + droppedFragments: []string{ + "useSSL", + }, }, } { t.Run(test.name, func(t *testing.T) { @@ -499,6 +729,11 @@ func TestBuildDSNOverridesPreferInNativeConnectionStrings(t *testing.T) { t.Fatalf("native DSN lost %q: %s", fragment, dsn) } } + for _, fragment := range test.droppedFragments { + if dsnContainsParam(dsn, fragment) { + t.Fatalf("native DSN kept unsupported %q: %s", fragment, dsn) + } + } }) } } @@ -511,18 +746,20 @@ func TestOpenAndPingDBNativeConnectionStringsWithoutSSLModeUsePreferFallback(t * }{ { name: "keyword DSN", - connectionString: "host=db.example.com application_name=dbx", + connectionString: "host=db.example.com application_name=dbx fallback_application_name=dbx useSSL=false", preservedFragments: []string{ "host=db.example.com", "application_name=dbx", + "fallback_application_name=dbx", }, }, { name: "Kingbase URL", - connectionString: "kingbase://system:secret@db.example.com/test?application_name=dbx", + connectionString: "kingbase://system:secret@db.example.com/test?application_name=dbx&fallback_application_name=dbx&useSSL=false", preservedFragments: []string{ "kingbase://system:secret@db.example.com/test?", "application_name=dbx", + "fallback_application_name=dbx", }, }, } {