Handle too long PR titles correctly (#16517)

The CompareAndPullRequestPost handler for POST to /compare
incorrectly handles returning errors to the user. For a start
it does not set the necessary markers to switch SimpleMDE
but it also does not immediately return to the form.

This PR fixes this by setting the appropriate values, fixing
the templates and preventing the suggestion of a too long
title.

Fix #16507

Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
zeripath 2021-07-25 03:59:27 +01:00 committed by GitHub
parent 4f23624b16
commit fd15fd4c67
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 64 additions and 4 deletions

35
modules/util/truncate.go Normal file
View file

@ -0,0 +1,35 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package util
import "unicode/utf8"
// SplitStringAtByteN splits a string at byte n accounting for rune boundaries. (Combining characters are not accounted for.)
func SplitStringAtByteN(input string, n int) (left, right string) {
if len(input) <= n {
left = input
return
}
if !utf8.ValidString(input) {
left = input[:n-3] + "..."
right = "..." + input[n-3:]
return
}
// in UTF8 "…" is 3 bytes so doesn't really gain us anything...
end := 0
for end <= n-3 {
_, size := utf8.DecodeRuneInString(input[end:])
if end+size > n-3 {
break
}
end += size
}
left = input[:end] + "…"
right = "…" + input[end:]
return
}

View file

@ -24,6 +24,7 @@ import (
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/upload" "code.gitea.io/gitea/modules/upload"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/gitdiff" "code.gitea.io/gitea/services/gitdiff"
) )
@ -567,6 +568,18 @@ func PrepareCompareDiff(
} else { } else {
title = headBranch title = headBranch
} }
if len(title) > 255 {
var trailer string
title, trailer = util.SplitStringAtByteN(title, 255)
if len(trailer) > 0 {
if ctx.Data["content"] != nil {
ctx.Data["content"] = fmt.Sprintf("%s\n\n%s", trailer, ctx.Data["content"])
} else {
ctx.Data["content"] = trailer + "\n"
}
}
}
ctx.Data["title"] = title ctx.Data["title"] = title
ctx.Data["Username"] = headUser.Name ctx.Data["Username"] = headUser.Name
ctx.Data["Reponame"] = headRepo.Name ctx.Data["Reponame"] = headRepo.Name

View file

@ -1001,10 +1001,14 @@ func CompareAndPullRequestPost(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("repo.pulls.compare_changes") ctx.Data["Title"] = ctx.Tr("repo.pulls.compare_changes")
ctx.Data["PageIsComparePull"] = true ctx.Data["PageIsComparePull"] = true
ctx.Data["IsDiffCompare"] = true ctx.Data["IsDiffCompare"] = true
ctx.Data["IsRepoToolbarCommits"] = true
ctx.Data["RequireTribute"] = true
ctx.Data["RequireSimpleMDE"] = true
ctx.Data["RequireHighlightJS"] = true ctx.Data["RequireHighlightJS"] = true
ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
upload.AddUploadContext(ctx, "comment") upload.AddUploadContext(ctx, "comment")
ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.CanWrite(models.UnitTypePullRequests)
var ( var (
repo = ctx.Repo.Repository repo = ctx.Repo.Repository
@ -1037,6 +1041,14 @@ func CompareAndPullRequestPost(ctx *context.Context) {
return return
} }
if len(form.Title) > 255 {
var trailer string
form.Title, trailer = util.SplitStringAtByteN(form.Title, 255)
form.Content = trailer + "\n\n" + form.Content
}
middleware.AssignForm(form, ctx.Data)
ctx.HTML(http.StatusOK, tplCompareDiff) ctx.HTML(http.StatusOK, tplCompareDiff)
return return
} }

View file

@ -176,10 +176,10 @@
{{if .IsNothingToCompare}} {{if .IsNothingToCompare}}
{{if and $.IsSigned $.AllowEmptyPr (not .Repository.IsArchived) }} {{if and $.IsSigned $.AllowEmptyPr (not .Repository.IsArchived) }}
<div class="ui segment">{{.i18n.Tr "repo.pulls.nothing_to_compare_and_allow_empty_pr"}}</div> <div class="ui segment">{{.i18n.Tr "repo.pulls.nothing_to_compare_and_allow_empty_pr"}}</div>
<div class="ui info message show-form-container"> <div class="ui info message show-form-container" {{if .Flash}}style="display: none"{{end}}>
<button class="ui button green show-form">{{.i18n.Tr "repo.pulls.new"}}</button> <button class="ui button green show-form">{{.i18n.Tr "repo.pulls.new"}}</button>
</div> </div>
<div class="pullrequest-form" style="display: none"> <div class="pullrequest-form" {{if not .Flash}}style="display: none"{{end}}>
{{template "repo/issue/new_form" .}} {{template "repo/issue/new_form" .}}
</div> </div>
{{else}} {{else}}
@ -192,7 +192,7 @@
</div> </div>
{{else}} {{else}}
{{if and $.IsSigned (not .Repository.IsArchived)}} {{if and $.IsSigned (not .Repository.IsArchived)}}
<div class="ui info message show-form-container"> <div class="ui info message show-form-container" {{if .Flash}}style="display: none"{{end}}>
<button class="ui button green show-form">{{.i18n.Tr "repo.pulls.new"}}</button> <button class="ui button green show-form">{{.i18n.Tr "repo.pulls.new"}}</button>
</div> </div>
{{else if .Repository.IsArchived}} {{else if .Repository.IsArchived}}
@ -201,7 +201,7 @@
</div> </div>
{{end}} {{end}}
{{if $.IsSigned}} {{if $.IsSigned}}
<div class="pullrequest-form" style="display: none"> <div class="pullrequest-form" {{if not .Flash}}style="display: none"{{end}}>
{{template "repo/issue/new_form" .}} {{template "repo/issue/new_form" .}}
</div> </div>
{{end}} {{end}}