Skip to content

Commit

Permalink
pgwire: remove excess call to urldecode
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
knz committed Mar 9, 2023
1 parent 1b162d1 commit 782aa7c
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 17 deletions.
23 changes: 15 additions & 8 deletions pkg/sql/pgwire/conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
11 changes: 2 additions & 9 deletions pkg/sql/pgwire/pre_serve_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"context"
"encoding/base64"
"net"
"net/url"
"regexp"
"strconv"
"strings"
Expand Down Expand Up @@ -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 := ""
Expand Down

0 comments on commit 782aa7c

Please sign in to comment.