From a5247f29e021512005225736fa19835394de681a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 13 Jun 2023 20:39:37 +0800 Subject: [PATCH 1/6] Fix index generation parallelly failure --- models/db/index.go | 29 +++++++++++++++++++++++++++++ models/git/commit_status.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/models/db/index.go b/models/db/index.go index 259ddd6ade7a..4c7e4be9c9ad 100644 --- a/models/db/index.go +++ b/models/db/index.go @@ -89,6 +89,33 @@ func mysqlGetNextResourceIndex(ctx context.Context, tableName string, groupID in return idx, nil } +func mssqlGetNextResourceIndex(ctx context.Context, tableName string, groupID int64) (int64, error) { + if _, err := GetEngine(ctx).Exec(fmt.Sprintf(` +MERGE INTO %s WITH (HOLDLOCK) AS target +USING (SELECT %d AS group_id) AS source +(group_id) +ON target.group_id = source.group_id +WHEN MATCHED + THEN UPDATE + SET max_index = max_index + 1 +WHEN NOT MATCHED + THEN INSERT (group_id, max_index) + VALUES (%d, 1)) +`, tableName, groupID, groupID)); err != nil { + return 0, err + } + + var idx int64 + _, err := GetEngine(ctx).SQL(fmt.Sprintf("SELECT max_index FROM %s WHERE group_id = ?", tableName), groupID).Get(&idx) + if err != nil { + return 0, err + } + if idx == 0 { + return 0, errors.New("cannot get the correct index") + } + return idx, nil +} + // GetNextResourceIndex generates a resource index, it must run in the same transaction where the resource is created func GetNextResourceIndex(ctx context.Context, tableName string, groupID int64) (int64, error) { switch { @@ -96,6 +123,8 @@ func GetNextResourceIndex(ctx context.Context, tableName string, groupID int64) return postgresGetNextResourceIndex(ctx, tableName, groupID) case setting.Database.Type.IsMySQL(): return mysqlGetNextResourceIndex(ctx, tableName, groupID) + case setting.Database.Type.IsMSSQL(): + return mssqlGetNextResourceIndex(ctx, tableName, groupID) } e := GetEngine(ctx) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index a018bb0553a4..92f1017a2d43 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -83,6 +83,34 @@ func mysqlGetCommitStatusIndex(ctx context.Context, repoID int64, sha string) (i return idx, nil } +func mssqlGetCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { + if _, err := db.GetEngine(ctx).Exec(fmt.Sprintf(` +MERGE INTO commit_status_index WITH (HOLDLOCK) AS target +USING (SELECT %d AS repo_id, %s AS sha) AS source +(repo_id, sha) +ON target.repo_id = source.repo_id AND target.sha = source.sha +WHEN MATCHED + THEN UPDATE + SET max_index = max_index + 1 +WHEN NOT MATCHED + THEN INSERT (repo_id, sha, max_index) + VALUES (%d, %s, 1)) +`, repoID, sha, repoID, sha)); err != nil { + return 0, err + } + + var idx int64 + _, err := db.GetEngine(ctx).SQL("SELECT max_index FROM `commit_status_index` WHERE repo_id = ? AND sha = ?", + repoID, sha).Get(&idx) + if err != nil { + return 0, err + } + if idx == 0 { + return 0, errors.New("cannot get the correct index") + } + return idx, nil +} + // GetNextCommitStatusIndex retried 3 times to generate a resource index func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { switch { @@ -90,6 +118,8 @@ func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (in return postgresGetCommitStatusIndex(ctx, repoID, sha) case setting.Database.Type.IsMySQL(): return mysqlGetCommitStatusIndex(ctx, repoID, sha) + case setting.Database.Type.IsMSSQL(): + return mssqlGetCommitStatusIndex(ctx, repoID, sha) } e := db.GetEngine(ctx) From 6a6068f051bc03ad40979b2122821b2dc7e3e019 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 13 Jun 2023 20:59:47 +0800 Subject: [PATCH 2/6] Fix bug --- models/db/index.go | 2 +- models/git/commit_status.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/db/index.go b/models/db/index.go index 4c7e4be9c9ad..7fb3c83403ed 100644 --- a/models/db/index.go +++ b/models/db/index.go @@ -100,7 +100,7 @@ WHEN MATCHED SET max_index = max_index + 1 WHEN NOT MATCHED THEN INSERT (group_id, max_index) - VALUES (%d, 1)) + VALUES (%d, 1) `, tableName, groupID, groupID)); err != nil { return 0, err } diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 92f1017a2d43..d8360d6bde19 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -94,7 +94,7 @@ WHEN MATCHED SET max_index = max_index + 1 WHEN NOT MATCHED THEN INSERT (repo_id, sha, max_index) - VALUES (%d, %s, 1)) + VALUES (%d, %s, 1) `, repoID, sha, repoID, sha)); err != nil { return 0, err } From 662804c4389cd6a02f8b21b8b958726d0f9f5b88 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 13 Jun 2023 22:25:27 +0800 Subject: [PATCH 3/6] Fix test --- models/db/index.go | 2 +- models/git/commit_status.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/db/index.go b/models/db/index.go index 7fb3c83403ed..29254b1f07a0 100644 --- a/models/db/index.go +++ b/models/db/index.go @@ -100,7 +100,7 @@ WHEN MATCHED SET max_index = max_index + 1 WHEN NOT MATCHED THEN INSERT (group_id, max_index) - VALUES (%d, 1) + VALUES (%d, 1); `, tableName, groupID, groupID)); err != nil { return 0, err } diff --git a/models/git/commit_status.go b/models/git/commit_status.go index d8360d6bde19..74d52bb08db6 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -94,7 +94,7 @@ WHEN MATCHED SET max_index = max_index + 1 WHEN NOT MATCHED THEN INSERT (repo_id, sha, max_index) - VALUES (%d, %s, 1) + VALUES (%d, %s, 1); `, repoID, sha, repoID, sha)); err != nil { return 0, err } From 2dca80e8ab4e0ebec97e0d7f39a92f45258f31f3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 14 Jun 2023 11:50:04 +0800 Subject: [PATCH 4/6] Fix test --- models/git/commit_status.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 74d52bb08db6..30168097c201 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -86,7 +86,7 @@ func mysqlGetCommitStatusIndex(ctx context.Context, repoID int64, sha string) (i func mssqlGetCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { if _, err := db.GetEngine(ctx).Exec(fmt.Sprintf(` MERGE INTO commit_status_index WITH (HOLDLOCK) AS target -USING (SELECT %d AS repo_id, %s AS sha) AS source +USING (SELECT %d AS repo_id, '%s' AS sha) AS source (repo_id, sha) ON target.repo_id = source.repo_id AND target.sha = source.sha WHEN MATCHED @@ -94,7 +94,7 @@ WHEN MATCHED SET max_index = max_index + 1 WHEN NOT MATCHED THEN INSERT (repo_id, sha, max_index) - VALUES (%d, %s, 1); + VALUES (%d, '%s', 1); `, repoID, sha, repoID, sha)); err != nil { return 0, err } From 6eb55a58bedd95478eebae39e742750e4bc17727 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 14 Jun 2023 14:54:19 +0800 Subject: [PATCH 5/6] Prevent possible SQL inject --- models/git/commit_status.go | 4 ++++ modules/git/sha1.go | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 30168097c201..f1c1b5fb02a4 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -113,6 +113,10 @@ WHEN NOT MATCHED // GetNextCommitStatusIndex retried 3 times to generate a resource index func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { + if !git.IsValidSHAPattern(sha) { + return 0, git.ErrInvalidSHA{SHA: sha} + } + switch { case setting.Database.Type.IsPostgreSQL(): return postgresGetCommitStatusIndex(ctx, repoID, sha) diff --git a/modules/git/sha1.go b/modules/git/sha1.go index 4d69653e09ac..7d9d9776da9d 100644 --- a/modules/git/sha1.go +++ b/modules/git/sha1.go @@ -28,6 +28,14 @@ func IsValidSHAPattern(sha string) bool { return shaPattern.MatchString(sha) } +type ErrInvalidSHA struct { + SHA string +} + +func (err ErrInvalidSHA) Error() string { + return fmt.Sprintf("invalid sha: %s", err.SHA) +} + // MustID always creates a new SHA1 from a [20]byte array with no validation of input. func MustID(b []byte) SHA1 { var id SHA1 From 322e90235c8982dd91d6c55b02a575c11598dcec Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 14 Jun 2023 16:00:28 +0800 Subject: [PATCH 6/6] Use pre statement --- models/git/commit_status.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index f1c1b5fb02a4..49143a87e83f 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -84,9 +84,9 @@ func mysqlGetCommitStatusIndex(ctx context.Context, repoID int64, sha string) (i } func mssqlGetCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { - if _, err := db.GetEngine(ctx).Exec(fmt.Sprintf(` + if _, err := db.GetEngine(ctx).Exec(` MERGE INTO commit_status_index WITH (HOLDLOCK) AS target -USING (SELECT %d AS repo_id, '%s' AS sha) AS source +USING (SELECT ? AS repo_id, ? AS sha) AS source (repo_id, sha) ON target.repo_id = source.repo_id AND target.sha = source.sha WHEN MATCHED @@ -94,8 +94,8 @@ WHEN MATCHED SET max_index = max_index + 1 WHEN NOT MATCHED THEN INSERT (repo_id, sha, max_index) - VALUES (%d, '%s', 1); -`, repoID, sha, repoID, sha)); err != nil { + VALUES (?, ?, 1); +`, repoID, sha, repoID, sha); err != nil { return 0, err }