From bfec14a857e69949bf385f81dd296c5e24c832b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=B4=8A=E1=B4=8F=E1=B4=87=20=E1=B4=84=CA=9C=E1=B4=87?= =?UTF-8?q?=C9=B4?= Date: Tue, 19 May 2026 12:20:13 -0400 Subject: [PATCH] refactor: render mail templates with html/template directly (#8272) --- internal/email/email.go | 158 +++++++++++++++++++++++++++-------- internal/email/email_test.go | 124 +++++++++++++++++++++++++++ templates/embed.go | 21 +++++ 3 files changed, 268 insertions(+), 35 deletions(-) create mode 100644 internal/email/email_test.go diff --git a/internal/email/email.go b/internal/email/email.go index 1dfd0adf1..64739ecf9 100644 --- a/internal/email/email.go +++ b/internal/email/email.go @@ -1,15 +1,19 @@ package email import ( + "bytes" "fmt" "html/template" + "io/fs" "net/mail" + "os" "path/filepath" + "slices" + "strings" "sync" "time" "github.com/cockroachdb/errors" - "gopkg.in/macaron.v1" "gogs.io/gogs/internal/conf" "gogs.io/gogs/internal/markup" @@ -37,46 +41,130 @@ const ( ) var ( - tplRender *macaron.TplRender - tplRenderOnce sync.Once + tplSet *template.Template + tplSetOnce sync.Once + tplSetErr error ) -// render renders a mail template with given data. -func render(tpl string, data map[string]any) (string, error) { - tplRenderOnce.Do(func() { - customDir := filepath.Join(conf.CustomDir(), "templates") - opt := &macaron.RenderOptions{ - Directory: filepath.Join(conf.WorkDir(), "templates", "mail"), - AppendDirectories: []string{filepath.Join(customDir, "mail")}, - Extensions: []string{".tmpl", ".html"}, - Funcs: []template.FuncMap{map[string]any{ - "AppName": func() string { - return conf.App.BrandName - }, - "AppURL": func() string { - return conf.Server.ExternalURL - }, - "Year": func() int { - return time.Now().Year() - }, - "Str2HTML": func(raw string) template.HTML { - return template.HTML(markup.Sanitize(raw)) - }, - }}, - } - if !conf.Server.LoadAssetsFromDisk { - opt.TemplateFileSystem = templates.NewTemplateFileSystem("mail", customDir) - } +func funcMap() template.FuncMap { + return template.FuncMap{ + "AppName": func() string { + return conf.App.BrandName + }, + "AppURL": func() string { + return conf.Server.ExternalURL + }, + "Year": func() int { + return time.Now().Year() + }, + "Str2HTML": func(raw string) template.HTML { + return template.HTML(markup.Sanitize(raw)) + }, + } +} - ts := macaron.NewTemplateSet() - ts.Set(macaron.DEFAULT_TPL_SET_NAME, opt) - tplRender = &macaron.TplRender{ - TemplateSet: ts, - Opt: opt, +// Recognized mail-template file extensions. A template's name is its path +// relative to the "mail" directory, without extension (e.g. "auth/activate"). +var mailTemplateExts = []string{".tmpl", ".html"} + +// loadMailTemplates parses every mail template under the embedded "mail" tree +// (or "/templates/mail" when LoadAssetsFromDisk is set), then overlays +// files from "/templates/mail" so an admin can override any builtin. +func loadMailTemplates() (*template.Template, error) { + root := template.New("").Funcs(funcMap()) + parse := func(name string, data []byte) error { + _, err := root.New(name).Parse(string(data)) + return errors.Wrapf(err, "parse %q", name) + } + + if conf.Server.LoadAssetsFromDisk { + baseRoot := filepath.Join(conf.WorkDir(), "templates", "mail") + if _, err := os.Stat(baseRoot); err != nil { + return nil, errors.Wrapf(err, "stat base mail templates %q", baseRoot) } + if err := overlayDiskMailTemplates(baseRoot, parse); err != nil { + return nil, err + } + } else { + for _, name := range templates.MailFileNames() { + ext := strings.ToLower(filepath.Ext(name)) + if !slices.Contains(mailTemplateExts, ext) { + continue + } + data, err := templates.ReadMailFile(name) + if err != nil { + return nil, errors.Wrapf(err, "read embedded %q", name) + } + if err := parse(strings.TrimSuffix(filepath.ToSlash(name), ext), data); err != nil { + return nil, err + } + } + } + if err := overlayDiskMailTemplates(filepath.Join(conf.CustomDir(), "templates", "mail"), parse); err != nil { + return nil, err + } + return root, nil +} + +// overlayDiskMailTemplates walks root and parses every recognized template +// file via parse. A missing root is not an error: custom overrides are optional. +func overlayDiskMailTemplates(root string, parse func(name string, data []byte) error) error { + return filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return fs.SkipAll + } + return err + } + if d.IsDir() { + return nil + } + ext := strings.ToLower(filepath.Ext(p)) + if !slices.Contains(mailTemplateExts, ext) { + return nil + } + data, err := os.ReadFile(p) + if err != nil { + return errors.Wrapf(err, "read %q", p) + } + rel, err := filepath.Rel(root, p) + if err != nil { + return err + } + return parse(strings.TrimSuffix(filepath.ToSlash(rel), ext), data) }) +} - return tplRender.HTMLString(tpl, data) +func render(tpl string, data map[string]any) (string, error) { + set, err := mailTemplateSet() + if err != nil { + return "", errors.Wrap(err, "load mail templates") + } + t := set.Lookup(tpl) + if t == nil { + return "", errors.Newf("mail template %q not found", tpl) + } + var buf bytes.Buffer + if err := t.Execute(&buf, data); err != nil { + return "", errors.Wrapf(err, "execute %q", tpl) + } + return buf.String(), nil +} + +// mailTemplateSet returns the parsed template set. When assets are loaded from +// disk, templates are reloaded on every call so admin edits under +// /templates/mail or /templates/mail take effect without a +// restart — matching the hot-reload behavior of the previous macaron renderer +// for non-production environments. When assets are embedded, the set is loaded +// once and cached for the process lifetime. +func mailTemplateSet() (*template.Template, error) { + if conf.Server.LoadAssetsFromDisk { + return loadMailTemplates() + } + tplSetOnce.Do(func() { + tplSet, tplSetErr = loadMailTemplates() + }) + return tplSet, tplSetErr } func SendTestMail(email string) error { diff --git a/internal/email/email_test.go b/internal/email/email_test.go new file mode 100644 index 000000000..2ca1af847 --- /dev/null +++ b/internal/email/email_test.go @@ -0,0 +1,124 @@ +package email + +import ( + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "gogs.io/gogs/internal/conf" +) + +// TestRenderEmbeddedTemplates ensures every builtin mail template parses and +// executes against the data shape its production caller supplies, so a syntax +// regression or missing field is caught at build time, not on the first email. +func TestRenderEmbeddedTemplates(t *testing.T) { + conf.SetMockApp(t, conf.AppOpts{BrandName: "Gogs"}) + conf.SetMockServer(t, conf.ServerOpts{ + ExternalURL: "https://example.test/", + LoadAssetsFromDisk: false, + }) + resetTemplateCache(t) + + tests := []struct { + name string + data map[string]any + }{ + { + name: tmplAuthActivate, + data: map[string]any{ + "Username": "alice", + "ActiveCodeLives": 1440, + "ResetPwdCodeLives": 1440, + "Code": "abc", + }, + }, + { + name: tmplAuthActivateEmail, + data: map[string]any{ + "Username": "alice", + "ActiveCodeLives": 1440, + "Code": "abc", + "Email": "alice@example.test", + }, + }, + { + name: tmplAuthResetPassword, + data: map[string]any{ + "Username": "alice", + "ActiveCodeLives": 1440, + "ResetPwdCodeLives": 1440, + "Code": "abc", + }, + }, + { + name: tmplAuthRegisterNotify, + data: map[string]any{"Username": "alice"}, + }, + { + name: tmplNotifyCollaborator, + data: map[string]any{ + "Subject": "alice added you to bob/repo", + "RepoName": "bob/repo", + "Link": "https://example.test/bob/repo", + }, + }, + { + name: tmplIssueComment, + data: map[string]any{ + "Subject": "[bob/repo] Re: Issue title", + "Body": "

comment body

", + "Link": "https://example.test/bob/repo/issues/1", + "Doer": testDoer{}, + }, + }, + { + name: tmplIssueMention, + data: map[string]any{ + "Subject": "[bob/repo] @alice mentioned you", + "Body": "

mention body

", + "Link": "https://example.test/bob/repo/issues/1", + "Doer": testDoer{}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + body, err := render(tc.name, tc.data) + require.NoError(t, err) + assert.NotEmpty(t, body) + assert.False(t, strings.Contains(body, ""), "template referenced a missing data key") + }) + } +} + +// TestRenderUnknownTemplate asserts callers get a useful error rather than an +// empty body when asking for a name that doesn't exist. +func TestRenderUnknownTemplate(t *testing.T) { + conf.SetMockServer(t, conf.ServerOpts{LoadAssetsFromDisk: false}) + resetTemplateCache(t) + + _, err := render("does/not/exist", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "not found") +} + +// resetTemplateCache forces the next render call to reload templates, so each +// test starts from a clean state regardless of execution order. +func resetTemplateCache(t *testing.T) { + t.Helper() + tplSet = nil + tplSetErr = nil + tplSetOnce = sync.Once{} +} + +// testDoer satisfies the User interface for fields the issue templates touch. +type testDoer struct{} + +func (testDoer) ID() int64 { return 1 } +func (testDoer) DisplayName() string { return "alice" } +func (testDoer) Email() string { return "alice@example.test" } +func (testDoer) GenerateEmailActivateCode(string) string { return "abc" } diff --git a/templates/embed.go b/templates/embed.go index 945b17d0b..4af12426e 100644 --- a/templates/embed.go +++ b/templates/embed.go @@ -39,6 +39,9 @@ func (fs *fileSystem) Get(name string) (io.Reader, error) { func mustNames(fsys fs.FS) []string { var names []string walkDirFunc := func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } if !d.IsDir() { names = append(names, path) } @@ -50,6 +53,24 @@ func mustNames(fsys fs.FS) []string { return names } +// MailFileNames returns the embedded template file paths under "mail/", +// each relative to the "mail" directory (e.g. "auth/activate.tmpl"). +func MailFileNames() []string { + var names []string + for _, name := range mustNames(files) { + if rel, ok := strings.CutPrefix(name, "mail/"); ok { + names = append(names, rel) + } + } + return names +} + +// ReadMailFile returns the embedded mail template bytes at the given path +// relative to the "mail" directory. +func ReadMailFile(name string) ([]byte, error) { + return files.ReadFile(path.Join("mail", name)) +} + // NewTemplateFileSystem returns a macaron.TemplateFileSystem instance for embedded assets. // The argument "dir" can be used to serve subset of embedded assets. Template file // found under the "customDir" on disk has higher precedence over embedded assets.