Skip to content

Commit b1333cd

Browse files
fix(oracle): normalize encoded proxy usernames in go-ora DSN (googleapis#2469)
## Summary Fix Oracle go-ora DSN construction so proxy usernames with brackets (`user[client]`) are handled correctly and pre-encoded usernames are not double-encoded. Fixes googleapis#2454. ## Root cause `buildGoOraConnString` built DSNs using `url.URL{Host: connectStringBase, User: url.UserPassword(...)}`. For Oracle connect strings like `host:1521/SERVICE`, putting the full value in `Host` is lossy for URL formatting and brittle for userinfo handling. Also, when a username is already percent-encoded (`user%5Bclient%5D`), passing it directly to `url.UserPassword` can double-encode `%` without normalization. ## Fix - Normalize user/password with `url.PathUnescape` before URL userinfo encoding. - Construct DSN as: - encoded userinfo (`url.UserPassword(...).String()`) - raw Oracle connect string appended after `@` (preserves `host:port/service` shape) - Preserve wallet query args behavior (`ssl=true&wallet=...`). ## Tests Added regression coverage in `internal/sources/oracle/oracle_connstring_test.go`: - bracketed proxy username is encoded correctly. - already percent-encoded proxy username is not double-encoded. - existing wallet/non-wallet DSN cases remain correct. ## Validation - `go test ./internal/sources/oracle -run 'TestBuildGoOraConnString|TestParseFromYamlOracle|TestFailParseFromYaml'` --------- Co-authored-by: DEVELOPER-DEEVEN <144827577+DEVELOPER-DEEVEN@users.noreply.github.com> Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com>
1 parent 9590821 commit b1333cd

File tree

2 files changed

+110
-18
lines changed

2 files changed

+110
-18
lines changed

internal/sources/oracle/oracle.go

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"database/sql"
77
"encoding/json"
88
"fmt"
9+
"net/url"
910
"os"
1011
"strings"
1112

@@ -253,6 +254,41 @@ func (s *Source) RunSQL(ctx context.Context, statement string, params []any, rea
253254
return out, nil
254255
}
255256

257+
func buildGoOraConnString(user, password, connectStringBase, walletLocation string) string {
258+
userInfo := url.UserPassword(
259+
decodePercentEncodedUserInfo(user),
260+
decodePercentEncodedUserInfo(password),
261+
).String()
262+
263+
base := fmt.Sprintf("oracle://%s@%s", userInfo, connectStringBase)
264+
trimmedWalletLocation := strings.TrimSpace(walletLocation)
265+
if trimmedWalletLocation == "" {
266+
return base
267+
}
268+
269+
q := url.Values{}
270+
q.Set("ssl", "true")
271+
q.Set("wallet", trimmedWalletLocation)
272+
273+
separator := "?"
274+
if strings.Contains(connectStringBase, "?") {
275+
separator = "&"
276+
if strings.HasSuffix(base, "?") || strings.HasSuffix(base, "&") {
277+
separator = ""
278+
}
279+
}
280+
281+
return fmt.Sprintf("%s%s%s", base, separator, q.Encode())
282+
}
283+
284+
func decodePercentEncodedUserInfo(value string) string {
285+
decoded, err := url.PathUnescape(value)
286+
if err != nil {
287+
return value
288+
}
289+
return decoded
290+
}
291+
256292
func initOracleConnection(ctx context.Context, tracer trace.Tracer, config Config) (*sql.DB, error) {
257293
//nolint:all // Reassigned ctx
258294
ctx, span := sources.InitConnectionSpan(ctx, tracer, SourceType, config.Name)
@@ -305,16 +341,11 @@ func initOracleConnection(ctx context.Context, tracer trace.Tracer, config Confi
305341
// Use go-ora driver (pure Go)
306342
driverName = "oracle"
307343

308-
user := config.User
309-
password := config.Password
344+
finalConnStr = buildGoOraConnString(config.User, config.Password, connectStringBase, config.WalletLocation)
310345

311346
if hasWallet {
312-
finalConnStr = fmt.Sprintf("oracle://%s:%s@%s?ssl=true&wallet=%s",
313-
user, password, connectStringBase, config.WalletLocation)
347+
logger.DebugContext(ctx, fmt.Sprintf("Using go-ora driver (pure-Go) with wallet and serverString: %s\n", connectStringBase))
314348
} else {
315-
// Standard go-ora connection
316-
finalConnStr = fmt.Sprintf("oracle://%s:%s@%s",
317-
config.User, config.Password, connectStringBase)
318349
logger.DebugContext(ctx, fmt.Sprintf("Using go-ora driver (pure-Go) with serverString: %s\n", connectStringBase))
319350
}
320351
}

internal/sources/oracle/oracle_test.go

Lines changed: 72 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Copyright © 2025, Oracle and/or its affiliates.
22

3-
package oracle_test
3+
package oracle
44

55
import (
66
"context"
@@ -11,7 +11,6 @@ import (
1111
"github.com/google/go-cmp/cmp"
1212
"github.com/googleapis/genai-toolbox/internal/server"
1313
"github.com/googleapis/genai-toolbox/internal/sources"
14-
"github.com/googleapis/genai-toolbox/internal/sources/oracle"
1514
"github.com/googleapis/genai-toolbox/internal/testutils"
1615
)
1716

@@ -33,9 +32,9 @@ func TestParseFromYamlOracle(t *testing.T) {
3332
useOCI: true
3433
`,
3534
want: map[string]sources.SourceConfig{
36-
"my-oracle-cs": oracle.Config{
35+
"my-oracle-cs": Config{
3736
Name: "my-oracle-cs",
38-
Type: oracle.SourceType,
37+
Type: SourceType,
3938
ConnectionString: "my-host:1521/XEPDB1",
4039
User: "my_user",
4140
Password: "my_pass",
@@ -56,9 +55,9 @@ func TestParseFromYamlOracle(t *testing.T) {
5655
password: my_pass
5756
`,
5857
want: map[string]sources.SourceConfig{
59-
"my-oracle-host": oracle.Config{
58+
"my-oracle-host": Config{
6059
Name: "my-oracle-host",
61-
Type: oracle.SourceType,
60+
Type: SourceType,
6261
Host: "my-host",
6362
Port: 1521,
6463
ServiceName: "ORCLPDB",
@@ -81,9 +80,9 @@ func TestParseFromYamlOracle(t *testing.T) {
8180
useOCI: true
8281
`,
8382
want: map[string]sources.SourceConfig{
84-
"my-oracle-tns-oci": oracle.Config{
83+
"my-oracle-tns-oci": Config{
8584
Name: "my-oracle-tns-oci",
86-
Type: oracle.SourceType,
85+
Type: SourceType,
8786
TnsAlias: "FINANCE_DB",
8887
TnsAdmin: "/opt/oracle/network/admin",
8988
User: "my_user",
@@ -106,6 +105,68 @@ func TestParseFromYamlOracle(t *testing.T) {
106105
}
107106
}
108107

108+
func TestBuildGoOraConnString(t *testing.T) {
109+
t.Parallel()
110+
111+
tcs := []struct {
112+
name string
113+
user string
114+
password string
115+
connectBase string
116+
walletLocation string
117+
want string
118+
}{
119+
{
120+
name: "encodes_credentials_and_wallet",
121+
user: "user[client]",
122+
password: "pa:ss@word",
123+
connectBase: "dbhost:1521/XEPDB1",
124+
walletLocation: "/tmp/my wallet",
125+
want: "oracle://user%5Bclient%5D:pa%3Ass%40word@dbhost:1521/XEPDB1?ssl=true&wallet=%2Ftmp%2Fmy+wallet",
126+
},
127+
{
128+
name: "no_wallet",
129+
user: "scott",
130+
password: "tiger",
131+
connectBase: "dbhost:1521/ORCL",
132+
want: "oracle://scott:tiger@dbhost:1521/ORCL",
133+
},
134+
{
135+
name: "does_not_double_encode_percent_encoded_user",
136+
user: "app_user%5BCLIENT_A%5D",
137+
password: "secret",
138+
connectBase: "dbhost:1521/ORCL",
139+
want: "oracle://app_user%5BCLIENT_A%5D:secret@dbhost:1521/ORCL",
140+
},
141+
{
142+
name: "uses_trimmed_wallet_location",
143+
user: "scott",
144+
password: "tiger",
145+
connectBase: "dbhost:1521/ORCL",
146+
walletLocation: " /tmp/wallet ",
147+
want: "oracle://scott:tiger@dbhost:1521/ORCL?ssl=true&wallet=%2Ftmp%2Fwallet",
148+
},
149+
{
150+
name: "appends_wallet_query_to_existing_query",
151+
user: "scott",
152+
password: "tiger",
153+
connectBase: "dbhost:1521/ORCL?custom_opt=true",
154+
walletLocation: " /tmp/wallet ",
155+
want: "oracle://scott:tiger@dbhost:1521/ORCL?custom_opt=true&ssl=true&wallet=%2Ftmp%2Fwallet",
156+
},
157+
}
158+
159+
for _, tc := range tcs {
160+
t.Run(tc.name, func(t *testing.T) {
161+
t.Parallel()
162+
got := buildGoOraConnString(tc.user, tc.password, tc.connectBase, tc.walletLocation)
163+
if got != tc.want {
164+
t.Fatalf("buildGoOraConnString() = %q, want %q", got, tc.want)
165+
}
166+
})
167+
}
168+
}
169+
109170
func TestFailParseFromYaml(t *testing.T) {
110171
tcs := []struct {
111172
desc string
@@ -205,10 +266,10 @@ func TestRunSQLExecutesDML(t *testing.T) {
205266
}
206267
defer db.Close()
207268

208-
src := &oracle.Source{
209-
Config: oracle.Config{
269+
src := &Source{
270+
Config: Config{
210271
Name: "test-dml-source",
211-
Type: oracle.SourceType,
272+
Type: SourceType,
212273
User: "test-user",
213274
},
214275
DB: db,

0 commit comments

Comments
 (0)