diff options
Diffstat (limited to 'modules')
| -rw-r--r-- | modules/context/context.go | 44 | ||||
| -rw-r--r-- | modules/templates/htmlrenderer.go | 251 | ||||
| -rw-r--r-- | modules/templates/htmlrenderer_test.go | 106 |
3 files changed, 242 insertions, 159 deletions
diff --git a/modules/context/context.go b/modules/context/context.go index cee533e42a..2507cc10c0 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -16,10 +16,8 @@ import ( "net/http" "net/url" "path" - "regexp" "strconv" "strings" - texttemplate "text/template" "time" "code.gitea.io/gitea/models/db" @@ -216,7 +214,7 @@ func (ctx *Context) RedirectToFirst(location ...string) { ctx.Redirect(setting.AppSubURL + "/") } -var templateExecutingErr = regexp.MustCompile(`^template: (.*):([1-9][0-9]*):([1-9][0-9]*): executing (?:"(.*)" at <(.*)>: )?`) +const tplStatus500 base.TplName = "status/500" // HTML calls Context.HTML and renders the template to HTTP response func (ctx *Context) HTML(status int, name base.TplName) { @@ -229,34 +227,11 @@ func (ctx *Context) HTML(status int, name base.TplName) { return strconv.FormatInt(time.Since(tmplStartTime).Nanoseconds()/1e6, 10) + "ms" } if err := ctx.Render.HTML(ctx.Resp, status, string(name), templates.BaseVars().Merge(ctx.Data)); err != nil { - if status == http.StatusInternalServerError && name == base.TplName("status/500") { + if status == http.StatusInternalServerError && name == tplStatus500 { ctx.PlainText(http.StatusInternalServerError, "Unable to find HTML templates, the template system is not initialized, or Gitea can't find your template files.") return } - if execErr, ok := err.(texttemplate.ExecError); ok { - if groups := templateExecutingErr.FindStringSubmatch(err.Error()); len(groups) > 0 { - errorTemplateName, lineStr, posStr := groups[1], groups[2], groups[3] - target := "" - if len(groups) == 6 { - target = groups[5] - } - line, _ := strconv.Atoi(lineStr) // Cannot error out as groups[2] is [1-9][0-9]* - pos, _ := strconv.Atoi(posStr) // Cannot error out as groups[3] is [1-9][0-9]* - assetLayerName := templates.AssetFS().GetFileLayerName(errorTemplateName + ".tmpl") - filename := fmt.Sprintf("(%s) %s", assetLayerName, errorTemplateName) - if errorTemplateName != string(name) { - filename += " (subtemplate of " + string(name) + ")" - } - err = fmt.Errorf("failed to render %s, error: %w:\n%s", filename, err, templates.GetLineFromTemplate(errorTemplateName, line, target, pos)) - } else { - assetLayerName := templates.AssetFS().GetFileLayerName(execErr.Name + ".tmpl") - filename := fmt.Sprintf("(%s) %s", assetLayerName, execErr.Name) - if execErr.Name != string(name) { - filename += " (subtemplate of " + string(name) + ")" - } - err = fmt.Errorf("failed to render %s, error: %w", filename, err) - } - } + err = fmt.Errorf("failed to render template: %s, error: %s", name, templates.HandleTemplateRenderingError(err)) ctx.ServerError("Render failed", err) } } @@ -324,24 +299,25 @@ func (ctx *Context) serverErrorInternal(logMsg string, logErr error) { return } - if !setting.IsProd { + // it's safe to show internal error to admin users, and it helps + if !setting.IsProd || (ctx.Doer != nil && ctx.Doer.IsAdmin) { ctx.Data["ErrorMsg"] = logErr } } ctx.Data["Title"] = "Internal Server Error" - ctx.HTML(http.StatusInternalServerError, base.TplName("status/500")) + ctx.HTML(http.StatusInternalServerError, tplStatus500) } // NotFoundOrServerError use error check function to determine if the error // is about not found. It responds with 404 status code for not found error, // or error context description for logging purpose of 500 server error. -func (ctx *Context) NotFoundOrServerError(logMsg string, errCheck func(error) bool, err error) { - if errCheck(err) { - ctx.notFoundInternal(logMsg, err) +func (ctx *Context) NotFoundOrServerError(logMsg string, errCheck func(error) bool, logErr error) { + if errCheck(logErr) { + ctx.notFoundInternal(logMsg, logErr) return } - ctx.serverErrorInternal(logMsg, err) + ctx.serverErrorInternal(logMsg, logErr) } // PlainTextBytes renders bytes as plain text diff --git a/modules/templates/htmlrenderer.go b/modules/templates/htmlrenderer.go index 26dd365e4c..833c2acdca 100644 --- a/modules/templates/htmlrenderer.go +++ b/modules/templates/htmlrenderer.go @@ -4,6 +4,7 @@ package templates import ( + "bufio" "bytes" "context" "errors" @@ -18,19 +19,13 @@ import ( "sync/atomic" texttemplate "text/template" + "code.gitea.io/gitea/modules/assetfs" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" ) -var ( - rendererKey interface{} = "templatesHtmlRenderer" - - templateError = regexp.MustCompile(`^template: (.*):([0-9]+): (.*)`) - notDefinedError = regexp.MustCompile(`^template: (.*):([0-9]+): function "(.*)" not defined`) - unexpectedError = regexp.MustCompile(`^template: (.*):([0-9]+): unexpected "(.*)" in operand`) - expectedEndError = regexp.MustCompile(`^template: (.*):([0-9]+): expected end; found (.*)`) -) +var rendererKey interface{} = "templatesHtmlRenderer" type HTMLRender struct { templates atomic.Pointer[template.Template] @@ -107,11 +102,12 @@ func HTMLRenderer(ctx context.Context) (context.Context, *HTMLRender) { renderer := &HTMLRender{} if err := renderer.CompileTemplates(); err != nil { - wrapFatal(handleNotDefinedPanicError(err)) - wrapFatal(handleUnexpected(err)) - wrapFatal(handleExpectedEnd(err)) - wrapFatal(handleGenericTemplateError(err)) - log.Fatal("HTMLRenderer error: %v", err) + p := &templateErrorPrettier{assets: AssetFS()} + wrapFatal(p.handleFuncNotDefinedError(err)) + wrapFatal(p.handleUnexpectedOperandError(err)) + wrapFatal(p.handleExpectedEndError(err)) + wrapFatal(p.handleGenericTemplateError(err)) + log.Fatal("HTMLRenderer CompileTemplates error: %v", err) } if !setting.IsProd { go AssetFS().WatchLocalChanges(ctx, func() { @@ -123,148 +119,153 @@ func HTMLRenderer(ctx context.Context) (context.Context, *HTMLRender) { return context.WithValue(ctx, rendererKey, renderer), renderer } -func wrapFatal(format string, args []interface{}) { - if format == "" { +func wrapFatal(msg string) { + if msg == "" { return } - log.FatalWithSkip(1, format, args...) + log.FatalWithSkip(1, "Unable to compile templates, %s", msg) } -func handleGenericTemplateError(err error) (string, []interface{}) { - groups := templateError.FindStringSubmatch(err.Error()) - if len(groups) != 4 { - return "", nil - } - - templateName, lineNumberStr, message := groups[1], groups[2], groups[3] - filename := fmt.Sprintf("%s (provided by %s)", templateName, AssetFS().GetFileLayerName(templateName+".tmpl")) - lineNumber, _ := strconv.Atoi(lineNumberStr) - line := GetLineFromTemplate(templateName, lineNumber, "", -1) - - return "PANIC: Unable to compile templates!\n%s in template file %s at line %d:\n\n%s\nStacktrace:\n\n%s", []interface{}{message, filename, lineNumber, log.NewColoredValue(line, log.Reset), log.Stack(2)} +type templateErrorPrettier struct { + assets *assetfs.LayeredFS } -func handleNotDefinedPanicError(err error) (string, []interface{}) { - groups := notDefinedError.FindStringSubmatch(err.Error()) - if len(groups) != 4 { - return "", nil - } - - templateName, lineNumberStr, functionName := groups[1], groups[2], groups[3] - functionName, _ = strconv.Unquote(`"` + functionName + `"`) - filename := fmt.Sprintf("%s (provided by %s)", templateName, AssetFS().GetFileLayerName(templateName+".tmpl")) - lineNumber, _ := strconv.Atoi(lineNumberStr) - line := GetLineFromTemplate(templateName, lineNumber, functionName, -1) +var reGenericTemplateError = regexp.MustCompile(`^template: (.*):([0-9]+): (.*)`) - return "PANIC: Unable to compile templates!\nUndefined function %q in template file %s at line %d:\n\n%s", []interface{}{functionName, filename, lineNumber, log.NewColoredValue(line, log.Reset)} -} - -func handleUnexpected(err error) (string, []interface{}) { - groups := unexpectedError.FindStringSubmatch(err.Error()) +func (p *templateErrorPrettier) handleGenericTemplateError(err error) string { + groups := reGenericTemplateError.FindStringSubmatch(err.Error()) if len(groups) != 4 { - return "", nil + return "" } - - templateName, lineNumberStr, unexpected := groups[1], groups[2], groups[3] - unexpected, _ = strconv.Unquote(`"` + unexpected + `"`) - filename := fmt.Sprintf("%s (provided by %s)", templateName, AssetFS().GetFileLayerName(templateName+".tmpl")) - lineNumber, _ := strconv.Atoi(lineNumberStr) - line := GetLineFromTemplate(templateName, lineNumber, unexpected, -1) - - return "PANIC: Unable to compile templates!\nUnexpected %q in template file %s at line %d:\n\n%s", []interface{}{unexpected, filename, lineNumber, log.NewColoredValue(line, log.Reset)} + tmplName, lineStr, message := groups[1], groups[2], groups[3] + return p.makeDetailedError(message, tmplName, lineStr, -1, "") } -func handleExpectedEnd(err error) (string, []interface{}) { - groups := expectedEndError.FindStringSubmatch(err.Error()) - if len(groups) != 4 { - return "", nil - } - - templateName, lineNumberStr, unexpected := groups[1], groups[2], groups[3] - filename := fmt.Sprintf("%s (provided by %s)", templateName, AssetFS().GetFileLayerName(templateName+".tmpl")) - lineNumber, _ := strconv.Atoi(lineNumberStr) - line := GetLineFromTemplate(templateName, lineNumber, unexpected, -1) +var reFuncNotDefinedError = regexp.MustCompile(`^template: (.*):([0-9]+): (function "(.*)" not defined)`) - return "PANIC: Unable to compile templates!\nMissing end with unexpected %q in template file %s at line %d:\n\n%s", []interface{}{unexpected, filename, lineNumber, log.NewColoredValue(line, log.Reset)} +func (p *templateErrorPrettier) handleFuncNotDefinedError(err error) string { + groups := reFuncNotDefinedError.FindStringSubmatch(err.Error()) + if len(groups) != 5 { + return "" + } + tmplName, lineStr, message, funcName := groups[1], groups[2], groups[3], groups[4] + funcName, _ = strconv.Unquote(`"` + funcName + `"`) + return p.makeDetailedError(message, tmplName, lineStr, -1, funcName) } -const dashSeparator = "----------------------------------------------------------------------\n" +var reUnexpectedOperandError = regexp.MustCompile(`^template: (.*):([0-9]+): (unexpected "(.*)" in operand)`) -// GetLineFromTemplate returns a line from a template with some context -func GetLineFromTemplate(templateName string, targetLineNum int, target string, position int) string { - bs, err := AssetFS().ReadFile(templateName + ".tmpl") - if err != nil { - return fmt.Sprintf("(unable to read template file: %v)", err) +func (p *templateErrorPrettier) handleUnexpectedOperandError(err error) string { + groups := reUnexpectedOperandError.FindStringSubmatch(err.Error()) + if len(groups) != 5 { + return "" } + tmplName, lineStr, message, unexpected := groups[1], groups[2], groups[3], groups[4] + unexpected, _ = strconv.Unquote(`"` + unexpected + `"`) + return p.makeDetailedError(message, tmplName, lineStr, -1, unexpected) +} - sb := &strings.Builder{} - - // Write the header - sb.WriteString(dashSeparator) +var reExpectedEndError = regexp.MustCompile(`^template: (.*):([0-9]+): (expected end; found (.*))`) - var lineBs []byte +func (p *templateErrorPrettier) handleExpectedEndError(err error) string { + groups := reExpectedEndError.FindStringSubmatch(err.Error()) + if len(groups) != 5 { + return "" + } + tmplName, lineStr, message, unexpected := groups[1], groups[2], groups[3], groups[4] + return p.makeDetailedError(message, tmplName, lineStr, -1, unexpected) +} - // Iterate through the lines from the asset file to find the target line - for start, currentLineNum := 0, 1; currentLineNum <= targetLineNum && start < len(bs); currentLineNum++ { - // Find the next new line - end := bytes.IndexByte(bs[start:], '\n') +var ( + reTemplateExecutingError = regexp.MustCompile(`^template: (.*):([1-9][0-9]*):([1-9][0-9]*): (executing .*)`) + reTemplateExecutingErrorMsg = regexp.MustCompile(`^executing "(.*)" at <(.*)>: `) +) - // adjust the end to be a direct pointer in to []byte - if end < 0 { - end = len(bs) - } else { - end += start +func (p *templateErrorPrettier) handleTemplateRenderingError(err error) string { + if groups := reTemplateExecutingError.FindStringSubmatch(err.Error()); len(groups) > 0 { + tmplName, lineStr, posStr, msgPart := groups[1], groups[2], groups[3], groups[4] + target := "" + if groups = reTemplateExecutingErrorMsg.FindStringSubmatch(msgPart); len(groups) > 0 { + target = groups[2] } + return p.makeDetailedError(msgPart, tmplName, lineStr, posStr, target) + } else if execErr, ok := err.(texttemplate.ExecError); ok { + layerName := p.assets.GetFileLayerName(execErr.Name + ".tmpl") + return fmt.Sprintf("asset from: %s, %s", layerName, err.Error()) + } else { + return err.Error() + } +} - // set lineBs to the current line []byte - lineBs = bs[start:end] +func HandleTemplateRenderingError(err error) string { + p := &templateErrorPrettier{assets: AssetFS()} + return p.handleTemplateRenderingError(err) +} - // move start to after the current new line position - start = end + 1 +const dashSeparator = "----------------------------------------------------------------------" - // Write 2 preceding lines + the target line - if targetLineNum-currentLineNum < 3 { - _, _ = sb.Write(lineBs) - _ = sb.WriteByte('\n') - } +func (p *templateErrorPrettier) makeDetailedError(errMsg, tmplName string, lineNum, posNum any, target string) string { + code, layer, err := p.assets.ReadLayeredFile(tmplName + ".tmpl") + if err != nil { + return fmt.Sprintf("template error: %s, and unable to find template file %q", errMsg, tmplName) + } + line, err := util.ToInt64(lineNum) + if err != nil { + return fmt.Sprintf("template error: %s, unable to parse template %q line number %q", errMsg, tmplName, lineNum) + } + pos, err := util.ToInt64(posNum) + if err != nil { + return fmt.Sprintf("template error: %s, unable to parse template %q pos number %q", errMsg, tmplName, posNum) } + detail := extractErrorLine(code, int(line), int(pos), target) - // FIXME: this algorithm could provide incorrect results and mislead the developers. - // For example: Undefined function "file" in template ..... - // {{Func .file.Addition file.Deletion .file.Addition}} - // ^^^^ ^(the real error is here) - // The pointer is added to the first one, but the second one is the real incorrect one. - // - // If there is a provided target to look for in the line add a pointer to it - // e.g. ^^^^^^^ - if target != "" { - targetPos := bytes.Index(lineBs, []byte(target)) - if targetPos >= 0 { - position = targetPos - } + var msg string + if pos >= 0 { + msg = fmt.Sprintf("template error: %s:%s:%d:%d : %s", layer, tmplName, line, pos, errMsg) + } else { + msg = fmt.Sprintf("template error: %s:%s:%d : %s", layer, tmplName, line, errMsg) } - if position >= 0 { - // take the current line and replace preceding text with whitespace (except for tab) - for i := range lineBs[:position] { - if lineBs[i] != '\t' { - lineBs[i] = ' ' + return msg + "\n" + dashSeparator + "\n" + detail + "\n" + dashSeparator +} + +func extractErrorLine(code []byte, lineNum, posNum int, target string) string { + b := bufio.NewReader(bytes.NewReader(code)) + var line []byte + var err error + for i := 0; i < lineNum; i++ { + if line, err = b.ReadBytes('\n'); err != nil { + if i == lineNum-1 && errors.Is(err, io.EOF) { + err = nil } + break } + } + if err != nil { + return fmt.Sprintf("unable to find target line %d", lineNum) + } - // write the preceding "space" - _, _ = sb.Write(lineBs[:position]) - - // Now write the ^^ pointer - targetLen := len(target) - if targetLen == 0 { - targetLen = 1 + line = bytes.TrimRight(line, "\r\n") + var indicatorLine []byte + targetBytes := []byte(target) + targetLen := len(targetBytes) + for i := 0; i < len(line); { + if posNum == -1 && target != "" && bytes.HasPrefix(line[i:], targetBytes) { + for j := 0; j < targetLen && i < len(line); j++ { + indicatorLine = append(indicatorLine, '^') + i++ + } + } else if i == posNum { + indicatorLine = append(indicatorLine, '^') + i++ + } else { + if line[i] == '\t' { + indicatorLine = append(indicatorLine, '\t') + } else { + indicatorLine = append(indicatorLine, ' ') + } + i++ } - _, _ = sb.WriteString(strings.Repeat("^", targetLen)) - _ = sb.WriteByte('\n') } - - // Finally write the footer - sb.WriteString(dashSeparator) - - return sb.String() + // if the indicatorLine only contains spaces, trim it together + return strings.TrimRight(string(line)+"\n"+string(indicatorLine), " \t\r\n") } diff --git a/modules/templates/htmlrenderer_test.go b/modules/templates/htmlrenderer_test.go new file mode 100644 index 0000000000..2a74b74c23 --- /dev/null +++ b/modules/templates/htmlrenderer_test.go @@ -0,0 +1,106 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package templates + +import ( + "errors" + "html/template" + "os" + "strings" + "testing" + + "code.gitea.io/gitea/modules/assetfs" + + "github.com/stretchr/testify/assert" +) + +func TestExtractErrorLine(t *testing.T) { + cases := []struct { + code string + line int + pos int + target string + expect string + }{ + {"hello world\nfoo bar foo bar\ntest", 2, -1, "bar", ` +foo bar foo bar + ^^^ ^^^ +`}, + + {"hello world\nfoo bar foo bar\ntest", 2, 4, "bar", ` +foo bar foo bar + ^ +`}, + + { + "hello world\nfoo bar foo bar\ntest", 2, 4, "", + ` +foo bar foo bar + ^ +`, + }, + + { + "hello world\nfoo bar foo bar\ntest", 5, 0, "", + `unable to find target line 5`, + }, + } + + for _, c := range cases { + actual := extractErrorLine([]byte(c.code), c.line, c.pos, c.target) + assert.Equal(t, strings.TrimSpace(c.expect), strings.TrimSpace(actual)) + } +} + +func TestHandleError(t *testing.T) { + dir := t.TempDir() + + p := &templateErrorPrettier{assets: assetfs.Layered(assetfs.Local("tmp", dir))} + + test := func(s string, h func(error) string, expect string) { + err := os.WriteFile(dir+"/test.tmpl", []byte(s), 0o644) + assert.NoError(t, err) + tmpl := template.New("test") + _, err = tmpl.Parse(s) + assert.Error(t, err) + msg := h(err) + assert.EqualValues(t, strings.TrimSpace(expect), strings.TrimSpace(msg)) + } + + test("{{", p.handleGenericTemplateError, ` +template error: tmp:test:1 : unclosed action +---------------------------------------------------------------------- +{{ +---------------------------------------------------------------------- +`) + + test("{{Func}}", p.handleFuncNotDefinedError, ` +template error: tmp:test:1 : function "Func" not defined +---------------------------------------------------------------------- +{{Func}} + ^^^^ +---------------------------------------------------------------------- +`) + + test("{{'x'3}}", p.handleUnexpectedOperandError, ` +template error: tmp:test:1 : unexpected "3" in operand +---------------------------------------------------------------------- +{{'x'3}} + ^ +---------------------------------------------------------------------- +`) + + // no idea about how to trigger such strange error, so mock an error to test it + err := os.WriteFile(dir+"/test.tmpl", []byte("god knows XXX"), 0o644) + assert.NoError(t, err) + expectedMsg := ` +template error: tmp:test:1 : expected end; found XXX +---------------------------------------------------------------------- +god knows XXX + ^^^ +---------------------------------------------------------------------- +` + actualMsg := p.handleExpectedEndError(errors.New("template: test:1: expected end; found XXX")) + assert.EqualValues(t, strings.TrimSpace(expectedMsg), strings.TrimSpace(actualMsg)) +} |