From a533b20d400e1af72233c18f6797971b5e3c49ea Mon Sep 17 00:00:00 2001 From: Ilya Pavlov Date: Tue, 22 Oct 2019 13:27:28 +0300 Subject: [PATCH 1/8] if a filename in a repository contains " or \ the owner can't merge pull request with this files because "git diff-tree" adds double quotes to that filepath example: filepath is ab"cd but "git diff-tree" returns "ab\"cd" now, when the owner click "Merge Pull Request" button the server returns 500 this commit fix it Signed-off-by: Ilya Pavlov --- services/pull/merge.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 5eb8eaa4d4701..5b13f156b22c9 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -393,7 +393,12 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { out := bytes.Buffer{} scanner := bufio.NewScanner(strings.NewReader(list)) for scanner.Scan() { - fmt.Fprintf(&out, "/%s\n", scanner.Text()) + filepath := scanner.Text() + if strings.HasPrefix(filepath, `"`) { // the filepath contains " or \. + filepath = strings.TrimPrefix(filepath, `"`) + filepath = strings.TrimSuffix(filepath, `"`) + } + fmt.Fprintf(&out, "/%s\n", filepath) } return out.String(), nil } From 5b5a221627969ccdc95073ee36fcc032080f8bac Mon Sep 17 00:00:00 2001 From: Ilya Pavlov Date: Sat, 26 Oct 2019 03:34:07 +0300 Subject: [PATCH 2/8] add -z option to getDiffTree escape spec symbols for sparse-checkout Signed-off-by: Ilya Pavlov --- services/pull/merge.go | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 5b13f156b22c9..17f3dfd7e414c 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -14,6 +14,7 @@ import ( "path/filepath" "strings" "time" + "regexp" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/cache" @@ -378,27 +379,55 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { var outbuf, errbuf strings.Builder // Compute the diff-tree for sparse-checkout - if err := git.NewCommand("diff-tree", "--no-commit-id", "--name-only", "-r", "--root", baseBranch, headBranch, "--").RunInDirPipeline(repoPath, &outbuf, &errbuf); err != nil { + if err := git.NewCommand("diff-tree", "--no-commit-id", "--name-only", "-r", "-z", "--root", baseBranch, headBranch, "--").RunInDirPipeline(repoPath, &outbuf, &errbuf); err != nil { return "", fmt.Errorf("git diff-tree [%s base:%s head:%s]: %s", repoPath, baseBranch, headBranch, errbuf.String()) } return outbuf.String(), nil } + scanNullTerminatedStrings := func(data []byte, atEOF bool) (advance int, token []byte, err error) { + if atEOF && len(data) == 0 { + return 0, nil, nil + } + if i := bytes.IndexByte(data, '\x00'); i >= 0 { + return i + 1, data[0:i], nil + } + if atEOF { + return len(data), data, nil + } + return 0, nil, nil + } + list, err := getDiffTreeFromBranch(repoPath, baseBranch, headBranch) if err != nil { return "", err } // Prefixing '/' for each entry, otherwise all files with the same name in subdirectories would be matched. + rightTrailingSpacesRE := regexp.MustCompile(`\ +$`) + optionalNPrefixRE := regexp.MustCompile(`(?:^|/)(!.+?)$`) + out := bytes.Buffer{} scanner := bufio.NewScanner(strings.NewReader(list)) + scanner.Split(scanNullTerminatedStrings) for scanner.Scan() { filepath := scanner.Text() - if strings.HasPrefix(filepath, `"`) { // the filepath contains " or \. - filepath = strings.TrimPrefix(filepath, `"`) - filepath = strings.TrimSuffix(filepath, `"`) - } + + // Trailing spaces + filepath = rightTrailingSpacesRE.ReplaceAllStringFunc(filepath, func(s string) string { + return strings.Repeat(`\ `, len(s)) + }) + + // An optional prefix ! + filepath = optionalNPrefixRE.ReplaceAllString(filepath, `/\$1`) + + // * ? [ + filepath = strings.Replace(filepath, "*", `\*`, -1) + filepath = strings.Replace(filepath, "?", `\?`, -1) + filepath = strings.Replace(filepath, "[", `\[`, -1) + fmt.Fprintf(&out, "/%s\n", filepath) } + return out.String(), nil } From 72c4e2f1f3109c58d584e599f65e98ca72c7afee Mon Sep 17 00:00:00 2001 From: Ilya Pavlov Date: Sat, 26 Oct 2019 03:58:00 +0300 Subject: [PATCH 3/8] go fmt Signed-off-by: Ilya Pavlov --- services/pull/merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 17f3dfd7e414c..7478c0f87d573 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -12,9 +12,9 @@ import ( "io/ioutil" "os" "path/filepath" + "regexp" "strings" "time" - "regexp" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/cache" From 97c392c06a6c025cab26432b370f812ed4889a22 Mon Sep 17 00:00:00 2001 From: Ilya Pavlov Date: Sat, 26 Oct 2019 04:35:15 +0300 Subject: [PATCH 4/8] typo Signed-off-by: Ilya Pavlov --- services/pull/merge.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 7478c0f87d573..927844035600b 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -419,13 +419,13 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { }) // An optional prefix ! - filepath = optionalNPrefixRE.ReplaceAllString(filepath, `/\$1`) + filepath = optionalNPrefixRE.ReplaceAllString(filepath, `\$1`) // * ? [ filepath = strings.Replace(filepath, "*", `\*`, -1) filepath = strings.Replace(filepath, "?", `\?`, -1) filepath = strings.Replace(filepath, "[", `\[`, -1) - +fmt.Printf("%s\n", filepath) fmt.Fprintf(&out, "/%s\n", filepath) } From 893fc6685d8e0dc73ff4a9e9bb1f19c167a73480 Mon Sep 17 00:00:00 2001 From: Ilya Pavlov Date: Mon, 28 Oct 2019 21:45:07 +0300 Subject: [PATCH 5/8] escape '\' escape all spaces and '!' --- services/pull/merge.go | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 927844035600b..b43f86cd22d02 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -12,7 +12,6 @@ import ( "io/ioutil" "os" "path/filepath" - "regexp" "strings" "time" @@ -404,28 +403,22 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { } // Prefixing '/' for each entry, otherwise all files with the same name in subdirectories would be matched. - rightTrailingSpacesRE := regexp.MustCompile(`\ +$`) - optionalNPrefixRE := regexp.MustCompile(`(?:^|/)(!.+?)$`) - out := bytes.Buffer{} scanner := bufio.NewScanner(strings.NewReader(list)) scanner.Split(scanNullTerminatedStrings) for scanner.Scan() { filepath := scanner.Text() - // Trailing spaces - filepath = rightTrailingSpacesRE.ReplaceAllStringFunc(filepath, func(s string) string { - return strings.Repeat(`\ `, len(s)) - }) - - // An optional prefix ! - filepath = optionalNPrefixRE.ReplaceAllString(filepath, `\$1`) + // escape '\', '*', '?', '[', spaces and '!' prefix + // replace '\' first + filepath = strings.ReplaceAll(filepath, `\`, `\\`) + filepath = strings.ReplaceAll(filepath, "*", `\*`) + filepath = strings.ReplaceAll(filepath, "?", `\?`) + filepath = strings.ReplaceAll(filepath, "[", `\[`) + filepath = strings.ReplaceAll(filepath, " ", `\ `) + filepath = strings.ReplaceAll(filepath, "!", `\!`) - // * ? [ - filepath = strings.Replace(filepath, "*", `\*`, -1) - filepath = strings.Replace(filepath, "?", `\?`, -1) - filepath = strings.Replace(filepath, "[", `\[`, -1) -fmt.Printf("%s\n", filepath) + // no necessary to escape the first '#' symbol because the first symbol is '/' fmt.Fprintf(&out, "/%s\n", filepath) } From 4794affd80ec5eb555e427a36fcaa26af760d032 Mon Sep 17 00:00:00 2001 From: Ilya Pavlov Date: Mon, 28 Oct 2019 22:10:29 +0300 Subject: [PATCH 6/8] use regexp.ReplaceAllString() Signed-off-by: Ilya Pavlov --- services/pull/merge.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index b43f86cd22d02..3a5c65f27a903 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -12,6 +12,7 @@ import ( "io/ioutil" "os" "path/filepath" + "regexp" "strings" "time" @@ -374,6 +375,8 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return nil } +var escapedSymbols = regexp.MustCompile(`([*[?! ])`) + func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { var outbuf, errbuf strings.Builder @@ -409,14 +412,10 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { for scanner.Scan() { filepath := scanner.Text() - // escape '\', '*', '?', '[', spaces and '!' prefix // replace '\' first filepath = strings.ReplaceAll(filepath, `\`, `\\`) - filepath = strings.ReplaceAll(filepath, "*", `\*`) - filepath = strings.ReplaceAll(filepath, "?", `\?`) - filepath = strings.ReplaceAll(filepath, "[", `\[`) - filepath = strings.ReplaceAll(filepath, " ", `\ `) - filepath = strings.ReplaceAll(filepath, "!", `\!`) + // escape '*', '?', '[', spaces and '!' prefix + filepath = escapedSymbols.ReplaceAllString(filepath, `\$1`) // no necessary to escape the first '#' symbol because the first symbol is '/' fmt.Fprintf(&out, "/%s\n", filepath) From f8da2d8dc9b95221fbb317adcc1bc438903cb293 Mon Sep 17 00:00:00 2001 From: Ilya Pavlov Date: Mon, 28 Oct 2019 23:33:10 +0300 Subject: [PATCH 7/8] strings.ReplaceAll was added in go 1.12 Signed-off-by: Ilya Pavlov --- services/pull/merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 3a5c65f27a903..0ebca8124f240 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -413,7 +413,7 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { filepath := scanner.Text() // replace '\' first - filepath = strings.ReplaceAll(filepath, `\`, `\\`) + filepath = strings.Replace(filepath, `\`, `\\`, -1) // escape '*', '?', '[', spaces and '!' prefix filepath = escapedSymbols.ReplaceAllString(filepath, `\$1`) From df576958485e4bbac6e53bc7c103726d394fc532 Mon Sep 17 00:00:00 2001 From: Ilya Pavlov Date: Mon, 28 Oct 2019 23:43:53 +0300 Subject: [PATCH 8/8] add '\' to regexp.MustCompile Signed-off-by: Ilya Pavlov --- services/pull/merge.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index 0ebca8124f240..2e093eef86afd 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -375,7 +375,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return nil } -var escapedSymbols = regexp.MustCompile(`([*[?! ])`) +var escapedSymbols = regexp.MustCompile(`([*[?! \\])`) func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { @@ -411,12 +411,8 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { scanner.Split(scanNullTerminatedStrings) for scanner.Scan() { filepath := scanner.Text() - - // replace '\' first - filepath = strings.Replace(filepath, `\`, `\\`, -1) // escape '*', '?', '[', spaces and '!' prefix filepath = escapedSymbols.ReplaceAllString(filepath, `\$1`) - // no necessary to escape the first '#' symbol because the first symbol is '/' fmt.Fprintf(&out, "/%s\n", filepath) }