From 782aa7cc1088fa7145e660c9bac3569f3ba667c5 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Thu, 9 Mar 2023 16:20:35 +0100 Subject: [PATCH] pgwire: remove excess call to urldecode Release note (bug fix): Certain special character combinations in the `options` field in connection URLs were not properly supported by CockroachDB. This has been fixed. --- pkg/sql/pgwire/conn_test.go | 23 +++++++++++++++-------- pkg/sql/pgwire/pre_serve_options.go | 11 ++--------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/pkg/sql/pgwire/conn_test.go b/pkg/sql/pgwire/conn_test.go index e61c10ae7fb0..8207d562cb7d 100644 --- a/pkg/sql/pgwire/conn_test.go +++ b/pkg/sql/pgwire/conn_test.go @@ -1570,6 +1570,15 @@ func TestParseClientProvidedSessionParameters(t *testing.T) { require.Equal(t, "ISO,YMD", args.SessionDefaults["datestyle"]) }, }, + { + // Regression test for issue #98301. + desc: "special characters that look like urlencoded must not be decoded during option parsing", + query: "options=-c application_name=%2566%256f%256f", + assert: func(t *testing.T, args sql.SessionArgs, err error) { + require.NoError(t, err) + require.Equal(t, "%66%6f%6f", args.SessionDefaults["application_name"]) + }, + }, } baseURL := fmt.Sprintf("postgres://%s/system?sslmode=disable", serverAddr) @@ -1623,14 +1632,12 @@ func TestSetSessionArguments(t *testing.T) { ) defer cleanupFunc() - q := pgURL.Query() - q.Add("options", " --user=test -c search_path=public,testsp %20 "+ - "--default-transaction-isolation=read\\ uncommitted "+ - "-capplication_name=test "+ - "--DateStyle=ymd\\ ,\\ iso\\ "+ - "-c intervalstyle%3DISO_8601 "+ - "-ccustom_option.custom_option=test2") - pgURL.RawQuery = q.Encode() + pgURL.RawQuery += `&options=` + " --user=test -c search_path=public,testsp %20 " + + "--default-transaction-isolation=read\\ uncommitted " + + "-capplication_name=test " + + "--DateStyle=ymd\\ ,\\ iso\\ " + + "-c intervalstyle%3DISO_8601 " + + "-ccustom_option.custom_option=test2" noBufferDB, err := gosql.Open("postgres", pgURL.String()) if err != nil { diff --git a/pkg/sql/pgwire/pre_serve_options.go b/pkg/sql/pgwire/pre_serve_options.go index c30ee2345d8e..de6490cd810e 100644 --- a/pkg/sql/pgwire/pre_serve_options.go +++ b/pkg/sql/pgwire/pre_serve_options.go @@ -14,7 +14,6 @@ import ( "context" "encoding/base64" "net" - "net/url" "regexp" "strconv" "strings" @@ -267,15 +266,9 @@ type option struct { // parseOptions parses the given string into the options. The options must be // separated by space and have one of the following patterns: // '-c key=value', '-ckey=value', '--key=value' -func parseOptions(optionsString string) ([]option, error) { - var res []option - optionsRaw, err := url.QueryUnescape(optionsString) - if err != nil { - return nil, pgerror.Newf(pgcode.ProtocolViolation, "failed to unescape options %q", optionsString) - } - +func parseOptions(optionsString string) (res []option, err error) { lastWasDashC := false - opts := splitOptions(optionsRaw) + opts := splitOptions(optionsString) for i := 0; i < len(opts); i++ { prefix := ""