Merge branch 'master' into lunny/fix_approvals_counting

lunny · web-flow · commit c4b82ea09dde · 2019-08-06T09:12:49.000+08:00
diff --git a/integrations/migration-test/gitea-v1.3.3.sqlite3.sql.gz b/integrations/migration-test/gitea-v1.3.3.sqlite3.sql.gz
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
@@ -327,11 +327,25 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin
 			return err
 		}
 		tableSQL := string(res[0]["sql"])
+
+		// Separate out the column definitions
 		tableSQL = tableSQL[strings.Index(tableSQL, "("):]
+
+		// Remove the required columnNames
 		for _, name := range columnNames {
-			tableSQL = regexp.MustCompile(regexp.QuoteMeta("`"+name+"`")+"[^`,)]*[,)]").ReplaceAllString(tableSQL, "")
+			tableSQL = regexp.MustCompile(regexp.QuoteMeta("`"+name+"`")+"[^`,)]*?[,)]").ReplaceAllString(tableSQL, "")
+		}
+
+		// Ensure the query is ended properly
+		tableSQL = strings.TrimSpace(tableSQL)
+		if tableSQL[len(tableSQL)-1] != ')' {
+			if tableSQL[len(tableSQL)-1] == ',' {
+				tableSQL = tableSQL[:len(tableSQL)-1]
+			}
+			tableSQL += ")"
 		}
 
+		// Find all the columns in the table
 		columns := regexp.MustCompile("`([^`]*)`").FindAllString(tableSQL, -1)
 
 		tableSQL = fmt.Sprintf("CREATE TABLE `new_%s_new` ", tableName) + tableSQL
diff --git a/models/migrations/v78.go b/models/migrations/v78.go
@@ -5,13 +5,7 @@
 package migrations
 
 import (
-	"fmt"
-
-	"code.gitea.io/gitea/models"
-	"code.gitea.io/gitea/modules/log"
-
 	"github.com/go-xorm/xorm"
-	"xorm.io/core"
 )
 
 func renameRepoIsBareToIsEmpty(x *xorm.Engine) error {
@@ -21,73 +15,28 @@ func renameRepoIsBareToIsEmpty(x *xorm.Engine) error {
 		IsEmpty bool `xorm:"INDEX"`
 	}
 
-	// First remove the index
 	sess := x.NewSession()
 	defer sess.Close()
 	if err := sess.Begin(); err != nil {
 		return err
 	}
 
-	var err error
-	if models.DbCfg.Type == core.POSTGRES || models.DbCfg.Type == core.SQLITE {
-		_, err = sess.Exec("DROP INDEX IF EXISTS IDX_repository_is_bare")
-	} else if models.DbCfg.Type == core.MSSQL {
-		_, err = sess.Exec(`DECLARE @ConstraintName VARCHAR(256)
-		DECLARE @SQL NVARCHAR(256)
-		SELECT @ConstraintName = obj.name FROM sys.columns col LEFT OUTER JOIN sys.objects obj ON obj.object_id = col.default_object_id AND obj.type = 'D' WHERE col.object_id = OBJECT_ID('repository') AND obj.name IS NOT NULL AND col.name = 'is_bare'
-		SET @SQL = N'ALTER TABLE [repository] DROP CONSTRAINT [' + @ConstraintName + N']'
-		EXEC sp_executesql @SQL`)
-		if err != nil {
-			return err
-		}
-	} else if models.DbCfg.Type == core.MYSQL {
-		indexes, err := sess.QueryString(`SHOW INDEX FROM repository WHERE KEY_NAME = 'IDX_repository_is_bare'`)
-		if err != nil {
-			return err
-		}
-
-		if len(indexes) >= 1 {
-			_, err = sess.Exec("DROP INDEX IDX_repository_is_bare ON repository")
-			if err != nil {
-				return fmt.Errorf("Drop index failed: %v", err)
-			}
-		}
-	} else {
-		_, err = sess.Exec("DROP INDEX IDX_repository_is_bare ON repository")
-	}
-
-	if err != nil {
-		return fmt.Errorf("Drop index failed: %v", err)
-	}
-
-	if err = sess.Commit(); err != nil {
-		return err
-	}
-
-	if err := sess.Begin(); err != nil {
-		return err
-	}
-
 	if err := sess.Sync2(new(Repository)); err != nil {
 		return err
 	}
 	if _, err := sess.Exec("UPDATE repository SET is_empty = is_bare;"); err != nil {
 		return err
 	}
-
-	if models.DbCfg.Type != core.SQLITE {
-		_, err = sess.Exec("ALTER TABLE repository DROP COLUMN is_bare")
-		if err != nil {
-			return fmt.Errorf("Drop column failed: %v", err)
-		}
+	if err := sess.Commit(); err != nil {
+		return err
 	}
 
-	if err = sess.Commit(); err != nil {
+	if err := sess.Begin(); err != nil {
 		return err
 	}
-
-	if models.DbCfg.Type == core.SQLITE {
-		log.Warn("TABLE repository's COLUMN is_bare should be DROP but sqlite is not supported, you could manually do that.")
+	if err := dropTableColumns(sess, "repository", "is_bare"); err != nil {
+		return err
 	}
-	return nil
+
+	return sess.Commit()
 }
diff --git a/models/migrations/v85.go b/models/migrations/v85.go
@@ -8,9 +8,7 @@ import (
 	"fmt"
 
 	"github.com/go-xorm/xorm"
-	"xorm.io/core"
 
-	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/generate"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/util"
@@ -37,41 +35,6 @@ func hashAppToken(x *xorm.Engine) error {
 	// First remove the index
 	sess := x.NewSession()
 	defer sess.Close()
-	if err := sess.Begin(); err != nil {
-		return err
-	}
-
-	var err error
-	if models.DbCfg.Type == core.POSTGRES || models.DbCfg.Type == core.SQLITE {
-		_, err = sess.Exec("DROP INDEX IF EXISTS UQE_access_token_sha1")
-	} else if models.DbCfg.Type == core.MSSQL {
-		_, err = sess.Exec(`DECLARE @ConstraintName VARCHAR(256)
-		DECLARE @SQL NVARCHAR(256)
-		SELECT @ConstraintName = obj.name FROM sys.columns col LEFT OUTER JOIN sys.objects obj ON obj.object_id = col.default_object_id AND obj.type = 'D' WHERE col.object_id = OBJECT_ID('access_token') AND obj.name IS NOT NULL AND col.name = 'sha1'
-		SET @SQL = N'ALTER TABLE [access_token] DROP CONSTRAINT [' + @ConstraintName + N']'
-		EXEC sp_executesql @SQL`)
-	} else if models.DbCfg.Type == core.MYSQL {
-		indexes, err := sess.QueryString(`SHOW INDEX FROM access_token WHERE KEY_NAME = 'UQE_access_token_sha1'`)
-		if err != nil {
-			return err
-		}
-
-		if len(indexes) >= 1 {
-			_, err = sess.Exec("DROP INDEX UQE_access_token_sha1 ON access_token")
-			if err != nil {
-				return err
-			}
-		}
-	} else {
-		_, err = sess.Exec("DROP INDEX UQE_access_token_sha1 ON access_token")
-	}
-	if err != nil {
-		return fmt.Errorf("Drop index failed: %v", err)
-	}
-
-	if err = sess.Commit(); err != nil {
-		return err
-	}
 
 	if err := sess.Begin(); err != nil {
 		return err
@@ -81,7 +44,7 @@ func hashAppToken(x *xorm.Engine) error {
 		return fmt.Errorf("Sync2: %v", err)
 	}
 
-	if err = sess.Commit(); err != nil {
+	if err := sess.Commit(); err != nil {
 		return err
 	}
 
diff --git a/modules/git/commit.go b/modules/git/commit.go
@@ -169,6 +169,7 @@ func AddChanges(repoPath string, all bool, files ...string) error {
 	if all {
 		cmd.AddArguments("--all")
 	}
+	cmd.AddArguments("--")
 	_, err := cmd.AddArguments(files...).RunInDir(repoPath)
 	return err
 }
@@ -304,6 +305,7 @@ func (c *Commit) GetFilesChangedSinceCommit(pastCommit string) ([]string, error)
 }
 
 // FileChangedSinceCommit Returns true if the file given has changed since the the past commit
+// YOU MUST ENSURE THAT pastCommit is a valid commit ID.
 func (c *Commit) FileChangedSinceCommit(filename, pastCommit string) (bool, error) {
 	return c.repo.FileChangedBetweenCommits(filename, pastCommit, c.ID.String())
 }
diff --git a/modules/git/repo.go b/modules/git/repo.go
@@ -187,8 +187,7 @@ func Pull(repoPath string, opts PullRemoteOptions) error {
 	if opts.All {
 		cmd.AddArguments("--all")
 	} else {
-		cmd.AddArguments(opts.Remote)
-		cmd.AddArguments(opts.Branch)
+		cmd.AddArguments("--", opts.Remote, opts.Branch)
 	}
 
 	if opts.Timeout <= 0 {
@@ -213,7 +212,7 @@ func Push(repoPath string, opts PushOptions) error {
 	if opts.Force {
 		cmd.AddArguments("-f")
 	}
-	cmd.AddArguments(opts.Remote, opts.Branch)
+	cmd.AddArguments("--", opts.Remote, opts.Branch)
 	_, err := cmd.RunInDirWithEnv(repoPath, opts.Env)
 	return err
 }
diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go
@@ -135,7 +135,7 @@ func (repo *Repository) DeleteBranch(name string, opts DeleteBranchOptions) erro
 		cmd.AddArguments("-d")
 	}
 
-	cmd.AddArguments(name)
+	cmd.AddArguments("--", name)
 	_, err := cmd.RunInDir(repo.Path)
 
 	return err
diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go
@@ -117,20 +117,26 @@ func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
 	return commit, nil
 }
 
-// GetCommit returns commit object of by ID string.
-func (repo *Repository) GetCommit(commitID string) (*Commit, error) {
+// ConvertToSHA1 returns a Hash object from a potential ID string
+func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
 	if len(commitID) != 40 {
 		var err error
-		actualCommitID, err := NewCommand("rev-parse", commitID).RunInDir(repo.Path)
+		actualCommitID, err := NewCommand("rev-parse", "--verify", commitID).RunInDir(repo.Path)
 		if err != nil {
-			if strings.Contains(err.Error(), "unknown revision or path") {
-				return nil, ErrNotExist{commitID, ""}
+			if strings.Contains(err.Error(), "unknown revision or path") ||
+				strings.Contains(err.Error(), "fatal: Needed a single revision") {
+				return SHA1{}, ErrNotExist{commitID, ""}
 			}
-			return nil, err
+			return SHA1{}, err
 		}
 		commitID = actualCommitID
 	}
-	id, err := NewIDFromString(commitID)
+	return NewIDFromString(commitID)
+}
+
+// GetCommit returns commit object of by ID string.
+func (repo *Repository) GetCommit(commitID string) (*Commit, error) {
+	id, err := repo.ConvertToSHA1(commitID)
 	if err != nil {
 		return nil, err
 	}
@@ -243,6 +249,7 @@ func (repo *Repository) getFilesChanged(id1, id2 string) ([]string, error) {
 }
 
 // FileChangedBetweenCommits Returns true if the file changed between commit IDs id1 and id2
+// You must ensure that id1 and id2 are valid commit ids.
 func (repo *Repository) FileChangedBetweenCommits(filename, id1, id2 string) (bool, error) {
 	stdout, err := NewCommand("diff", "--name-only", "-z", id1, id2, "--", filename).RunInDirBytes(repo.Path)
 	if err != nil {
diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go
@@ -58,5 +58,5 @@ func TestGetCommitWithBadCommitID(t *testing.T) {
 	commit, err := bareRepo1.GetCommit("bad_branch")
 	assert.Nil(t, commit)
 	assert.Error(t, err)
-	assert.EqualError(t, err, "object does not exist [id: bad_branch, rel_path: ]")
+	assert.True(t, IsErrNotExist(err))
 }
diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go
@@ -39,7 +39,7 @@ func (repo *Repository) GetMergeBase(tmpRemote string, base, head string) (strin
 		}
 	}
 
-	stdout, err := NewCommand("merge-base", base, head).RunInDir(repo.Path)
+	stdout, err := NewCommand("merge-base", "--", base, head).RunInDir(repo.Path)
 	return strings.TrimSpace(stdout), base, err
 }
 
diff --git a/modules/git/repo_index.go b/modules/git/repo_index.go
@@ -12,7 +12,7 @@ import (
 // ReadTreeToIndex reads a treeish to the index
 func (repo *Repository) ReadTreeToIndex(treeish string) error {
 	if len(treeish) != 40 {
-		res, err := NewCommand("rev-parse", treeish).RunInDir(repo.Path)
+		res, err := NewCommand("rev-parse", "--verify", treeish).RunInDir(repo.Path)
 		if err != nil {
 			return err
 		}
diff --git a/modules/git/repo_tag.go b/modules/git/repo_tag.go
@@ -29,13 +29,13 @@ func (repo *Repository) IsTagExist(name string) bool {
 
 // CreateTag create one tag in the repository
 func (repo *Repository) CreateTag(name, revision string) error {
-	_, err := NewCommand("tag", name, revision).RunInDir(repo.Path)
+	_, err := NewCommand("tag", "--", name, revision).RunInDir(repo.Path)
 	return err
 }
 
 // CreateAnnotatedTag create one annotated tag in the repository
 func (repo *Repository) CreateAnnotatedTag(name, message, revision string) error {
-	_, err := NewCommand("tag", "-a", "-m", message, name, revision).RunInDir(repo.Path)
+	_, err := NewCommand("tag", "-a", "-m", message, "--", name, revision).RunInDir(repo.Path)
 	return err
 }
 
@@ -153,7 +153,7 @@ func (repo *Repository) GetTagNameBySHA(sha string) (string, error) {
 
 // GetTagID returns the object ID for a tag (annotated tags have both an object SHA AND a commit SHA)
 func (repo *Repository) GetTagID(name string) (string, error) {
-	stdout, err := NewCommand("show-ref", name).RunInDir(repo.Path)
+	stdout, err := NewCommand("show-ref", "--", name).RunInDir(repo.Path)
 	if err != nil {
 		return "", err
 	}
diff --git a/modules/git/repo_tree.go b/modules/git/repo_tree.go
@@ -26,7 +26,7 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
 // GetTree find the tree object in the repository.
 func (repo *Repository) GetTree(idStr string) (*Tree, error) {
 	if len(idStr) != 40 {
-		res, err := NewCommand("rev-parse", idStr).RunInDir(repo.Path)
+		res, err := NewCommand("rev-parse", "--verify", idStr).RunInDir(repo.Path)
 		if err != nil {
 			return nil, err
 		}
diff --git a/modules/repofiles/delete.go b/modules/repofiles/delete.go
@@ -92,6 +92,12 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo
 	// Assigned LastCommitID in opts if it hasn't been set
 	if opts.LastCommitID == "" {
 		opts.LastCommitID = commit.ID.String()
+	} else {
+		lastCommitID, err := t.gitRepo.ConvertToSHA1(opts.LastCommitID)
+		if err != nil {
+			return nil, fmt.Errorf("DeleteRepoFile: Invalid last commit ID: %v", err)
+		}
+		opts.LastCommitID = lastCommitID.String()
 	}
 
 	// Get the files in the index
diff --git a/modules/repofiles/update.go b/modules/repofiles/update.go
@@ -190,6 +190,13 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up
 	// Assigned LastCommitID in opts if it hasn't been set
 	if opts.LastCommitID == "" {
 		opts.LastCommitID = commit.ID.String()
+	} else {
+		lastCommitID, err := t.gitRepo.ConvertToSHA1(opts.LastCommitID)
+		if err != nil {
+			return nil, fmt.Errorf("DeleteRepoFile: Invalid last commit ID: %v", err)
+		}
+		opts.LastCommitID = lastCommitID.String()
+
 	}
 
 	encoding := "UTF-8"
diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go
@@ -10,9 +10,9 @@ type FileOptions struct {
 	// message (optional) for the commit of this file. if not supplied, a default message will be used
 	Message string `json:"message"`
 	// branch (optional) to base this file from. if not given, the default branch is used
-	BranchName string `json:"branch"`
+	BranchName string `json:"branch" binding:"GitRefName;MaxSize(100)"`
 	// new_branch (optional) will make a new branch from `branch` before creating the file
-	NewBranchName string `json:"new_branch"`
+	NewBranchName string `json:"new_branch" binding:"GitRefName;MaxSize(100)"`
 	// `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)
 	Author    Identity `json:"author"`
 	Committer Identity `json:"committer"`

Original file line number	Diff line number	Diff line change
`@@ -169,6 +169,7 @@ func AddChanges(repoPath string, all bool, files ...string) error {`
`169`	`169`	`if all {`
`170`	`170`	`cmd.AddArguments("--all")`
`171`	`171`	`}`
	`172`	`+ cmd.AddArguments("--")`
`172`	`173`	`_, err := cmd.AddArguments(files...).RunInDir(repoPath)`
`173`	`174`	`return err`
`174`	`175`	`}`
`@@ -304,6 +305,7 @@ func (c *Commit) GetFilesChangedSinceCommit(pastCommit string) ([]string, error)`
`304`	`305`	`}`
`305`	`306`
`306`	`307`	`// FileChangedSinceCommit Returns true if the file given has changed since the the past commit`
	`308`	`+// YOU MUST ENSURE THAT pastCommit is a valid commit ID.`
`307`	`309`	`func (c *Commit) FileChangedSinceCommit(filename, pastCommit string) (bool, error) {`
`308`	`310`	`return c.repo.FileChangedBetweenCommits(filename, pastCommit, c.ID.String())`
`309`	`311`	`}`
Original file line number	Diff line number	Diff line change
`@@ -187,8 +187,7 @@ func Pull(repoPath string, opts PullRemoteOptions) error {`
`187`	`187`	`if opts.All {`
`188`	`188`	`cmd.AddArguments("--all")`
`189`	`189`	`} else {`
`190`		`- cmd.AddArguments(opts.Remote)`
`191`		`- cmd.AddArguments(opts.Branch)`
	`190`	`+ cmd.AddArguments("--", opts.Remote, opts.Branch)`
`192`	`191`	`}`
`193`	`192`
`194`	`193`	`if opts.Timeout <= 0 {`
`@@ -213,7 +212,7 @@ func Push(repoPath string, opts PushOptions) error {`
`213`	`212`	`if opts.Force {`
`214`	`213`	`cmd.AddArguments("-f")`
`215`	`214`	`}`
`216`		`- cmd.AddArguments(opts.Remote, opts.Branch)`
	`215`	`+ cmd.AddArguments("--", opts.Remote, opts.Branch)`
`217`	`216`	`_, err := cmd.RunInDirWithEnv(repoPath, opts.Env)`
`218`	`217`	`return err`
`219`	`218`	`}`
Original file line number	Diff line number	Diff line change
`@@ -135,7 +135,7 @@ func (repo *Repository) DeleteBranch(name string, opts DeleteBranchOptions) erro`
`135`	`135`	`cmd.AddArguments("-d")`
`136`	`136`	`}`
`137`	`137`
`138`		`- cmd.AddArguments(name)`
	`138`	`+ cmd.AddArguments("--", name)`
`139`	`139`	`_, err := cmd.RunInDir(repo.Path)`
`140`	`140`
`141`	`141`	`return err`
Original file line number	Diff line number	Diff line change
`@@ -58,5 +58,5 @@ func TestGetCommitWithBadCommitID(t *testing.T) {`
`58`	`58`	`commit, err := bareRepo1.GetCommit("bad_branch")`
`59`	`59`	`assert.Nil(t, commit)`
`60`	`60`	`assert.Error(t, err)`
`61`		`- assert.EqualError(t, err, "object does not exist [id: bad_branch, rel_path: ]")`
	`61`	`+ assert.True(t, IsErrNotExist(err))`
`62`	`62`	`}`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ func (repo *Repository) GetMergeBase(tmpRemote string, base, head string) (strin`
`39`	`39`	`}`
`40`	`40`	`}`
`41`	`41`
`42`		`- stdout, err := NewCommand("merge-base", base, head).RunInDir(repo.Path)`
	`42`	`+ stdout, err := NewCommand("merge-base", "--", base, head).RunInDir(repo.Path)`
`43`	`43`	`return strings.TrimSpace(stdout), base, err`
`44`	`44`	`}`
`45`	`45`