refactor postgres connection string building (#27723) (#27869)

Backport #27723 by @mpldr

This patchset changes the connection string builder to use net.URL and
the host/port parser to use the stdlib function for splitting host from
port. It also adds a footnote about a potentially required portnumber
for postgres UNIX sockets.

Fixes: #24552

Co-authored-by: Moritz Poldrack <33086936+mpldr@users.noreply.github.com>

3 files changed, 47 insertions, 30 deletions

diff --git a/docs/content/administration/config-cheat-sheet.en-us.md b/docs/content/administration/config-cheat-sheet.en-us.md
index feea68f212..f0a61851c2 100644
--- a/docs/content/administration/config-cheat-sheet.en-us.md
+++ b/docs/content/administration/config-cheat-sheet.en-us.md
@@ -423,7 +423,7 @@ The following configuration set `Content-Type: application/vnd.android.package-a
 ## Database (`database`)
 
 - `DB_TYPE`: **mysql**: The database type in use \[mysql, postgres, mssql, sqlite3\].
-- `HOST`: **127.0.0.1:3306**: Database host address and port or absolute path for unix socket \[mysql, postgres\] (ex: /var/run/mysqld/mysqld.sock).
+- `HOST`: **127.0.0.1:3306**: Database host address and port or absolute path for unix socket \[mysql, postgres[^1]\] (ex: /var/run/mysqld/mysqld.sock).
 - `NAME`: **gitea**: Database name.
 - `USER`: **root**: Database username.
 - `PASSWD`: **_empty_**: Database user password. Use \`your password\` or """your password""" for quoting if you use special characters in the password.
@@ -454,6 +454,8 @@ The following configuration set `Content-Type: application/vnd.android.package-a
 - `CONN_MAX_LIFETIME` **0 or 3s**: Sets the maximum amount of time a DB connection may be reused - default is 0, meaning there is no limit (except on MySQL where it is 3s - see #6804 & #7071).
 - `AUTO_MIGRATION` **true**: Whether execute database models migrations automatically.
 
+[^1]: It may be necessary to specify a hostport even when listening on a unix socket, as the port is part of the socket name. see [#24552](https://github.com/go-gitea/gitea/issues/24552#issuecomment-1681649367) for additional details.
+
 Please see #8540 & #8273 for further discussion of the appropriate values for `MAX_OPEN_CONNS`, `MAX_IDLE_CONNS` & `CONN_MAX_LIFETIME` and their
 relation to port exhaustion.
 
diff --git a/modules/setting/database.go b/modules/setting/database.go
index 709655368c..aa42f506bc 100644
--- a/modules/setting/database.go
+++ b/modules/setting/database.go
@@ -6,6 +6,7 @@ package setting
 import (
 	"errors"
 	"fmt"
+	"net"
 	"net/url"
 	"os"
 	"path"
@@ -135,15 +136,18 @@ func DBConnStr() (string, error) {
 // parsePostgreSQLHostPort parses given input in various forms defined in
 // https://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-CONNSTRING
 // and returns proper host and port number.
-func parsePostgreSQLHostPort(info string) (string, string) {
-	host, port := "127.0.0.1", "5432"
-	if strings.Contains(info, ":") && !strings.HasSuffix(info, "]") {
-		idx := strings.LastIndex(info, ":")
-		host = info[:idx]
-		port = info[idx+1:]
-	} else if len(info) > 0 {
+func parsePostgreSQLHostPort(info string) (host, port string) {
+	if h, p, err := net.SplitHostPort(info); err == nil {
+		host, port = h, p
+	} else {
+		// treat the "info" as "host", if it's an IPv6 address, remove the wrapper
 		host = info
+		if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") {
+			host = host[1 : len(host)-1]
+		}
 	}
+
+	// set fallback values
 	if host == "" {
 		host = "127.0.0.1"
 	}
@@ -155,14 +159,22 @@ func parsePostgreSQLHostPort(info string) (string, string) {
 
 func getPostgreSQLConnectionString(dbHost, dbUser, dbPasswd, dbName, dbParam, dbsslMode string) (connStr string) {
 	host, port := parsePostgreSQLHostPort(dbHost)
-	if host[0] == '/' { // looks like a unix socket
-		connStr = fmt.Sprintf("postgres://%s:%s@:%s/%s%ssslmode=%s&host=%s",
-			url.PathEscape(dbUser), url.PathEscape(dbPasswd), port, dbName, dbParam, dbsslMode, host)
-	} else {
-		connStr = fmt.Sprintf("postgres://%s:%s@%s:%s/%s%ssslmode=%s",
-			url.PathEscape(dbUser), url.PathEscape(dbPasswd), host, port, dbName, dbParam, dbsslMode)
+	connURL := url.URL{
+		Scheme:   "postgres",
+		User:     url.UserPassword(dbUser, dbPasswd),
+		Host:     net.JoinHostPort(host, port),
+		Path:     dbName,
+		OmitHost: false,
+		RawQuery: dbParam,
+	}
+	query := connURL.Query()
+	if dbHost[0] == '/' { // looks like a unix socket
+		query.Add("host", dbHost)
+		connURL.Host = ":" + port
 	}
-	return connStr
+	query.Set("sslmode", dbsslMode)
+	connURL.RawQuery = query.Encode()
+	return connURL.String()
 }
 
 // ParseMSSQLHostPort splits the host into host and port
diff --git a/modules/setting/database_test.go b/modules/setting/database_test.go
index 481ca969b1..85271c36cb 100644
--- a/modules/setting/database_test.go
+++ b/modules/setting/database_test.go
@@ -10,46 +10,49 @@ import (
 )
 
 func Test_parsePostgreSQLHostPort(t *testing.T) {
-	tests := []struct {
+	tests := map[string]struct {
 		HostPort string
 		Host     string
 		Port     string
 	}{
-		{
+		"host-port": {
 			HostPort: "127.0.0.1:1234",
 			Host:     "127.0.0.1",
 			Port:     "1234",
 		},
-		{
+		"no-port": {
 			HostPort: "127.0.0.1",
 			Host:     "127.0.0.1",
 			Port:     "5432",
 		},
-		{
+		"ipv6-port": {
 			HostPort: "[::1]:1234",
-			Host:     "[::1]",
+			Host:     "::1",
 			Port:     "1234",
 		},
-		{
+		"ipv6-no-port": {
 			HostPort: "[::1]",
-			Host:     "[::1]",
+			Host:     "::1",
 			Port:     "5432",
 		},
-		{
+		"unix-socket": {
 			HostPort: "/tmp/pg.sock:1234",
 			Host:     "/tmp/pg.sock",
 			Port:     "1234",
 		},
-		{
+		"unix-socket-no-port": {
 			HostPort: "/tmp/pg.sock",
 			Host:     "/tmp/pg.sock",
 			Port:     "5432",
 		},
 	}
-	for _, test := range tests {
-		host, port := parsePostgreSQLHostPort(test.HostPort)
-		assert.Equal(t, test.Host, host)
-		assert.Equal(t, test.Port, port)
+	for k, test := range tests {
+		t.Run(k, func(t *testing.T) {
+			t.Log(test.HostPort)
+			host, port := parsePostgreSQLHostPort(test.HostPort)
+			assert.Equal(t, test.Host, host)
+			assert.Equal(t, test.Port, port)
+		})
 	}
 }
 
@@ -72,7 +75,7 @@ func Test_getPostgreSQLConnectionString(t *testing.T) {
 			Name:    "gitea",
 			Param:   "",
 			SSLMode: "false",
-			Output:  "postgres://testuser:space%20space%20%21%23$%25%5E%5E%25%5E%60%60%60-=%3F=@:5432/giteasslmode=false&host=/tmp/pg.sock",
+			Output:  "postgres://testuser:space%20space%20%21%23$%25%5E%5E%25%5E%60%60%60-=%3F=@:5432/gitea?host=%2Ftmp%2Fpg.sock&sslmode=false",
 		},
 		{
 			Host:    "localhost",
@@ -82,7 +85,7 @@ func Test_getPostgreSQLConnectionString(t *testing.T) {
 			Name:    "gitea",
 			Param:   "",
 			SSLMode: "true",
-			Output:  "postgres://pgsqlusername:I%20love%20Gitea%21@localhost:5432/giteasslmode=true",
+			Output:  "postgres://pgsqlusername:I%20love%20Gitea%21@localhost:5432/gitea?sslmode=true",
 		},
 	}