From 9b6e77c489edf3af3a9782c977cc04fde7aa2fd9 Mon Sep 17 00:00:00 2001 From: Moritz Poldrack <33086936+mpldr@users.noreply.github.com> Date: Wed, 1 Nov 2023 19:00:20 +0100 Subject: [PATCH] refactor postgres connection string building (#27723) 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 --- .../config-cheat-sheet.en-us.md | 4 +- modules/setting/database.go | 40 ++++++++++++------- modules/setting/database_test.go | 33 ++++++++------- 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 617715fba..715bd6d32 100644 --- a/docs/content/administration/config-cheat-sheet.en-us.md +++ b/docs/content/administration/config-cheat-sheet.en-us.md @@ -424,7 +424,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. @@ -455,6 +455,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 709655368..aa42f506b 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, } - return connStr + query := connURL.Query() + if dbHost[0] == '/' { // looks like a unix socket + query.Add("host", dbHost) + connURL.Host = ":" + port + } + 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 481ca969b..85271c36c 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", }, }