Refactor HTTP request context (#17979)

Author: wxiaoguang

modules/context/api.go

@@ -181,24 +181,24 @@ func (ctx *APIContext) SetLinkHeader(total, pageSize int) {
 	links := genAPILinks(ctx.Req.URL, total, pageSize, ctx.FormInt("page"))
 
 	if len(links) > 0 {
-		ctx.Header().Set("Link", strings.Join(links, ","))
+		ctx.RespHeader().Set("Link", strings.Join(links, ","))
 		ctx.AppendAccessControlExposeHeaders("Link")
 	}
 }
 
 // SetTotalCountHeader set "X-Total-Count" header
 func (ctx *APIContext) SetTotalCountHeader(total int64) {
-	ctx.Header().Set("X-Total-Count", fmt.Sprint(total))
+	ctx.RespHeader().Set("X-Total-Count", fmt.Sprint(total))
 	ctx.AppendAccessControlExposeHeaders("X-Total-Count")
 }
 
 // AppendAccessControlExposeHeaders append headers by name to "Access-Control-Expose-Headers" header
 func (ctx *APIContext) AppendAccessControlExposeHeaders(names ...string) {
-	val := ctx.Header().Get("Access-Control-Expose-Headers")
+	val := ctx.RespHeader().Get("Access-Control-Expose-Headers")
 	if len(val) != 0 {
-		ctx.Header().Set("Access-Control-Expose-Headers", fmt.Sprintf("%s, %s", val, strings.Join(names, ", ")))
+		ctx.RespHeader().Set("Access-Control-Expose-Headers", fmt.Sprintf("%s, %s", val, strings.Join(names, ", ")))
 	} else {
-		ctx.Header().Set("Access-Control-Expose-Headers", strings.Join(names, ", "))
+		ctx.RespHeader().Set("Access-Control-Expose-Headers", strings.Join(names, ", "))
 	}
 }
 

modules/context/context.go

@@ -9,7 +9,6 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
-	"fmt"
 	"html"
 	"html/template"
 	"io"
@@ -156,6 +155,7 @@ func (ctx *Context) GetErrMsg() string {
 }
 
 // HasError returns true if error occurs in form validation.
+// Attention: this function changes ctx.Data and ctx.Flash
 func (ctx *Context) HasError() bool {
 	hasErr, ok := ctx.Data["HasError"]
 	if !ok {
@@ -191,29 +191,25 @@ func (ctx *Context) RedirectToFirst(location ...string) {
 	ctx.Redirect(setting.AppSubURL + "/")
 }
 
-// HTML calls Context.HTML and converts template name to string.
+// HTML calls Context.HTML and renders the template to HTTP response
 func (ctx *Context) HTML(status int, name base.TplName) {
 	log.Debug("Template: %s", name)
-	var startTime = time.Now()
+	tmplStartTime := time.Now()
 	ctx.Data["TmplLoadTimes"] = func() string {
-		return fmt.Sprint(time.Since(startTime).Nanoseconds()/1e6) + "ms"
+		return strconv.FormatInt(time.Since(tmplStartTime).Nanoseconds()/1e6, 10) + "ms"
 	}
 	if err := ctx.Render.HTML(ctx.Resp, status, string(name), ctx.Data); err != nil {
 		if status == http.StatusInternalServerError && name == base.TplName("status/500") {
-			ctx.PlainText(http.StatusInternalServerError, []byte("Unable to find status/500 template"))
+			ctx.PlainText(http.StatusInternalServerError, "Unable to find status/500 template")
 			return
 		}
 		ctx.ServerError("Render failed", err)
 	}
 }
 
-// HTMLString render content to a string but not http.ResponseWriter
-func (ctx *Context) HTMLString(name string, data interface{}) (string, error) {
+// RenderToString renders the template content to a string
+func (ctx *Context) RenderToString(name base.TplName, data map[string]interface{}) (string, error) {
 	var buf strings.Builder
-	var startTime = time.Now()
-	ctx.Data["TmplLoadTimes"] = func() string {
-		return fmt.Sprint(time.Since(startTime).Nanoseconds()/1e6) + "ms"
-	}
 	err := ctx.Render.HTML(&buf, 200, string(name), data)
 	return buf.String(), err
 }
@@ -229,51 +225,47 @@ func (ctx *Context) RenderWithErr(msg string, tpl base.TplName, form interface{}
 }
 
 // NotFound displays a 404 (Not Found) page and prints the given error, if any.
-func (ctx *Context) NotFound(title string, err error) {
-	ctx.notFoundInternal(title, err)
+func (ctx *Context) NotFound(logMsg string, logErr error) {
+	ctx.notFoundInternal(logMsg, logErr)
 }
 
-func (ctx *Context) notFoundInternal(title string, err error) {
-	if err != nil {
-		log.ErrorWithSkip(2, "%s: %v", title, err)
+func (ctx *Context) notFoundInternal(logMsg string, logErr error) {
+	if logErr != nil {
+		log.ErrorWithSkip(2, "%s: %v", logMsg, logErr)
 		if !setting.IsProd {
-			ctx.Data["ErrorMsg"] = err
+			ctx.Data["ErrorMsg"] = logErr
 		}
 	}
 
-	// response simple meesage if Accept isn't text/html
-	reqTypes, has := ctx.Req.Header["Accept"]
-	if has && len(reqTypes) > 0 {
-		notHTML := true
-		for _, part := range reqTypes {
-			if strings.Contains(part, "text/html") {
-				notHTML = false
-				break
-			}
+	// response simple message if Accept isn't text/html
+	showHTML := false
+	for _, part := range ctx.Req.Header["Accept"] {
+		if strings.Contains(part, "text/html") {
+			showHTML = true
+			break
 		}
+	}
 
-		if notHTML {
-			ctx.PlainText(404, []byte("Not found.\n"))
-			return
-		}
+	if !showHTML {
+		ctx.PlainText(http.StatusNotFound, "Not found.\n")
+		return
 	}
 
 	ctx.Data["IsRepo"] = ctx.Repo.Repository != nil
 	ctx.Data["Title"] = "Page Not Found"
 	ctx.HTML(http.StatusNotFound, base.TplName("status/404"))
 }
 
-// ServerError displays a 500 (Internal Server Error) page and prints the given
-// error, if any.
-func (ctx *Context) ServerError(title string, err error) {
-	ctx.serverErrorInternal(title, err)
+// ServerError displays a 500 (Internal Server Error) page and prints the given error, if any.
+func (ctx *Context) ServerError(logMsg string, logErr error) {
+	ctx.serverErrorInternal(logMsg, logErr)
 }
 
-func (ctx *Context) serverErrorInternal(title string, err error) {
-	if err != nil {
-		log.ErrorWithSkip(2, "%s: %v", title, err)
+func (ctx *Context) serverErrorInternal(logMsg string, logErr error) {
+	if logErr != nil {
+		log.ErrorWithSkip(2, "%s: %v", logMsg, logErr)
 		if !setting.IsProd {
-			ctx.Data["ErrorMsg"] = err
+			ctx.Data["ErrorMsg"] = logErr
 		}
 	}
 
@@ -282,37 +274,45 @@ func (ctx *Context) serverErrorInternal(title string, err error) {
 }
 
 // NotFoundOrServerError use error check function to determine if the error
-// is about not found. It responses with 404 status code for not found 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(title string, errck func(error) bool, err error) {
-	if errck(err) {
-		ctx.notFoundInternal(title, err)
+func (ctx *Context) NotFoundOrServerError(logMsg string, errCheck func(error) bool, err error) {
+	if errCheck(err) {
+		ctx.notFoundInternal(logMsg, err)
 		return
 	}
+	ctx.serverErrorInternal(logMsg, err)
+}
 
-	ctx.serverErrorInternal(title, err)
+// PlainTextBytes renders bytes as plain text
+func (ctx *Context) PlainTextBytes(status int, bs []byte) {
+	if (status/100 == 4) || (status/100 == 5) {
+		log.Error("PlainTextBytes: %s", string(bs))
+	}
+	ctx.Resp.WriteHeader(status)
+	ctx.Resp.Header().Set("Content-Type", "text/plain;charset=utf-8")
+	if _, err := ctx.Resp.Write(bs); err != nil {
+		log.Error("Write bytes failed: %v", err)
+	}
 }
 
-// Header returns a header
-func (ctx *Context) Header() http.Header {
-	return ctx.Resp.Header()
+// PlainText renders content as plain text
+func (ctx *Context) PlainText(status int, text string) {
+	ctx.PlainTextBytes(status, []byte(text))
 }
 
-// HandleText handles HTTP status code
-func (ctx *Context) HandleText(status int, title string) {
-	if (status/100 == 4) || (status/100 == 5) {
-		log.Error("%s", title)
-	}
-	ctx.PlainText(status, []byte(title))
+// RespHeader returns the response header
+func (ctx *Context) RespHeader() http.Header {
+	return ctx.Resp.Header()
 }
 
 // ServeContent serves content to http request
 func (ctx *Context) ServeContent(name string, r io.ReadSeeker, params ...interface{}) {
-	modtime := time.Now()
+	modTime := time.Now()
 	for _, p := range params {
 		switch v := p.(type) {
 		case time.Time:
-			modtime = v
+			modTime = v
 		}
 	}
 	ctx.Resp.Header().Set("Content-Description", "File Transfer")
@@ -323,16 +323,7 @@ func (ctx *Context) ServeContent(name string, r io.ReadSeeker, params ...interfa
 	ctx.Resp.Header().Set("Cache-Control", "must-revalidate")
 	ctx.Resp.Header().Set("Pragma", "public")
 	ctx.Resp.Header().Set("Access-Control-Expose-Headers", "Content-Disposition")
-	http.ServeContent(ctx.Resp, ctx.Req, name, modtime, r)
-}
-
-// PlainText render content as plain text
-func (ctx *Context) PlainText(status int, bs []byte) {
-	ctx.Resp.WriteHeader(status)
-	ctx.Resp.Header().Set("Content-Type", "text/plain;charset=utf-8")
-	if _, err := ctx.Resp.Write(bs); err != nil {
-		ctx.ServerError("Write bytes failed", err)
-	}
+	http.ServeContent(ctx.Resp, ctx.Req, name, modTime, r)
 }
 
 // ServeFile serves given file to response.
@@ -386,7 +377,7 @@ func (ctx *Context) JSON(status int, content interface{}) {
 	}
 }
 
-// Redirect redirect the request
+// Redirect redirects the request
 func (ctx *Context) Redirect(location string, status ...int) {
 	code := http.StatusFound
 	if len(status) == 1 {
@@ -506,7 +497,7 @@ func (ctx *Context) SetParams(k, v string) {
 	chiCtx.URLParams.Add(strings.TrimPrefix(k, ":"), url.PathEscape(v))
 }
 
-// Write writes data to webbrowser
+// Write writes data to web browser
 func (ctx *Context) Write(bs []byte) (int, error) {
 	return ctx.Resp.Write(bs)
 }
@@ -544,10 +535,9 @@ func (ctx *Context) Value(key interface{}) interface{} {
 // Handler represents a custom handler
 type Handler func(*Context)
 
-// enumerate all content
-var (
-	contextKey interface{} = "default_context"
-)
+type contextKeyType struct{}
+
+var contextKey interface{} = contextKeyType{}
 
 // WithContext set up install context in request
 func WithContext(req *http.Request, ctx *Context) *http.Request {
@@ -570,31 +560,6 @@ func GetContextUser(req *http.Request) *user_model.User {
 	return nil
 }
 
-// SignedUserName returns signed user's name via context
-func SignedUserName(req *http.Request) string {
-	if middleware.IsInternalPath(req) {
-		return ""
-	}
-	if middleware.IsAPIPath(req) {
-		ctx, ok := req.Context().Value(apiContextKey).(*APIContext)
-		if ok {
-			v := ctx.Data["SignedUserName"]
-			if res, ok := v.(string); ok {
-				return res
-			}
-		}
-	} else {
-		ctx, ok := req.Context().Value(contextKey).(*Context)
-		if ok {
-			v := ctx.Data["SignedUserName"]
-			if res, ok := v.(string); ok {
-				return res
-			}
-		}
-	}
-	return ""
-}
-
 func getCsrfOpts() CsrfOptions {
 	return CsrfOptions{
 		Secret:         setting.SecretKey,
@@ -727,8 +692,6 @@ func Contexter() func(next http.Handler) http.Handler {
 
 			ctx.Data["CsrfToken"] = html.EscapeString(ctx.csrf.GetToken())
 			ctx.Data["CsrfTokenHtml"] = template.HTML(`<input type="hidden" name="_csrf" value="` + ctx.Data["CsrfToken"].(string) + `">`)
-			log.Debug("Session ID: %s", ctx.Session.ID())
-			log.Debug("CSRF Token: %v", ctx.Data["CsrfToken"])
 
 			// FIXME: do we really always need these setting? There should be someway to have to avoid having to always set these
 			ctx.Data["IsLandingPageHome"] = setting.LandingPageURL == setting.LandingPageHome

modules/context/repo.go

@@ -9,6 +9,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"net/http"
 	"net/url"
 	"path"
 	"strings"
@@ -305,14 +306,14 @@ func EarlyResponseForGoGetMeta(ctx *Context) {
 	username := ctx.Params(":username")
 	reponame := strings.TrimSuffix(ctx.Params(":reponame"), ".git")
 	if username == "" || reponame == "" {
-		ctx.PlainText(400, []byte("invalid repository path"))
+		ctx.PlainText(http.StatusBadRequest, "invalid repository path")
 		return
 	}
-	ctx.PlainText(200, []byte(com.Expand(`<meta name="go-import" content="{GoGetImport} git {CloneLink}">`,
+	ctx.PlainText(http.StatusOK, com.Expand(`<meta name="go-import" content="{GoGetImport} git {CloneLink}">`,
 		map[string]string{
 			"GoGetImport": ComposeGoGetImport(username, reponame),
 			"CloneLink":   repo_model.ComposeHTTPSCloneURL(username, reponame),
-		})))
+		}))
 }
 
 // RedirectToRepo redirect to a differently-named repository
@@ -897,7 +898,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
 				}
 				// If short commit ID add canonical link header
 				if len(refName) < 40 {
-					ctx.Header().Set("Link", fmt.Sprintf("<%s>; rel=\"canonical\"",
+					ctx.RespHeader().Set("Link", fmt.Sprintf("<%s>; rel=\"canonical\"",
 						util.URLJoin(setting.AppURL, strings.Replace(ctx.Req.URL.RequestURI(), util.PathEscapeSegments(refName), url.PathEscape(ctx.Repo.Commit.ID.String()), 1))))
 				}
 			} else {

modules/templates/base.go

@@ -43,9 +43,6 @@ func BaseVars() Vars {
 		"EnableSwagger":      setting.API.EnableSwagger,
 		"EnableOpenIDSignIn": setting.Service.EnableOpenIDSignIn,
 		"PageStartTime":      startTime,
-		"TmplLoadTimes": func() string {
-			return time.Since(startTime).String()
-		},
 	}
 }
 

routers/api/v1/repo/commits.go

@@ -205,11 +205,11 @@ func GetAllCommits(ctx *context.APIContext) {
 	ctx.SetTotalCountHeader(commitsCountTotal)
 
 	// kept for backwards compatibility
-	ctx.Header().Set("X-Page", strconv.Itoa(listOptions.Page))
-	ctx.Header().Set("X-PerPage", strconv.Itoa(listOptions.PageSize))
-	ctx.Header().Set("X-Total", strconv.FormatInt(commitsCountTotal, 10))
-	ctx.Header().Set("X-PageCount", strconv.Itoa(pageCount))
-	ctx.Header().Set("X-HasMore", strconv.FormatBool(listOptions.Page < pageCount))
+	ctx.RespHeader().Set("X-Page", strconv.Itoa(listOptions.Page))
+	ctx.RespHeader().Set("X-PerPage", strconv.Itoa(listOptions.PageSize))
+	ctx.RespHeader().Set("X-Total", strconv.FormatInt(commitsCountTotal, 10))
+	ctx.RespHeader().Set("X-PageCount", strconv.Itoa(pageCount))
+	ctx.RespHeader().Set("X-HasMore", strconv.FormatBool(listOptions.Page < pageCount))
 	ctx.AppendAccessControlExposeHeaders("X-Page", "X-PerPage", "X-Total", "X-PageCount", "X-HasMore")
 
 	ctx.JSON(http.StatusOK, &apiCommits)

routers/api/v1/repo/pull.go

@@ -1233,10 +1233,10 @@ func GetPullRequestCommits(ctx *context.APIContext) {
 	ctx.SetLinkHeader(totalNumberOfCommits, listOptions.PageSize)
 	ctx.SetTotalCountHeader(int64(totalNumberOfCommits))
 
-	ctx.Header().Set("X-Page", strconv.Itoa(listOptions.Page))
-	ctx.Header().Set("X-PerPage", strconv.Itoa(listOptions.PageSize))
-	ctx.Header().Set("X-PageCount", strconv.Itoa(totalNumberOfPages))
-	ctx.Header().Set("X-HasMore", strconv.FormatBool(listOptions.Page < totalNumberOfPages))
+	ctx.RespHeader().Set("X-Page", strconv.Itoa(listOptions.Page))
+	ctx.RespHeader().Set("X-PerPage", strconv.Itoa(listOptions.PageSize))
+	ctx.RespHeader().Set("X-PageCount", strconv.Itoa(totalNumberOfPages))
+	ctx.RespHeader().Set("X-HasMore", strconv.FormatBool(listOptions.Page < totalNumberOfPages))
 	ctx.AppendAccessControlExposeHeaders("X-Page", "X-PerPage", "X-PageCount", "X-HasMore")
 
 	ctx.JSON(http.StatusOK, &apiCommits)

routers/api/v1/user/gpg_key.go

@@ -157,7 +157,7 @@ func GetVerificationToken(ctx *context.APIContext) {
 	//     "$ref": "#/responses/notFound"
 
 	token := asymkey_model.VerificationToken(ctx.User, 1)
-	ctx.PlainText(http.StatusOK, []byte(token))
+	ctx.PlainText(http.StatusOK, token)
 }
 
 // VerifyUserGPGKey creates new GPG key to given user by ID.