Merge remote-tracking branch 'origin/main' into feature/oauth_userinfo

Author: Nils Hillmann

docs/content/doc/help/faq.en-us.md

@@ -324,7 +324,13 @@ is too small. Gitea requires that the `ROWFORMAT` for its tables is `DYNAMIC`.
 
 If you are receiving an error line containing `Error 1071: Specified key was too long; max key length is 1000 bytes...`
 then you are attempting to run Gitea on tables which use the ISAM engine. While this may have worked by chance in previous versions of Gitea, it has never been officially supported and
-you must use InnoDB. You should run `ALTER TABLE table_name ENGINE=InnoDB;` for each table in the database.
+you must use InnoDB. You should run `ALTER TABLE table_name ENGINE=InnoDB;` for each table in the database.  
+If you are using MySQL 5, another possible fix is
+```mysql
+SET GLOBAL innodb_file_format=Barracuda;
+SET GLOBAL innodb_file_per_table=1;
+SET GLOBAL innodb_large_prefix=1;
+```
 
 ## Why Are Emoji Broken On MySQL
 

integrations/user_avatar_test.go

@@ -0,0 +1,87 @@
+// Copyright 2021 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package integrations
+
+import (
+	"bytes"
+	"image/png"
+	"io"
+	"mime/multipart"
+	"net/http"
+	"net/url"
+	"strings"
+	"testing"
+
+	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/avatar"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestUserAvatar(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		user2 := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) // owner of the repo3, is an org
+
+		seed := user2.Email
+		if len(seed) == 0 {
+			seed = user2.Name
+		}
+
+		img, err := avatar.RandomImage([]byte(seed))
+		if err != nil {
+			assert.NoError(t, err)
+			return
+		}
+
+		session := loginUser(t, "user2")
+		csrf := GetCSRF(t, session, "/user/settings")
+
+		imgData := &bytes.Buffer{}
+
+		body := &bytes.Buffer{}
+
+		//Setup multi-part
+		writer := multipart.NewWriter(body)
+		writer.WriteField("source", "local")
+		part, err := writer.CreateFormFile("avatar", "avatar-for-testuseravatar.png")
+		if err != nil {
+			assert.NoError(t, err)
+			return
+		}
+
+		if err := png.Encode(imgData, img); err != nil {
+			assert.NoError(t, err)
+			return
+		}
+
+		if _, err := io.Copy(part, imgData); err != nil {
+			assert.NoError(t, err)
+			return
+		}
+
+		if err := writer.Close(); err != nil {
+			assert.NoError(t, err)
+			return
+		}
+
+		req := NewRequestWithBody(t, "POST", "/user/settings/avatar", body)
+		req.Header.Add("X-Csrf-Token", csrf)
+		req.Header.Add("Content-Type", writer.FormDataContentType())
+
+		session.MakeRequest(t, req, http.StatusFound)
+
+		user2 = models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) // owner of the repo3, is an org
+
+		req = NewRequest(t, "GET", user2.AvatarLink())
+		resp := session.MakeRequest(t, req, http.StatusFound)
+		location := resp.Header().Get("Location")
+		if !strings.HasPrefix(location, "/avatars") {
+			assert.Fail(t, "Avatar location is not local: %s", location)
+		}
+		req = NewRequest(t, "GET", location)
+		session.MakeRequest(t, req, http.StatusOK)
+
+		// Can't test if the response matches because the image is regened on upload but checking that this at least doesn't give a 404 should be enough.
+	})
+}

modules/context/context.go

@@ -692,6 +692,7 @@ func Contexter() func(next http.Handler) http.Handler {
 			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
 			ctx.Data["IsLandingPageExplore"] = setting.LandingPageURL == setting.LandingPageExplore
 			ctx.Data["IsLandingPageOrganizations"] = setting.LandingPageURL == setting.LandingPageOrganizations
@@ -708,6 +709,11 @@ func Contexter() func(next http.Handler) http.Handler {
 
 			ctx.Data["ManifestData"] = setting.ManifestData
 
+			ctx.Data["UnitWikiGlobalDisabled"] = models.UnitTypeWiki.UnitGlobalDisabled()
+			ctx.Data["UnitIssuesGlobalDisabled"] = models.UnitTypeIssues.UnitGlobalDisabled()
+			ctx.Data["UnitPullsGlobalDisabled"] = models.UnitTypePullRequests.UnitGlobalDisabled()
+			ctx.Data["UnitProjectsGlobalDisabled"] = models.UnitTypeProjects.UnitGlobalDisabled()
+
 			ctx.Data["i18n"] = locale
 			ctx.Data["Tr"] = i18n.Tr
 			ctx.Data["Lang"] = locale.Language()

routers/api/v1/api.go

@@ -572,10 +572,6 @@ func Routes() *web.Route {
 	}
 	m.Use(context.APIContexter())
 
-	if setting.EnableAccessLog {
-		m.Use(context.AccessLogger())
-	}
-
 	m.Use(context.ToggleAPI(&context.ToggleOptions{
 		SignInRequired: setting.Service.RequireSignInView,
 	}))

routers/routes/web.go

@@ -89,6 +89,9 @@ func commonMiddlewares() []func(http.Handler) http.Handler {
 			handlers = append(handlers, LoggerHandler(setting.RouterLogLevel))
 		}
 	}
+	if setting.EnableAccessLog {
+		handlers = append(handlers, context.AccessLogger())
+	}
 
 	handlers = append(handlers, func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
@@ -128,9 +131,9 @@ func NormalRoutes() *web.Route {
 
 // WebRoutes returns all web routes
 func WebRoutes() *web.Route {
-	r := web.NewRoute()
+	routes := web.NewRoute()
 
-	r.Use(session.Sessioner(session.Options{
+	routes.Use(session.Sessioner(session.Options{
 		Provider:       setting.SessionConfig.Provider,
 		ProviderConfig: setting.SessionConfig.ProviderConfig,
 		CookieName:     setting.SessionConfig.CookieName,
@@ -141,93 +144,99 @@ func WebRoutes() *web.Route {
 		Domain:         setting.SessionConfig.Domain,
 	}))
 
-	r.Use(Recovery())
+	routes.Use(Recovery())
 
-	r.Use(public.Custom(
+	// TODO: we should consider if there is a way to mount these using r.Route as at present
+	// these two handlers mean that every request has to hit these "filesystems" twice
+	// before finally getting to the router. It allows them to override any matching router below.
+	routes.Use(public.Custom(
 		&public.Options{
 			SkipLogging: setting.DisableRouterLog,
 		},
 	))
-	r.Use(public.Static(
+	routes.Use(public.Static(
 		&public.Options{
 			Directory:   path.Join(setting.StaticRootPath, "public"),
 			SkipLogging: setting.DisableRouterLog,
 			Prefix:      "/assets",
 		},
 	))
 
-	r.Use(storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars))
-	r.Use(storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars))
+	// We use r.Route here over r.Use because this prevents requests that are not for avatars having to go through this additional handler
+	routes.Route("/avatars/*", "GET, HEAD", storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars))
+	routes.Route("/repo-avatars/*", "GET, HEAD", storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars))
+
+	// for health check - doeesn't need to be passed through gzip handler
+	routes.Head("/", func(w http.ResponseWriter, req *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	})
+
+	// this png is very likely to always be below the limit for gzip so it doesn't need to pass through gzip
+	routes.Get("/apple-touch-icon.png", func(w http.ResponseWriter, req *http.Request) {
+		http.Redirect(w, req, path.Join(setting.StaticURLPrefix, "img/apple-touch-icon.png"), 301)
+	})
 
 	gob.Register(&u2f.Challenge{})
 
+	common := []interface{}{}
+
 	if setting.EnableGzip {
 		h, err := gziphandler.GzipHandlerWithOpts(gziphandler.MinSize(GzipMinSize))
 		if err != nil {
 			log.Fatal("GzipHandlerWithOpts failed: %v", err)
 		}
-		r.Use(h)
+		common = append(common, h)
 	}
 
 	mailer.InitMailRender(templates.Mailer())
 
 	if setting.Service.EnableCaptcha {
-		r.Use(captcha.Captchaer(context.GetImageCaptcha()))
-	}
-	// Removed: toolbox.Toolboxer middleware will provide debug informations which seems unnecessary
-	r.Use(context.Contexter())
-	// GetHead allows a HEAD request redirect to GET if HEAD method is not defined for that route
-	r.Use(middleware.GetHead)
-
-	if setting.EnableAccessLog {
-		r.Use(context.AccessLogger())
+		// The captcha http.Handler should only fire on /captcha/* so we can just mount this on that url
+		routes.Route("/captcha/*", "GET,HEAD", append(common, captcha.Captchaer(context.GetImageCaptcha()))...)
 	}
 
-	r.Use(user.GetNotificationCount)
-	r.Use(repo.GetActiveStopwatch)
-	r.Use(func(ctx *context.Context) {
-		ctx.Data["UnitWikiGlobalDisabled"] = models.UnitTypeWiki.UnitGlobalDisabled()
-		ctx.Data["UnitIssuesGlobalDisabled"] = models.UnitTypeIssues.UnitGlobalDisabled()
-		ctx.Data["UnitPullsGlobalDisabled"] = models.UnitTypePullRequests.UnitGlobalDisabled()
-		ctx.Data["UnitProjectsGlobalDisabled"] = models.UnitTypeProjects.UnitGlobalDisabled()
-	})
-
-	// for health check
-	r.Head("/", func(w http.ResponseWriter, req *http.Request) {
-		w.WriteHeader(http.StatusOK)
-	})
-
 	if setting.HasRobotsTxt {
-		r.Get("/robots.txt", func(w http.ResponseWriter, req *http.Request) {
+		routes.Get("/robots.txt", append(common, func(w http.ResponseWriter, req *http.Request) {
 			filePath := path.Join(setting.CustomPath, "robots.txt")
 			fi, err := os.Stat(filePath)
 			if err == nil && httpcache.HandleTimeCache(req, w, fi) {
 				return
 			}
 			http.ServeFile(w, req, filePath)
-		})
+		})...)
 	}
 
-	r.Get("/apple-touch-icon.png", func(w http.ResponseWriter, req *http.Request) {
-		http.Redirect(w, req, path.Join(setting.StaticURLPrefix, "img/apple-touch-icon.png"), 301)
-	})
-
-	// prometheus metrics endpoint
+	// prometheus metrics endpoint - do not need to go through contexter
 	if setting.Metrics.Enabled {
 		c := metrics.NewCollector()
 		prometheus.MustRegister(c)
 
-		r.Get("/metrics", routers.Metrics)
+		routes.Get("/metrics", append(common, routers.Metrics)...)
 	}
 
+	// Removed: toolbox.Toolboxer middleware will provide debug informations which seems unnecessary
+	common = append(common, context.Contexter())
+
+	// GetHead allows a HEAD request redirect to GET if HEAD method is not defined for that route
+	common = append(common, middleware.GetHead)
+
 	if setting.API.EnableSwagger {
 		// Note: The route moved from apiroutes because it's in fact want to render a web page
-		r.Get("/api/swagger", misc.Swagger) // Render V1 by default
+		routes.Get("/api/swagger", append(common, misc.Swagger)...) // Render V1 by default
 	}
 
-	RegisterRoutes(r)
+	// TODO: These really seem like things that could be folded into Contexter or as helper functions
+	common = append(common, user.GetNotificationCount)
+	common = append(common, repo.GetActiveStopwatch)
 
-	return r
+	others := web.NewRoute()
+	for _, middle := range common {
+		others.Use(middle)
+	}
+
+	RegisterRoutes(others)
+	routes.Mount("", others)
+	return routes
 }
 
 func goGet(ctx *context.Context) {