Skip to content

Commit e71eb89

Browse files
wxiaoguangwxiaoguang
authored
Refactor some Str2html code (#29397)
This PR touches the most interesting part of the "template refactoring". 1. Unclear variable type. Especially for "web/feed/convert.go": sometimes it uses text, sometimes it uses HTML. 2. Assign text content to "RenderedContent" field, for example: ` project.RenderedContent = project.Description` in web/org/projects.go 3. Assign rendered content to text field, for example: `r.Note = rendered content` in web/repo/release.go 4. (possible) Incorrectly calling `{{Str2html .PackageDescriptor.Metadata.ReleaseNotes}}` in package/content/nuget.tmpl, I guess the name Str2html misleads developers to use it to "render string to html", but it only sanitizes. if ReleaseNotes really contains HTML, then this is not a problem.
1 parent 58ce1de commit e71eb89

32 files changed

+91
-61
lines changed

‎models/issues/comment.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package issues
88
import (
99
"context"
1010
"fmt"
11+
"html/template"
1112
"strconv"
1213
"unicode/utf8"
1314

@@ -259,8 +260,8 @@ type Comment struct {
259260
CommitID int64
260261
Line int64 // - previous line / + proposed line
261262
TreePath string
262-
Content string `xorm:"LONGTEXT"`
263-
RenderedContent string `xorm:"-"`
263+
Content string `xorm:"LONGTEXT"`
264+
RenderedContent template.HTML `xorm:"-"`
264265

265266
// Path represents the 4 lines of code cemented by this comment
266267
Patch string `xorm:"-"`

‎models/issues/issue.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package issues
77
import (
88
"context"
99
"fmt"
10+
"html/template"
1011
"regexp"
1112
"slices"
1213

@@ -105,7 +106,7 @@ type Issue struct {
105106
OriginalAuthorID int64 `xorm:"index"`
106107
Title string `xorm:"name"`
107108
Content string `xorm:"LONGTEXT"`
108-
RenderedContent string `xorm:"-"`
109+
RenderedContent template.HTML `xorm:"-"`
109110
Labels []*Label `xorm:"-"`
110111
MilestoneID int64 `xorm:"INDEX"`
111112
Milestone *Milestone `xorm:"-"`

‎models/issues/milestone.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package issues
66
import (
77
"context"
88
"fmt"
9+
"html/template"
910
"strings"
1011

1112
"code.gitea.io/gitea/models/db"
@@ -47,8 +48,8 @@ type Milestone struct {
4748
RepoID int64 `xorm:"INDEX"`
4849
Repo *repo_model.Repository `xorm:"-"`
4950
Name string
50-
Content string `xorm:"TEXT"`
51-
RenderedContent string `xorm:"-"`
51+
Content string `xorm:"TEXT"`
52+
RenderedContent template.HTML `xorm:"-"`
5253
IsClosed bool
5354
NumIssues int
5455
NumClosedIssues int

‎models/project/project.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package project
66
import (
77
"context"
88
"fmt"
9+
"html/template"
910

1011
"code.gitea.io/gitea/models/db"
1112
repo_model "code.gitea.io/gitea/models/repo"
@@ -100,7 +101,7 @@ type Project struct {
100101
CardType CardType
101102
Type Type
102103

103-
RenderedContent string `xorm:"-"`
104+
RenderedContent template.HTML `xorm:"-"`
104105

105106
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
106107
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`

‎models/repo/release.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package repo
77
import (
88
"context"
99
"fmt"
10+
"html/template"
1011
"net/url"
1112
"sort"
1213
"strconv"
@@ -80,7 +81,7 @@ type Release struct {
8081
NumCommits int64
8182
NumCommitsBehind int64 `xorm:"-"`
8283
Note string `xorm:"TEXT"`
83-
RenderedNote string `xorm:"-"`
84+
RenderedNote template.HTML `xorm:"-"`
8485
IsDraft bool `xorm:"NOT NULL DEFAULT false"`
8586
IsPrerelease bool `xorm:"NOT NULL DEFAULT false"`
8687
IsTag bool `xorm:"NOT NULL DEFAULT false"` // will be true only if the record is a tag and has no related releases

‎modules/markup/html_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ func TestRender_ShortLinks(t *testing.T) {
397397
},
398398
}, input)
399399
assert.NoError(t, err)
400-
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
400+
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer)))
401401
buffer, err = markdown.RenderString(&markup.RenderContext{
402402
Ctx: git.DefaultContext,
403403
Links: markup.Links{
@@ -407,7 +407,7 @@ func TestRender_ShortLinks(t *testing.T) {
407407
IsWiki: true,
408408
}, input)
409409
assert.NoError(t, err)
410-
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(buffer))
410+
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer)))
411411
}
412412

413413
mediatree := util.URLJoin(markup.TestRepoURL, "media", "master")
@@ -510,7 +510,7 @@ func TestRender_RelativeImages(t *testing.T) {
510510
Metas: localMetas,
511511
}, input)
512512
assert.NoError(t, err)
513-
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
513+
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer)))
514514
buffer, err = markdown.RenderString(&markup.RenderContext{
515515
Ctx: git.DefaultContext,
516516
Links: markup.Links{
@@ -520,7 +520,7 @@ func TestRender_RelativeImages(t *testing.T) {
520520
IsWiki: true,
521521
}, input)
522522
assert.NoError(t, err)
523-
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(buffer))
523+
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer)))
524524
}
525525

526526
rawwiki := util.URLJoin(markup.TestRepoURL, "wiki", "raw")

‎modules/markup/markdown/markdown.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package markdown
66

77
import (
88
"fmt"
9+
"html/template"
910
"io"
1011
"strings"
1112
"sync"
@@ -262,12 +263,12 @@ func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error
262263
}
263264

264265
// RenderString renders Markdown string to HTML with all specific handling stuff and return string
265-
func RenderString(ctx *markup.RenderContext, content string) (string, error) {
266+
func RenderString(ctx *markup.RenderContext, content string) (template.HTML, error) {
266267
var buf strings.Builder
267268
if err := Render(ctx, strings.NewReader(content), &buf); err != nil {
268269
return "", err
269270
}
270-
return buf.String(), nil
271+
return template.HTML(buf.String()), nil
271272
}
272273

273274
// RenderRaw renders Markdown to HTML without handling special links.

‎modules/markup/markdown/markdown_test.go

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package markdown_test
55

66
import (
77
"context"
8+
"html/template"
89
"os"
910
"strings"
1011
"testing"
@@ -56,7 +57,7 @@ func TestRender_StandardLinks(t *testing.T) {
5657
},
5758
}, input)
5859
assert.NoError(t, err)
59-
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
60+
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer)))
6061

6162
buffer, err = markdown.RenderString(&markup.RenderContext{
6263
Ctx: git.DefaultContext,
@@ -66,7 +67,7 @@ func TestRender_StandardLinks(t *testing.T) {
6667
IsWiki: true,
6768
}, input)
6869
assert.NoError(t, err)
69-
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(buffer))
70+
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer)))
7071
}
7172

7273
googleRendered := `<p><a href="https://google.com/" rel="nofollow">https://google.com/</a></p>`
@@ -90,7 +91,7 @@ func TestRender_Images(t *testing.T) {
9091
},
9192
}, input)
9293
assert.NoError(t, err)
93-
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
94+
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer)))
9495
}
9596

9697
url := "../../.images/src/02/train.jpg"
@@ -299,7 +300,7 @@ func TestTotal_RenderWiki(t *testing.T) {
299300
IsWiki: true,
300301
}, sameCases[i])
301302
assert.NoError(t, err)
302-
assert.Equal(t, answers[i], line)
303+
assert.Equal(t, template.HTML(answers[i]), line)
303304
}
304305

305306
testCases := []string{
@@ -324,7 +325,7 @@ func TestTotal_RenderWiki(t *testing.T) {
324325
IsWiki: true,
325326
}, testCases[i])
326327
assert.NoError(t, err)
327-
assert.Equal(t, testCases[i+1], line)
328+
assert.Equal(t, template.HTML(testCases[i+1]), line)
328329
}
329330
}
330331

@@ -343,7 +344,7 @@ func TestTotal_RenderString(t *testing.T) {
343344
Metas: localMetas,
344345
}, sameCases[i])
345346
assert.NoError(t, err)
346-
assert.Equal(t, answers[i], line)
347+
assert.Equal(t, template.HTML(answers[i]), line)
347348
}
348349

349350
testCases := []string{}
@@ -356,7 +357,7 @@ func TestTotal_RenderString(t *testing.T) {
356357
},
357358
}, testCases[i])
358359
assert.NoError(t, err)
359-
assert.Equal(t, testCases[i+1], line)
360+
assert.Equal(t, template.HTML(testCases[i+1]), line)
360361
}
361362
}
362363

@@ -423,7 +424,7 @@ func TestRenderEmojiInLinks_Issue12331(t *testing.T) {
423424
`
424425
res, err := markdown.RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, testcase)
425426
assert.NoError(t, err)
426-
assert.Equal(t, expected, res)
427+
assert.Equal(t, template.HTML(expected), res)
427428
}
428429

429430
func TestColorPreview(t *testing.T) {
@@ -457,7 +458,7 @@ func TestColorPreview(t *testing.T) {
457458
for _, test := range positiveTests {
458459
res, err := markdown.RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, test.testcase)
459460
assert.NoError(t, err, "Unexpected error in testcase: %q", test.testcase)
460-
assert.Equal(t, test.expected, res, "Unexpected result in testcase %q", test.testcase)
461+
assert.Equal(t, template.HTML(test.expected), res, "Unexpected result in testcase %q", test.testcase)
461462

462463
}
463464

@@ -524,7 +525,7 @@ func TestMathBlock(t *testing.T) {
524525
for _, test := range testcases {
525526
res, err := markdown.RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, test.testcase)
526527
assert.NoError(t, err, "Unexpected error in testcase: %q", test.testcase)
527-
assert.Equal(t, test.expected, res, "Unexpected result in testcase %q", test.testcase)
528+
assert.Equal(t, template.HTML(test.expected), res, "Unexpected result in testcase %q", test.testcase)
528529

529530
}
530531
}
@@ -562,12 +563,12 @@ foo: bar
562563
for _, test := range testcases {
563564
res, err := markdown.RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, test.testcase)
564565
assert.NoError(t, err, "Unexpected error in testcase: %q", test.testcase)
565-
assert.Equal(t, test.expected, res, "Unexpected result in testcase %q", test.testcase)
566+
assert.Equal(t, template.HTML(test.expected), res, "Unexpected result in testcase %q", test.testcase)
566567
}
567568
}
568569

569570
func TestRenderLinks(t *testing.T) {
570-
input := ` space @mention-user
571+
input := ` space @mention-user${SPACE}${SPACE}
571572
/just/a/path.bin
572573
https://example.com/file.bin
573574
[local link](file.bin)
@@ -588,8 +589,9 @@ com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
588589
[email protected]
589590
@mention-user test
590591
#123
591-
space
592+
space${SPACE}${SPACE}
592593
`
594+
input = strings.ReplaceAll(input, "${SPACE}", " ") // replace ${SPACE} with " ", to avoid some editor's auto-trimming
593595
cases := []struct {
594596
Links markup.Links
595597
IsWiki bool
@@ -952,6 +954,6 @@ space</p>
952954
for i, c := range cases {
953955
result, err := markdown.RenderString(&markup.RenderContext{Ctx: context.Background(), Links: c.Links, IsWiki: c.IsWiki}, input)
954956
assert.NoError(t, err, "Unexpected error in testcase: %v", i)
955-
assert.Equal(t, c.Expected, result, "Unexpected result in testcase %v", i)
957+
assert.Equal(t, template.HTML(c.Expected), result, "Unexpected result in testcase %v", i)
956958
}
957959
}

‎modules/templates/util_render.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func RenderMarkdownToHtml(ctx context.Context, input string) template.HTML { //n
208208
if err != nil {
209209
log.Error("RenderString: %v", err)
210210
}
211-
return template.HTML(output)
211+
return output
212212
}
213213

214214
func RenderLabels(ctx context.Context, labels []*issues_model.Label, repoLink string) template.HTML {

‎modules/templates/util_string.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package templates
55

66
import (
7+
"fmt"
8+
"html/template"
79
"strings"
810

911
"code.gitea.io/gitea/modules/base"
@@ -17,6 +19,19 @@ func NewStringUtils() *StringUtils {
1719
return &stringUtils
1820
}
1921

22+
func (su *StringUtils) ToString(v any) string {
23+
switch v := v.(type) {
24+
case string:
25+
return v
26+
case template.HTML:
27+
return string(v)
28+
case fmt.Stringer:
29+
return v.String()
30+
default:
31+
return fmt.Sprint(v)
32+
}
33+
}
34+
2035
func (su *StringUtils) HasPrefix(s, prefix string) bool {
2136
return strings.HasPrefix(s, prefix)
2237
}

‎routers/web/feed/convert.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func toReleaseLink(ctx *context.Context, act *activities_model.Action) string {
5050

5151
// renderMarkdown creates a minimal markdown render context from an action.
5252
// If rendering fails, the original markdown text is returned
53-
func renderMarkdown(ctx *context.Context, act *activities_model.Action, content string) string {
53+
func renderMarkdown(ctx *context.Context, act *activities_model.Action, content string) template.HTML {
5454
markdownCtx := &markup.RenderContext{
5555
Ctx: ctx,
5656
Links: markup.Links{
@@ -64,7 +64,7 @@ func renderMarkdown(ctx *context.Context, act *activities_model.Action, content
6464
}
6565
markdown, err := markdown.RenderString(markdownCtx, content)
6666
if err != nil {
67-
return content
67+
return templates.Str2html(content) // old code did so: use Str2html to render in tmpl
6868
}
6969
return markdown
7070
}
@@ -74,7 +74,11 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio
7474
for _, act := range actions {
7575
act.LoadActUser(ctx)
7676

77-
var content, desc, title string
77+
// TODO: the code seems quite strange (maybe not right)
78+
// sometimes it uses text content but sometimes it uses HTML content
79+
// it should clearly defines which kind of content it should use for the feed items: plan text or rich HTML
80+
var title, desc string
81+
var content template.HTML
7882

7983
link := &feeds.Link{Href: act.GetCommentHTMLURL(ctx)}
8084

@@ -228,7 +232,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio
228232
desc = act.GetIssueTitle(ctx)
229233
comment := act.GetIssueInfos()[1]
230234
if len(comment) != 0 {
231-
desc += "\n\n" + renderMarkdown(ctx, act, comment)
235+
desc += "\n\n" + string(renderMarkdown(ctx, act, comment))
232236
}
233237
case activities_model.ActionMergePullRequest, activities_model.ActionAutoMergePullRequest:
234238
desc = act.GetIssueInfos()[1]
@@ -239,7 +243,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio
239243
}
240244
}
241245
if len(content) == 0 {
242-
content = desc
246+
content = templates.Str2html(desc)
243247
}
244248

245249
items = append(items, &feeds.Item{
@@ -253,7 +257,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio
253257
},
254258
Id: fmt.Sprintf("%v: %v", strconv.FormatInt(act.ID, 10), link.Href),
255259
Created: act.CreatedUnix.AsTime(),
256-
Content: content,
260+
Content: string(content),
257261
})
258262
}
259263
return items, err
@@ -282,7 +286,8 @@ func releasesToFeedItems(ctx *context.Context, releases []*repo_model.Release, i
282286
return nil, err
283287
}
284288

285-
var title, content string
289+
var title string
290+
var content template.HTML
286291

287292
if rel.IsTag {
288293
title = rel.TagName
@@ -311,7 +316,7 @@ func releasesToFeedItems(ctx *context.Context, releases []*repo_model.Release, i
311316
Email: rel.Publisher.GetEmail(),
312317
},
313318
Id: fmt.Sprintf("%v: %v", strconv.FormatInt(rel.ID, 10), link.Href),
314-
Content: content,
319+
Content: string(content),
315320
})
316321
}
317322

0 commit comments

Comments
 (0)