SQL providers: make sure we don't exceed the allowed placeholders

Fixes #1415

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>

go.mod

@@ -74,12 +74,12 @@ require (
 	golang.org/x/sys v0.12.0
 	golang.org/x/term v0.12.0
 	golang.org/x/time v0.3.0
-	google.golang.org/api v0.139.0
+	google.golang.org/api v0.140.0
 	gopkg.in/natefinch/lumberjack.v2 v2.2.1
 )
 
 require (
-	cloud.google.com/go v0.110.7 // indirect
+	cloud.google.com/go v0.110.8 // indirect
 	cloud.google.com/go/compute v1.23.0 // indirect
 	cloud.google.com/go/compute/metadata v0.2.3 // indirect
 	cloud.google.com/go/iam v1.1.2 // indirect
@@ -135,7 +135,7 @@ require (
 	github.com/mattn/go-colorable v0.1.13 // indirect
 	github.com/mattn/go-isatty v0.0.19 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
-	github.com/miekg/dns v1.1.55 // indirect
+	github.com/miekg/dns v1.1.56 // indirect
 	github.com/minio/sha256-simd v1.0.1 // indirect
 	github.com/mitchellh/go-testing-interface v1.14.1 // indirect
 	github.com/mitchellh/mapstructure v1.5.0 // indirect
@@ -162,9 +162,9 @@ require (
 	golang.org/x/tools v0.13.0 // indirect
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
 	google.golang.org/appengine v1.6.8 // indirect
-	google.golang.org/genproto v0.0.0-20230822172742-b8732ec3820d // indirect
-	google.golang.org/genproto/googleapis/api v0.0.0-20230822172742-b8732ec3820d // indirect
-	google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d // indirect
+	google.golang.org/genproto v0.0.0-20230911183012-2d3300fd4832 // indirect
+	google.golang.org/genproto/googleapis/api v0.0.0-20230911183012-2d3300fd4832 // indirect
+	google.golang.org/genproto/googleapis/rpc v0.0.0-20230911183012-2d3300fd4832 // indirect
 	google.golang.org/grpc v1.58.0 // indirect
 	google.golang.org/protobuf v1.31.0 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect

go.sum

@@ -17,8 +17,8 @@ cloud.google.com/go v0.65.0/go.mod h1:O5N8zS7uWy9vkA9vayVHs65eM1ubvY4h553ofrNHOb
 cloud.google.com/go v0.72.0/go.mod h1:M+5Vjvlc2wnp6tjzE102Dw08nGShTscUx2nZMufOKPI=
 cloud.google.com/go v0.74.0/go.mod h1:VV1xSbzvo+9QJOxLDaJfTjx5e+MePCpCWwvftOeQmWk=
 cloud.google.com/go v0.75.0/go.mod h1:VGuuCn7PG0dwsd5XPVm2Mm3wlh3EL55/79EKB6hlPTY=
-cloud.google.com/go v0.110.7 h1:rJyC7nWRg2jWGZ4wSJ5nY65GTdYJkg0cd/uXb+ACI6o=
-cloud.google.com/go v0.110.7/go.mod h1:+EYjdK8e5RME/VY/qLCAtuyALQ9q67dvuum8i+H5xsI=
+cloud.google.com/go v0.110.8 h1:tyNdfIxjzaWctIiLYOTalaLKZ17SI44SKFW26QbOhME=
+cloud.google.com/go v0.110.8/go.mod h1:Iz8AkXJf1qmxC3Oxoep8R1T36w8B92yU29PcBhHO5fk=
 cloud.google.com/go/bigquery v1.0.1/go.mod h1:i/xbL2UlR5RvWAURpBYZTtm/cXjCha9lbfbpx4poX+o=
 cloud.google.com/go/bigquery v1.3.0/go.mod h1:PjpwJnslEMmckchkHFfq+HTD2DmtT67aNFKH1/VBDHE=
 cloud.google.com/go/bigquery v1.4.0/go.mod h1:S8dzgnTigyfTmLBfrtrhyYhwRxG72rYxvftPBK2Dvzc=
@@ -377,8 +377,8 @@ github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zk
 github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4=
 github.com/mhale/smtpd v0.8.0 h1:5JvdsehCg33PQrZBvFyDMMUDQmvbzVpZgKob7eYBJc0=
 github.com/mhale/smtpd v0.8.0/go.mod h1:MQl+y2hwIEQCXtNhe5+55n0GZOjSmeqORDIXbqUL3x4=
-github.com/miekg/dns v1.1.55 h1:GoQ4hpsj0nFLYe+bWiCToyrBEJXkQfOOIvFGFy0lEgo=
-github.com/miekg/dns v1.1.55/go.mod h1:uInx36IzPl7FYnDcMeVWxj9byh7DutNykX4G9Sj60FY=
+github.com/miekg/dns v1.1.56 h1:5imZaSeoRNvpM9SzWNhEcP9QliKiz20/dA2QabIGVnE=
+github.com/miekg/dns v1.1.56/go.mod h1:cRm6Oo2C8TY9ZS/TqsSrseAcncm74lfK5G+ikN2SWWY=
 github.com/minio/sha256-simd v1.0.1 h1:6kaan5IFmwTNynnKKpDHe6FWHohJOHhCPchzK49dzMM=
 github.com/minio/sha256-simd v1.0.1/go.mod h1:Pz6AKMiUdngCLpeTL/RJY1M9rUuPMYujV5xJjtbRSN8=
 github.com/minio/sio v0.3.1 h1:d59r5RTHb1OsQaSl1EaTWurzMMDRLA5fgNmjzD4eVu4=
@@ -768,8 +768,8 @@ google.golang.org/api v0.30.0/go.mod h1:QGmEvQ87FHZNiUVJkT14jQNYJ4ZJjdRF23ZXz513
 google.golang.org/api v0.35.0/go.mod h1:/XrVsuzM0rZmrsbjJutiuftIzeuTQcEeaYcSk/mQ1dg=
 google.golang.org/api v0.36.0/go.mod h1:+z5ficQTmoYpPn8LCUNVpK5I7hwkpjbcgqA7I34qYtE=
 google.golang.org/api v0.40.0/go.mod h1:fYKFpnQN0DsDSKRVRcQSDQNtqWPfM9i+zNPxepjRCQ8=
-google.golang.org/api v0.139.0 h1:A1TrCPgMmOiYu0AiNkvQIpIx+D8blHTDcJ5EogkP7LI=
-google.golang.org/api v0.139.0/go.mod h1:CVagp6Eekz9CjGZ718Z+sloknzkDJE7Vc1Ckj9+viBk=
+google.golang.org/api v0.140.0 h1:CaXNdYOH5oQQI7l6iKTHHiMTdxZca4/02hRg2U8c2hM=
+google.golang.org/api v0.140.0/go.mod h1:aGbCiFgtwb2P6badchFbSBUurV6oR5d50Af4iNJtDdI=
 google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
 google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
 google.golang.org/appengine v1.5.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
@@ -815,12 +815,12 @@ google.golang.org/genproto v0.0.0-20201210142538-e3217bee35cc/go.mod h1:FWY/as6D
 google.golang.org/genproto v0.0.0-20201214200347-8c77b98c765d/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
 google.golang.org/genproto v0.0.0-20210108203827-ffc7fda8c3d7/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
 google.golang.org/genproto v0.0.0-20210226172003-ab064af71705/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
-google.golang.org/genproto v0.0.0-20230822172742-b8732ec3820d h1:VBu5YqKPv6XiJ199exd8Br+Aetz+o08F+PLMnwJQHAY=
-google.golang.org/genproto v0.0.0-20230822172742-b8732ec3820d/go.mod h1:yZTlhN0tQnXo3h00fuXNCxJdLdIdnVFVBaRJ5LWBbw4=
-google.golang.org/genproto/googleapis/api v0.0.0-20230822172742-b8732ec3820d h1:DoPTO70H+bcDXcd39vOqb2viZxgqeBeSGtZ55yZU4/Q=
-google.golang.org/genproto/googleapis/api v0.0.0-20230822172742-b8732ec3820d/go.mod h1:KjSP20unUpOx5kyQUFa7k4OJg0qeJ7DEZflGDu2p6Bk=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d h1:uvYuEyMHKNt+lT4K3bN6fGswmK8qSvcreM3BwjDh+y4=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d/go.mod h1:+Bk1OCOj40wS2hwAMA+aCW9ypzm63QTBBHp6lQ3p+9M=
+google.golang.org/genproto v0.0.0-20230911183012-2d3300fd4832 h1:/30npZKtUjXqju7ZA2MsvpkGKD4mQFtf+zPnZasABjg=
+google.golang.org/genproto v0.0.0-20230911183012-2d3300fd4832/go.mod h1:yZTlhN0tQnXo3h00fuXNCxJdLdIdnVFVBaRJ5LWBbw4=
+google.golang.org/genproto/googleapis/api v0.0.0-20230911183012-2d3300fd4832 h1:4E7rZzBdR5LmiZx6n47Dg4AjH8JLhMQWywsYqvXNLcs=
+google.golang.org/genproto/googleapis/api v0.0.0-20230911183012-2d3300fd4832/go.mod h1:KjSP20unUpOx5kyQUFa7k4OJg0qeJ7DEZflGDu2p6Bk=
+google.golang.org/genproto/googleapis/rpc v0.0.0-20230911183012-2d3300fd4832 h1:o4LtQxebKIJ4vkzyhtD2rfUNZ20Zf0ik5YVP5E7G7VE=
+google.golang.org/genproto/googleapis/rpc v0.0.0-20230911183012-2d3300fd4832/go.mod h1:+Bk1OCOj40wS2hwAMA+aCW9ypzm63QTBBHp6lQ3p+9M=
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
 google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38=
 google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM=

internal/common/common_test.go

@@ -1644,6 +1644,112 @@ func TestIPList(t *testing.T) {
 	}
 }
 
+func TestSQLPlaceholderLimits(t *testing.T) {
+	numGroups := 120
+	numUsers := 120
+	var groupMapping []sdk.GroupMapping
+
+	folder := vfs.BaseVirtualFolder{
+		Name:       "testfolder",
+		MappedPath: filepath.Join(os.TempDir(), "folder"),
+	}
+	err := dataprovider.AddFolder(&folder, "", "", "")
+	assert.NoError(t, err)
+
+	for i := 0; i < numGroups; i++ {
+		group := dataprovider.Group{
+			BaseGroup: sdk.BaseGroup{
+				Name: fmt.Sprintf("testgroup%d", i),
+			},
+			UserSettings: dataprovider.GroupUserSettings{
+				BaseGroupUserSettings: sdk.BaseGroupUserSettings{
+					Permissions: map[string][]string{
+						fmt.Sprintf("/dir%d", i): {dataprovider.PermAny},
+					},
+				},
+			},
+		}
+		group.VirtualFolders = append(group.VirtualFolders, vfs.VirtualFolder{
+			BaseVirtualFolder: folder,
+			VirtualPath:       "/vdir",
+		})
+		err := dataprovider.AddGroup(&group, "", "", "")
+		assert.NoError(t, err)
+
+		groupMapping = append(groupMapping, sdk.GroupMapping{
+			Name: group.Name,
+			Type: sdk.GroupTypeSecondary,
+		})
+	}
+
+	user := dataprovider.User{
+		BaseUser: sdk.BaseUser{
+			Username: "testusername",
+			HomeDir:  filepath.Join(os.TempDir(), "testhome"),
+			Status:   1,
+			Permissions: map[string][]string{
+				"/": {dataprovider.PermAny},
+			},
+		},
+		Groups: groupMapping,
+	}
+	err = dataprovider.AddUser(&user, "", "", "")
+	assert.NoError(t, err)
+
+	users, err := dataprovider.GetUsersForQuotaCheck(map[string]bool{user.Username: true})
+	assert.NoError(t, err)
+	if assert.Len(t, users, 1) {
+		for i := 0; i < numGroups; i++ {
+			_, ok := users[0].Permissions[fmt.Sprintf("/dir%d", i)]
+			assert.True(t, ok)
+		}
+	}
+
+	err = dataprovider.DeleteUser(user.Username, "", "", "")
+	assert.NoError(t, err)
+
+	for i := 0; i < numUsers; i++ {
+		user := dataprovider.User{
+			BaseUser: sdk.BaseUser{
+				Username: fmt.Sprintf("testusername%d", i),
+				HomeDir:  filepath.Join(os.TempDir()),
+				Status:   1,
+				Permissions: map[string][]string{
+					"/": {dataprovider.PermAny},
+				},
+			},
+			Groups: []sdk.GroupMapping{
+				{
+					Name: "testgroup0",
+					Type: sdk.GroupTypePrimary,
+				},
+			},
+		}
+		err := dataprovider.AddUser(&user, "", "", "")
+		assert.NoError(t, err)
+	}
+
+	time.Sleep(100 * time.Millisecond)
+
+	err = dataprovider.DeleteFolder(folder.Name, "", "", "")
+	assert.NoError(t, err)
+
+	for i := 0; i < numUsers; i++ {
+		username := fmt.Sprintf("testusername%d", i)
+		user, err := dataprovider.UserExists(username, "")
+		assert.NoError(t, err)
+		assert.Greater(t, user.UpdatedAt, user.CreatedAt)
+		err = dataprovider.DeleteUser(username, "", "", "")
+		assert.NoError(t, err)
+	}
+
+	for i := 0; i < numGroups; i++ {
+		groupName := fmt.Sprintf("testgroup%d", i)
+		err = dataprovider.DeleteGroup(groupName, "", "", "")
+		assert.NoError(t, err)
+	}
+}
+
 func BenchmarkBcryptHashing(b *testing.B) {
 	bcryptPassword := "bcryptpassword"
 	for i := 0; i < b.N; i++ {

internal/common/transferschecker_test.go

@@ -597,7 +597,7 @@ func TestDataTransferExceeded(t *testing.T) {
 
 func TestGetUsersForQuotaCheck(t *testing.T) {
 	usersToFetch := make(map[string]bool)
-	for i := 0; i < 50; i++ {
+	for i := 0; i < 70; i++ {
 		usersToFetch[fmt.Sprintf("user%v", i)] = i%2 == 0
 	}
 
@@ -605,7 +605,7 @@ func TestGetUsersForQuotaCheck(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Len(t, users, 0)
 
-	for i := 0; i < 40; i++ {
+	for i := 0; i < 60; i++ {
 		folder := vfs.BaseVirtualFolder{
 			Name:       fmt.Sprintf("f%v", i),
 			MappedPath: filepath.Join(os.TempDir(), fmt.Sprintf("f%v", i)),
@@ -641,7 +641,7 @@ func TestGetUsersForQuotaCheck(t *testing.T) {
 
 	users, err = dataprovider.GetUsersForQuotaCheck(usersToFetch)
 	assert.NoError(t, err)
-	assert.Len(t, users, 40)
+	assert.Len(t, users, 60)
 
 	for _, user := range users {
 		userIdxStr := strings.Replace(user.Username, "user", "", 1)
@@ -665,7 +665,7 @@ func TestGetUsersForQuotaCheck(t *testing.T) {
 		assert.Equal(t, int64(0), total)
 	}
 
-	for i := 0; i < 40; i++ {
+	for i := 0; i < 60; i++ {
 		err = dataprovider.DeleteUser(fmt.Sprintf("user%v", i), "", "", "")
 		assert.NoError(t, err)
 		err = dataprovider.DeleteFolder(fmt.Sprintf("f%v", i), "", "", "")

internal/dataprovider/sqlcommon.go

@@ -958,63 +958,86 @@ func sqlCommonGetUsersInGroups(names []string, dbHandle sqlQuerier) ([]string, e
 	if len(names) == 0 {
 		return nil, nil
 	}
+	maxNames := len(sqlPlaceholders)
+	usernames := make([]string, 0, len(names))
+
 	ctx, cancel := context.WithTimeout(context.Background(), defaultSQLQueryTimeout)
 	defer cancel()
 
-	q := getUsersInGroupsQuery(len(names))
-	args := make([]any, 0, len(names))
-	for _, name := range names {
-		args = append(args, name)
-	}
+	for len(names) > 0 {
+		if maxNames > len(names) {
+			maxNames = len(names)
+		}
 
-	usernames := make([]string, 0, len(names))
-	rows, err := dbHandle.QueryContext(ctx, q, args...)
-	if err != nil {
-		return nil, err
-	}
-	defer rows.Close()
+		q := getUsersInGroupsQuery(maxNames)
+		args := make([]any, 0, maxNames)
+		for _, name := range names[:maxNames] {
+			args = append(args, name)
+		}
 
-	for rows.Next() {
-		var username string
-		err = rows.Scan(&username)
+		rows, err := dbHandle.QueryContext(ctx, q, args...)
+		if err != nil {
+			return nil, err
+		}
+		defer rows.Close()
+
+		for rows.Next() {
+			var username string
+			err = rows.Scan(&username)
+			if err != nil {
+				return usernames, err
+			}
+			usernames = append(usernames, username)
+		}
+		err = rows.Err()
 		if err != nil {
 			return usernames, err
 		}
-		usernames = append(usernames, username)
+		names = names[maxNames:]
 	}
-	return usernames, rows.Err()
+	return usernames, nil
 }
 
 func sqlCommonGetGroupsWithNames(names []string, dbHandle sqlQuerier) ([]Group, error) {
 	if len(names) == 0 {
 		return nil, nil
 	}
-	ctx, cancel := context.WithTimeout(context.Background(), defaultSQLQueryTimeout)
-	defer cancel()
-
-	q := getGroupsWithNamesQuery(len(names))
-	args := make([]any, 0, len(names))
-	for _, name := range names {
-		args = append(args, name)
-	}
+	maxNames := len(sqlPlaceholders)
 	groups := make([]Group, 0, len(names))
-	rows, err := dbHandle.QueryContext(ctx, q, args...)
-	if err != nil {
-		return groups, err
-	}
-	defer rows.Close()
+	for len(names) > 0 {
+		if maxNames > len(names) {
+			maxNames = len(names)
+		}
+		ctx, cancel := context.WithTimeout(context.Background(), defaultSQLQueryTimeout)
+		defer cancel()
 
-	for rows.Next() {
-		group, err := getGroupFromDbRow(rows)
+		q := getGroupsWithNamesQuery(maxNames)
+		args := make([]any, 0, maxNames)
+		for _, name := range names[:maxNames] {
+			args = append(args, name)
+		}
+		rows, err := dbHandle.QueryContext(ctx, q, args...)
 		if err != nil {
 			return groups, err
 		}
-		groups = append(groups, group)
-	}
-	err = rows.Err()
-	if err != nil {
-		return groups, err
+		defer rows.Close()
+
+		for rows.Next() {
+			group, err := getGroupFromDbRow(rows)
+			if err != nil {
+				return groups, err
+			}
+			groups = append(groups, group)
+		}
+		err = rows.Err()
+		if err != nil {
+			return groups, err
+		}
+		names = names[maxNames:]
 	}
+	ctx, cancel := context.WithTimeout(context.Background(), defaultSQLQueryTimeout)
+	defer cancel()
+
 	return getGroupsWithVirtualFolders(ctx, groups, dbHandle)
 }
 
@@ -1535,6 +1558,9 @@ func sqlCommonGetRecentlyUpdatedUsers(after int64, dbHandle sqlQuerier) ([]User,
 		}
 	}
 	groupNames = util.RemoveDuplicates(groupNames, false)
+	if len(groupNames) == 0 {
+		return users, nil
+	}
 	groups, err := sqlCommonGetGroupsWithNames(groupNames, dbHandle)
 	if err != nil {
 		return users, err
@@ -1553,15 +1579,23 @@ func sqlCommonGetRecentlyUpdatedUsers(after int64, dbHandle sqlQuerier) ([]User,
 	return users, nil
 }
 
+func sqlGetMaxUsersForQuotaCheckRange() int {
+	maxUsers := 50
+	if maxUsers > len(sqlPlaceholders) {
+		maxUsers = len(sqlPlaceholders)
+	}
+	return maxUsers
+}
+
 func sqlCommonGetUsersForQuotaCheck(toFetch map[string]bool, dbHandle sqlQuerier) ([]User, error) {
-	users := make([]User, 0, 30)
+	maxUsers := sqlGetMaxUsersForQuotaCheckRange()
+	users := make([]User, 0, maxUsers)
 
 	usernames := make([]string, 0, len(toFetch))
 	for k := range toFetch {
 		usernames = append(usernames, k)
 	}
 
-	maxUsers := 30
 	for len(usernames) > 0 {
 		if maxUsers > len(usernames) {
 			maxUsers = len(usernames)

internal/dataprovider/sqlqueries.go

@@ -42,7 +42,7 @@ const (
 
 func getSQLPlaceholders() []string {
 	var placeholders []string
-	for i := 1; i <= 50; i++ {
+	for i := 1; i <= 100; i++ {
 		if config.Driver == PGSQLDataProviderName || config.Driver == CockroachDataProviderName {
 			placeholders = append(placeholders, fmt.Sprintf("$%d", i))
 		} else {
@@ -400,7 +400,7 @@ func getUsersInGroupsQuery(numArgs int) string {
 	} else {
 		sb.WriteString("('')")
 	}
-	return fmt.Sprintf(`SELECT username FROM %s WHERE id IN (SELECT user_id from %s WHERE group_id IN (SELECT id FROM %s WHERE name IN (%s)))`,
+	return fmt.Sprintf(`SELECT username FROM %s WHERE id IN (SELECT user_id from %s WHERE group_id IN (SELECT id FROM %s WHERE name IN %s))`,
 		sqlTableUsers, sqlTableUsersGroupsMapping, getSQLQuotedName(sqlTableGroups), sb.String())
 }
 

internal/dataprovider/user.go

@@ -1835,6 +1835,9 @@ func (u *User) mergeVirtualFolders(group *Group, groupType int, replacer *string
 }
 
 func (u *User) mergePermissions(group *Group, groupType int, replacer *strings.Replacer) {
+	if u.Permissions == nil {
+		u.Permissions = make(map[string][]string)
+	}
 	for k, v := range group.UserSettings.Permissions {
 		if k == "/" {
 			if groupType == sdk.GroupTypePrimary {

internal/httpd/webadmin.go

@@ -122,7 +122,7 @@ const (
 	pageForgotPwdTitle       = "SFTPGo Admin - Forgot password"
 	pageResetPwdTitle        = "SFTPGo Admin - Reset password"
 	pageSetupTitle           = "Create first admin user"
-	defaultQueryLimit        = 500
+	defaultQueryLimit        = 1000
 	inversePatternType       = "inverse"
 )