refactor uri parsing, log error when database index is provided incorrectly

Author: jsievenpiper

modules/nosql/manager_redis.go

@@ -11,6 +11,8 @@ import (
 	"strconv"
 	"strings"
 
+	"code.gitea.io/gitea/modules/log"
+
 	"github.com/go-redis/redis/v8"
 )
 
@@ -76,30 +78,13 @@ func (m *Manager) GetRedisClient(connection string) redis.UniversalClient {
 		opts.TLSConfig = tlsConfig
 		fallthrough
 	case "redis+sentinel":
-		if uri.Host != "" {
-			opts.Addrs = append(opts.Addrs, strings.Split(uri.Host, ",")...)
-		}
-		if uri.Path != "" {
-			if db, err := strconv.Atoi(uri.Path[1:]); err == nil {
-				opts.DB = db
-			}
-		}
-
 		client.UniversalClient = redis.NewFailoverClient(opts.Failover())
 	case "redis+clusters":
 		fallthrough
 	case "rediss+cluster":
 		opts.TLSConfig = tlsConfig
 		fallthrough
 	case "redis+cluster":
-		if uri.Host != "" {
-			opts.Addrs = append(opts.Addrs, strings.Split(uri.Host, ",")...)
-		}
-		if uri.Path != "" {
-			if db, err := strconv.Atoi(uri.Path[1:]); err == nil {
-				opts.DB = db
-			}
-		}
 		client.UniversalClient = redis.NewClusterClient(opts.Cluster())
 	case "redis+socket":
 		simpleOpts := opts.Simple()
@@ -110,14 +95,6 @@ func (m *Manager) GetRedisClient(connection string) redis.UniversalClient {
 		opts.TLSConfig = tlsConfig
 		fallthrough
 	case "redis":
-		if uri.Host != "" {
-			opts.Addrs = append(opts.Addrs, strings.Split(uri.Host, ",")...)
-		}
-		if uri.Path != "" {
-			if db, err := strconv.Atoi(uri.Path[1:]); err == nil {
-				opts.DB = db
-			}
-		}
 		client.UniversalClient = redis.NewClient(opts.Simple())
 	default:
 		return nil
@@ -215,6 +192,22 @@ func getRedisOptions(uri *url.URL) *redis.UniversalOptions {
 		}
 	}
 
+	if uri.Host != "" {
+		opts.Addrs = append(opts.Addrs, strings.Split(uri.Host, ",")...)
+	}
+
+	// A redis connection string uses the path section of the URI in two different ways. In a TCP-based connection, the
+	// path will be a database index to automatically have the client SELECT. In a Unix socket connection, it will be the
+	// file path. We only want to try to coerce this to the database index when we're not expecting a file path so that
+	// the error log stays clean.
+	if uri.Path != "" && uri.Scheme != "redis+socket" {
+		if db, err := strconv.Atoi(uri.Path[1:]); err == nil {
+			opts.DB = db
+		} else {
+			log.Error("Provided database identifier '%s' is not a valid integer. Gitea will ignore this option.", uri.Path)
+		}
+	}
+
 	return opts
 }
 

modules/nosql/manager_redis_test.go

@@ -13,7 +13,7 @@ func TestRedisUsernameOpt(t *testing.T) {
 	uri, _ := url.Parse("redis://redis:password@myredis/0")
 	opts := getRedisOptions(uri)
 
-	if "redis" != opts.Username {
+	if opts.Username != "redis" {
 		t.Fail()
 	}
 }
@@ -22,7 +22,7 @@ func TestRedisPasswordOpt(t *testing.T) {
 	uri, _ := url.Parse("redis://redis:password@myredis/0")
 	opts := getRedisOptions(uri)
 
-	if "password" != opts.Password {
+	if opts.Password != "password" {
 		t.Fail()
 	}
 }
@@ -31,7 +31,7 @@ func TestRedisSentinelUsernameOpt(t *testing.T) {
 	uri, _ := url.Parse("redis+sentinel://redis:password@myredis/0?sentinelusername=suser&sentinelpassword=spass")
 	opts := getRedisOptions(uri).Failover()
 
-	if "suser" != opts.SentinelUsername {
+	if opts.SentinelUsername != "suser" {
 		t.Fail()
 	}
 }
@@ -40,7 +40,25 @@ func TestRedisSentinelPasswordOpt(t *testing.T) {
 	uri, _ := url.Parse("redis+sentinel://redis:password@myredis/0?sentinelusername=suser&sentinelpassword=spass")
 	opts := getRedisOptions(uri).Failover()
 
-	if "spass" != opts.SentinelPassword {
+	if opts.SentinelPassword != "spass" {
+		t.Fail()
+	}
+}
+
+func TestRedisDatabaseIndexTcp(t *testing.T) {
+	uri, _ := url.Parse("redis://redis:password@myredis/12")
+	opts := getRedisOptions(uri)
+
+	if opts.DB != 12 {
+		t.Fail()
+	}
+}
+
+func TestRedisDatabaseIndexUnix(t *testing.T) {
+	uri, _ := url.Parse("redis+socket:///var/run/redis.sock?database=12")
+	opts := getRedisOptions(uri)
+
+	if opts.DB != 12 {
 		t.Fail()
 	}
 }