Move some bits to internal/

Move some of the OS abstractions, parsing, and other bits to internal/
package.

Also split out conn.go to rows.go and stmt.go.

Author: arp242

conn.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/lib/pq/internal/pgpass"
 	"github.com/lib/pq/internal/pqtest"
 )
 
@@ -82,57 +83,8 @@ func TestOpenURL(t *testing.T) {
 	testURL("postgresql://")
 }
 
-const pgpassFile = "/tmp/pqgotest_pgpass"
-
 func TestPgpass(t *testing.T) {
-	testAssert := func(conninfo string, expected string, reason string) {
-		conn := pqtest.MustDB(t, conninfo)
-
-		txn, err := conn.Begin()
-		if err != nil {
-			if expected != "fail" {
-				t.Fatalf(reason, err)
-			}
-			return
-		}
-		rows, err := txn.Query("SELECT USER")
-		if err != nil {
-			txn.Rollback()
-			if expected != "fail" {
-				t.Fatalf(reason, err)
-			}
-		} else {
-			rows.Close()
-			if expected != "ok" {
-				t.Fatalf(reason, err)
-			}
-		}
-		txn.Rollback()
-	}
-	testAssert("", "ok", "missing .pgpass, unexpected error %#v")
-	os.Setenv("PGPASSFILE", pgpassFile)
-	defer os.Unsetenv("PGPASSFILE")
-	testAssert("host=/tmp", "fail", ", unexpected error %#v")
-	os.Unsetenv("PGPASSFILE")
-
-	os.Remove(pgpassFile)
-	pgpass, err := os.OpenFile(pgpassFile, os.O_RDWR|os.O_CREATE, 0644)
-	if err != nil {
-		t.Fatalf("Unexpected error writing pgpass file %#v", err)
-	}
-	_, err = pgpass.WriteString(`# comment
-server:5432:some_db:some_user:pass_A
-*:5432:some_db:some_user:pass_B
-localhost:*:*:*:pass_C
-*:*:*:*:pass_fallback
-	`)
-	if err != nil {
-		t.Fatalf("Unexpected error writing pgpass file %#v", err)
-	}
-	defer os.Remove(pgpassFile)
-	pgpass.Close()
-
-	assertPassword := func(extra values, expected string) {
+	assertPassword := func(want string, extra values) {
 		o := values{
 			"host":            "localhost",
 			"sslmode":         "disable",
@@ -146,27 +98,42 @@ localhost:*:*:*:pass_C
 		for k, v := range extra {
 			o[k] = v
 		}
-		(&conn{}).handlePgpass(o)
-		if pw := o["password"]; pw != expected {
-			t.Fatalf("For %v expected %s got %s", extra, expected, pw)
+		have := pgpass.PasswordFromPgpass(o)
+		if have != want {
+			t.Fatalf("wrong password\nhave: %q\nwant: %q", have, want)
 		}
 	}
-	// missing passfile means empty psasword
-	assertPassword(values{"host": "server", "dbname": "some_db", "user": "some_user"}, "")
+
+	file := pqtest.TempFile(t, "pgpass", pqtest.NormalizeIndent(`
+		# comment
+		server:5432:some_db:some_user:pass_A
+		*:5432:some_db:some_user:pass_B
+		localhost:*:*:*:pass_C
+		*:*:*:*:pass_fallback
+	`))
+
+	// Missing passfile means empty password.
+	assertPassword("", values{"host": "server", "dbname": "some_db", "user": "some_user"})
+
 	// wrong permissions for the pgpass file means it should be ignored
-	assertPassword(values{"host": "example.com", "passfile": pgpassFile, "user": "foo"}, "")
-	// fix the permissions and check if it has taken effect
-	os.Chmod(pgpassFile, 0600)
+	assertPassword("", values{"host": "example.com", "passfile": file, "user": "foo"})
+
+	if err := os.Chmod(file, 0600); err != nil { // Fix the permissions
+		t.Fatal(err)
+	}
+
+	assertPassword("pass_A", values{"host": "server", "passfile": file, "dbname": "some_db", "user": "some_user"})
+	assertPassword("pass_fallback", values{"host": "example.com", "passfile": file, "user": "foo"})
+	assertPassword("pass_B", values{"host": "example.com", "passfile": file, "dbname": "some_db", "user": "some_user"})
 
-	assertPassword(values{"host": "server", "passfile": pgpassFile, "dbname": "some_db", "user": "some_user"}, "pass_A")
-	assertPassword(values{"host": "example.com", "passfile": pgpassFile, "user": "foo"}, "pass_fallback")
-	assertPassword(values{"host": "example.com", "passfile": pgpassFile, "dbname": "some_db", "user": "some_user"}, "pass_B")
 	// localhost also matches the default "" and UNIX sockets
-	assertPassword(values{"host": "", "passfile": pgpassFile, "user": "some_user"}, "pass_C")
-	assertPassword(values{"host": "/tmp", "passfile": pgpassFile, "user": "some_user"}, "pass_C")
-	// passfile connection parameter takes precedence
+	assertPassword("pass_C", values{"host": "", "passfile": file, "user": "some_user"})
+	assertPassword("pass_C", values{"host": "/tmp", "passfile": file, "user": "some_user"})
+
+	// Connection parameter takes precedence
 	os.Setenv("PGPASSFILE", "/tmp")
-	assertPassword(values{"host": "server", "passfile": pgpassFile, "dbname": "some_db", "user": "some_user"}, "pass_A")
+	defer os.Unsetenv("PGPASSFILE")
+	assertPassword("pass_A", values{"host": "server", "passfile": file, "dbname": "some_db", "user": "some_user"})
 }
 
 func TestExecNilSlice(t *testing.T) {
@@ -1473,58 +1440,6 @@ func TestRuntimeParameters(t *testing.T) {
 	}
 }
 
-func TestQuoteIdentifier(t *testing.T) {
-	var cases = []struct {
-		input string
-		want  string
-	}{
-		{`foo`, `"foo"`},
-		{`foo bar baz`, `"foo bar baz"`},
-		{`foo"bar`, `"foo""bar"`},
-		{"foo\x00bar", `"foo"`},
-		{"\x00foo", `""`},
-	}
-
-	for _, test := range cases {
-		got := QuoteIdentifier(test.input)
-		if got != test.want {
-			t.Errorf("QuoteIdentifier(%q) = %v want %v", test.input, got, test.want)
-		}
-	}
-}
-
-func TestQuoteLiteral(t *testing.T) {
-	var cases = []struct {
-		input string
-		want  string
-	}{
-		{`foo`, `'foo'`},
-		{`foo bar baz`, `'foo bar baz'`},
-		{`foo'bar`, `'foo''bar'`},
-		{`foo\bar`, ` E'foo\\bar'`},
-		{`foo\ba'r`, ` E'foo\\ba''r'`},
-		{`foo"bar`, `'foo"bar'`},
-		{`foo\x00bar`, ` E'foo\\x00bar'`},
-		{`\x00foo`, ` E'\\x00foo'`},
-		{`'`, `''''`},
-		{`''`, `''''''`},
-		{`\`, ` E'\\'`},
-		{`'abc'; DROP TABLE users;`, `'''abc''; DROP TABLE users;'`},
-		{`\'`, ` E'\\'''`},
-		{`E'\''`, ` E'E''\\'''''`},
-		{`e'\''`, ` E'e''\\'''''`},
-		{`E'\'abc\'; DROP TABLE users;'`, ` E'E''\\''abc\\''; DROP TABLE users;'''`},
-		{`e'\'abc\'; DROP TABLE users;'`, ` E'e''\\''abc\\''; DROP TABLE users;'''`},
-	}
-
-	for _, test := range cases {
-		got := QuoteLiteral(test.input)
-		if got != test.want {
-			t.Errorf("QuoteLiteral(%q) = %v want %v", test.input, got, test.want)
-		}
-	}
-}
-
 func TestRowsResultTag(t *testing.T) {
 	type ResultTag interface {
 		Result() driver.Result

conn_test.go

@@ -6,10 +6,14 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	neturl "net/url"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"unicode"
+
+	"github.com/lib/pq/internal/pqutil"
 )
 
 // Connector represents a fixed configuration for the pq driver with a given
@@ -104,9 +108,9 @@ func NewConnector(dsn string) (*Connector, error) {
 	// resort is to use the current operating system provided user
 	// name.
 	if _, ok := o["user"]; !ok {
-		u, err := userCurrent()
+		u, err := pqutil.User()
 		if err != nil {
-			return nil, err
+			return nil, ErrCouldNotDetectUsername
 		}
 		o["user"] = u
 	}
@@ -245,6 +249,52 @@ func parseOpts(name string, o values) error {
 	return nil
 }
 
+func convertURL(url string) (string, error) {
+	u, err := neturl.Parse(url)
+	if err != nil {
+		return "", err
+	}
+
+	if u.Scheme != "postgres" && u.Scheme != "postgresql" {
+		return "", fmt.Errorf("invalid connection protocol: %s", u.Scheme)
+	}
+
+	var kvs []string
+	escaper := strings.NewReplacer(`'`, `\'`, `\`, `\\`)
+	accrue := func(k, v string) {
+		if v != "" {
+			kvs = append(kvs, k+"='"+escaper.Replace(v)+"'")
+		}
+	}
+
+	if u.User != nil {
+		v := u.User.Username()
+		accrue("user", v)
+
+		v, _ = u.User.Password()
+		accrue("password", v)
+	}
+
+	if host, port, err := net.SplitHostPort(u.Host); err != nil {
+		accrue("host", u.Host)
+	} else {
+		accrue("host", host)
+		accrue("port", port)
+	}
+
+	if u.Path != "" {
+		accrue("dbname", u.Path[1:])
+	}
+
+	q := u.Query()
+	for k := range q {
+		accrue(k, q.Get(k))
+	}
+
+	sort.Strings(kvs) // Makes testing easier (not a performance concern)
+	return strings.Join(kvs, " "), nil
+}
+
 // parseEnviron tries to mimic some of libpq's environment handling
 //
 // To ease testing, it does not directly reference os.Environ, but is
@@ -338,16 +388,14 @@ func parseEnviron(env []string) (out map[string]string) {
 // isUTF8 returns whether name is a fuzzy variation of the string "UTF-8".
 func isUTF8(name string) bool {
 	// Recognize all sorts of silly things as "UTF-8", like Postgres does
-	s := strings.Map(alnumLowerASCII, name)
+	s := strings.Map(func(c rune) rune {
+		if 'A' <= c && c <= 'Z' {
+			return c + ('a' - 'A')
+		}
+		if 'a' <= c && c <= 'z' || '0' <= c && c <= '9' {
+			return c
+		}
+		return -1 // discard
+	}, name)
 	return s == "utf8" || s == "unicode"
 }
-
-func alnumLowerASCII(ch rune) rune {
-	if 'A' <= ch && ch <= 'Z' {
-		return ch + ('a' - 'A')
-	}
-	if 'a' <= ch && ch <= 'z' || '0' <= ch && ch <= '9' {
-		return ch
-	}
-	return -1 // discard
-}

connector.go

@@ -9,6 +9,8 @@ import (
 	"os"
 	"reflect"
 	"testing"
+
+	"github.com/lib/pq/internal/pqtest"
 )
 
 func TestNewConnector_WorksWithOpenDB(t *testing.T) {
@@ -130,3 +132,35 @@ func TestIsUTF8(t *testing.T) {
 		}
 	}
 }
+
+func TestParseURL(t *testing.T) {
+	tests := []struct {
+		in, want, wantErr string
+	}{
+		{"postgres://", "", ""},
+		{"postgres://hostname.remote", "host='hostname.remote'", ""},
+		{"postgres://[::1]:1234", "host='::1' port='1234'", ""},
+		{"postgres://username:top%[email protected]:1234/database",
+			`dbname='database' host='hostname.remote' password='top secret' port='1234' user='username'`, ""},
+		{"postgres://localhost/a%2Fb", "dbname='a/b' host='localhost'", ""},
+
+		{"", "", "invalid connection protocol:"},
+		{"http://hostname.remote", "", "invalid connection protocol: http"},
+
+		//{"postgresql://%2Fvar%2Flib%2Fpostgresql/dbname", "", ``},
+		//{"postgres:// host/db", "dbname='db' host='host'", ""},
+		//{"postgres://host/db ", "dbname='db' host='host'", ""},
+	}
+
+	for _, tt := range tests {
+		t.Run("", func(t *testing.T) {
+			have, err := ParseURL(tt.in)
+			if !pqtest.ErrorContains(err, tt.wantErr) {
+				t.Fatal(err)
+			}
+			if have != tt.want {
+				t.Errorf("\nhave: %q\nwant: %q", have, tt.want)
+			}
+		})
+	}
+}

connector_test.go

@@ -64,4 +64,4 @@ func (e *Error) Get(k byte) (v string) {
 //
 // Deprecated: directly passing an URL to sql.Open("postgres", "postgres://...")
 // now works, and calling this manually is no longer required.
-func ParseURL(url string) (string, error) { return parseURL(url) }
+func ParseURL(url string) (string, error) { return convertURL(url) }

deprecated.go

@@ -92,6 +92,23 @@ func binaryDecode(parameterStatus *parameterStatus, s []byte, typ oid.Oid) any {
 	panic("not reached")
 }
 
+// decodeUUIDBinary interprets the binary format of a uuid, returning it in text format.
+func decodeUUIDBinary(src []byte) ([]byte, error) {
+	if len(src) != 16 {
+		return nil, fmt.Errorf("pq: unable to decode uuid; bad length: %d", len(src))
+	}
+
+	dst := make([]byte, 36)
+	dst[8], dst[13], dst[18], dst[23] = '-', '-', '-', '-'
+	hex.Encode(dst[0:], src[0:4])
+	hex.Encode(dst[9:], src[4:6])
+	hex.Encode(dst[14:], src[6:8])
+	hex.Encode(dst[19:], src[8:10])
+	hex.Encode(dst[24:], src[10:16])
+
+	return dst, nil
+}
+
 func textDecode(parameterStatus *parameterStatus, s []byte, typ oid.Oid) any {
 	switch typ {
 	case oid.T_char, oid.T_bpchar, oid.T_varchar, oid.T_text: