Fix commit message rendering and some UI problems (#34680)

* Fix #34679
* Fix #34676
* Fix #34674
* Fix #34526
pull/34556/head^2
wxiaoguang 2025-06-10 23:20:32 +08:00 committed by GitHub
parent e9f5105e95
commit 1610a63bfd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 31 additions and 20 deletions

View File

@ -48,10 +48,7 @@ type RepoCommentOptions struct {
} }
func NewRenderContextRepoComment(ctx context.Context, repo *repo_model.Repository, opts ...RepoCommentOptions) *markup.RenderContext { func NewRenderContextRepoComment(ctx context.Context, repo *repo_model.Repository, opts ...RepoCommentOptions) *markup.RenderContext {
helper := &RepoComment{ helper := &RepoComment{opts: util.OptionalArg(opts)}
repoLink: repo.Link(),
opts: util.OptionalArg(opts),
}
rctx := markup.NewRenderContext(ctx) rctx := markup.NewRenderContext(ctx)
helper.ctx = rctx helper.ctx = rctx
var metas map[string]string var metas map[string]string
@ -60,15 +57,16 @@ func NewRenderContextRepoComment(ctx context.Context, repo *repo_model.Repositor
helper.commitChecker = newCommitChecker(ctx, repo) helper.commitChecker = newCommitChecker(ctx, repo)
metas = repo.ComposeCommentMetas(ctx) metas = repo.ComposeCommentMetas(ctx)
} else { } else {
// this is almost dead code, only to pass the incorrect tests // repo can be nil when rendering a commit message in user's dashboard feedback whose repository has been deleted
helper.repoLink = fmt.Sprintf("%s/%s", helper.opts.DeprecatedOwnerName, helper.opts.DeprecatedRepoName) metas = map[string]string{}
rctx = rctx.WithMetas(map[string]string{ if helper.opts.DeprecatedOwnerName != "" {
"user": helper.opts.DeprecatedOwnerName, // this is almost dead code, only to pass the incorrect tests
"repo": helper.opts.DeprecatedRepoName, helper.repoLink = fmt.Sprintf("%s/%s", helper.opts.DeprecatedOwnerName, helper.opts.DeprecatedRepoName)
metas["user"] = helper.opts.DeprecatedOwnerName
"markdownNewLineHardBreak": "true", metas["repo"] = helper.opts.DeprecatedRepoName
"markupAllowShortIssuePattern": "true", }
}) metas["markdownNewLineHardBreak"] = "true"
metas["markupAllowShortIssuePattern"] = "true"
} }
metas["footnoteContextId"] = helper.opts.FootnoteContextID metas["footnoteContextId"] = helper.opts.FootnoteContextID
rctx = rctx.WithMetas(metas).WithHelper(helper) rctx = rctx.WithMetas(metas).WithHelper(helper)

View File

@ -72,4 +72,11 @@ func TestRepoComment(t *testing.T) {
<a href="/user2/repo1/commit/1234/image" target="_blank" rel="nofollow noopener"><img src="/user2/repo1/commit/1234/image" alt="./image"/></a></p> <a href="/user2/repo1/commit/1234/image" target="_blank" rel="nofollow noopener"><img src="/user2/repo1/commit/1234/image" alt="./image"/></a></p>
`, rendered) `, rendered)
}) })
t.Run("NoRepo", func(t *testing.T) {
rctx := NewRenderContextRepoComment(t.Context(), nil).WithMarkupType(markdown.MarkupName)
rendered, err := markup.RenderString(rctx, "any")
assert.NoError(t, err)
assert.Equal(t, "<p>any</p>\n", rendered)
})
} }

View File

@ -86,8 +86,8 @@ var globalVars = sync.OnceValue(func() *globalVarsType {
// codePreviewPattern matches "http://domain/.../{owner}/{repo}/src/commit/{commit}/{filepath}#L10-L20" // codePreviewPattern matches "http://domain/.../{owner}/{repo}/src/commit/{commit}/{filepath}#L10-L20"
v.codePreviewPattern = regexp.MustCompile(`https?://\S+/([^\s/]+)/([^\s/]+)/src/commit/([0-9a-f]{7,64})(/\S+)#(L\d+(-L\d+)?)`) v.codePreviewPattern = regexp.MustCompile(`https?://\S+/([^\s/]+)/([^\s/]+)/src/commit/([0-9a-f]{7,64})(/\S+)#(L\d+(-L\d+)?)`)
// cleans: "<foo/bar", "<any words/", ("<html", "<head", "<script", "<style") // cleans: "<foo/bar", "<any words/", ("<html", "<head", "<script", "<style", "<?", "<%")
v.tagCleaner = regexp.MustCompile(`(?i)<(/?\w+/\w+|/[\w ]+/|/?(html|head|script|style\b))`) v.tagCleaner = regexp.MustCompile(`(?i)<(/?\w+/\w+|/[\w ]+/|/?(html|head|script|style|%|\?)\b)`)
v.nulCleaner = strings.NewReplacer("\000", "") v.nulCleaner = strings.NewReplacer("\000", "")
return v return v
}) })
@ -253,7 +253,7 @@ func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output
node, err := html.Parse(io.MultiReader( node, err := html.Parse(io.MultiReader(
// prepend "<html><body>" // prepend "<html><body>"
strings.NewReader("<html><body>"), strings.NewReader("<html><body>"),
// Strip out nuls - they're always invalid // strip out NULLs (they're always invalid), and escape known tags
bytes.NewReader(globalVars().tagCleaner.ReplaceAll([]byte(globalVars().nulCleaner.Replace(string(rawHTML))), []byte("&lt;$1"))), bytes.NewReader(globalVars().tagCleaner.ReplaceAll([]byte(globalVars().nulCleaner.Replace(string(rawHTML))), []byte("&lt;$1"))),
// close the tags // close the tags
strings.NewReader("</body></html>"), strings.NewReader("</body></html>"),

View File

@ -525,6 +525,10 @@ func TestPostProcess(t *testing.T) {
test("<script>a</script>", `&lt;script&gt;a&lt;/script&gt;`) test("<script>a</script>", `&lt;script&gt;a&lt;/script&gt;`)
test("<STYLE>a", `&lt;STYLE&gt;a`) test("<STYLE>a", `&lt;STYLE&gt;a`)
test("<style>a</STYLE>", `&lt;style&gt;a&lt;/STYLE&gt;`) test("<style>a</STYLE>", `&lt;style&gt;a&lt;/STYLE&gt;`)
// other special tags, our special behavior
test("<?php\nfoo", "&lt;?php\nfoo")
test("<%asp\nfoo", "&lt;%asp\nfoo")
} }
func TestIssue16020(t *testing.T) { func TestIssue16020(t *testing.T) {

View File

@ -38,8 +38,8 @@ func NewRenderUtils(ctx reqctx.RequestContext) *RenderUtils {
// RenderCommitMessage renders commit message with XSS-safe and special links. // RenderCommitMessage renders commit message with XSS-safe and special links.
func (ut *RenderUtils) RenderCommitMessage(msg string, repo *repo.Repository) template.HTML { func (ut *RenderUtils) RenderCommitMessage(msg string, repo *repo.Repository) template.HTML {
cleanMsg := template.HTMLEscapeString(msg) cleanMsg := template.HTMLEscapeString(msg)
// we can safely assume that it will not return any error, since there // we can safely assume that it will not return any error, since there shouldn't be any special HTML.
// shouldn't be any special HTML. // "repo" can be nil when rendering commit messages for deleted repositories in a user's dashboard feed.
fullMessage, err := markup.PostProcessCommitMessage(renderhelper.NewRenderContextRepoComment(ut.ctx, repo), cleanMsg) fullMessage, err := markup.PostProcessCommitMessage(renderhelper.NewRenderContextRepoComment(ut.ctx, repo), cleanMsg)
if err != nil { if err != nil {
log.Error("PostProcessCommitMessage: %v", err) log.Error("PostProcessCommitMessage: %v", err)
@ -47,7 +47,7 @@ func (ut *RenderUtils) RenderCommitMessage(msg string, repo *repo.Repository) te
} }
msgLines := strings.Split(strings.TrimSpace(fullMessage), "\n") msgLines := strings.Split(strings.TrimSpace(fullMessage), "\n")
if len(msgLines) == 0 { if len(msgLines) == 0 {
return template.HTML("") return ""
} }
return renderCodeBlock(template.HTML(msgLines[0])) return renderCodeBlock(template.HTML(msgLines[0]))
} }

View File

@ -33,7 +33,8 @@
</div> </div>
{{end}} {{end}}
<div class="ui field"> <div class="ui field">
<button class="ui tiny primary button" type="submit">{{ctx.Locale.Tr "actions.workflow.run"}}</button> {{/* use autofocus here to prevent the "branch selection" dropdown from getting focus, otherwise it will auto popup */}}
<button class="ui tiny primary button" autofocus type="submit">{{ctx.Locale.Tr "actions.workflow.run"}}</button>
</div> </div>
{{end}} {{end}}
{{range .workflows}} {{range .workflows}}

View File

@ -7,6 +7,7 @@
padding: 8px 10px; padding: 8px 10px;
border: 1px solid var(--color-secondary); border: 1px solid var(--color-secondary);
background: var(--color-card); background: var(--color-card);
color: var(--color-text); /* it can't inherit from parent because the card already has its own background */
} }
.issue-card-icon, .issue-card-icon,