diff options
author | wxiaoguang <wxiaoguang@gmail.com> | 2024-06-18 07:28:47 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-06-17 23:28:47 +0000 |
commit | d32648b204395fe3590ca2de5f38f0f97da510aa (patch) | |
tree | 7ef6a94955dbd55c7e00ba7c1fb87e198bca87b8 /modules/web | |
parent | 5a7376c0605415e63cb5b3b8f89ead01e567229b (diff) | |
download | gitea-d32648b204395fe3590ca2de5f38f0f97da510aa.tar.gz gitea-d32648b204395fe3590ca2de5f38f0f97da510aa.zip |
Refactor route path normalization (#31381)
Refactor route path normalization and decouple it from the chi router.
Fix the TODO, fix the legacy strange path behavior.
Diffstat (limited to 'modules/web')
-rw-r--r-- | modules/web/route.go | 59 | ||||
-rw-r--r-- | modules/web/route_test.go | 44 |
2 files changed, 102 insertions, 1 deletions
diff --git a/modules/web/route.go b/modules/web/route.go index 805fcb4411..34290bd8e5 100644 --- a/modules/web/route.go +++ b/modules/web/route.go @@ -5,8 +5,10 @@ package web import ( "net/http" + "net/url" "strings" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web/middleware" "gitea.com/go-chi/binding" @@ -160,7 +162,7 @@ func (r *Route) Patch(pattern string, h ...any) { // ServeHTTP implements http.Handler func (r *Route) ServeHTTP(w http.ResponseWriter, req *http.Request) { - r.R.ServeHTTP(w, req) + r.normalizeRequestPath(w, req, r.R) } // NotFound defines a handler to respond whenever a route could not be found. @@ -168,6 +170,61 @@ func (r *Route) NotFound(h http.HandlerFunc) { r.R.NotFound(h) } +func (r *Route) normalizeRequestPath(resp http.ResponseWriter, req *http.Request, next http.Handler) { + normalized := false + normalizedPath := req.URL.EscapedPath() + if normalizedPath == "" { + normalizedPath, normalized = "/", true + } else if normalizedPath != "/" { + normalized = strings.HasSuffix(normalizedPath, "/") + normalizedPath = strings.TrimRight(normalizedPath, "/") + } + removeRepeatedSlashes := strings.Contains(normalizedPath, "//") + normalized = normalized || removeRepeatedSlashes + + // the following code block is a slow-path for replacing all repeated slashes "//" to one single "/" + // if the path doesn't have repeated slashes, then no need to execute it + if removeRepeatedSlashes { + buf := &strings.Builder{} + for i := 0; i < len(normalizedPath); i++ { + if i == 0 || normalizedPath[i-1] != '/' || normalizedPath[i] != '/' { + buf.WriteByte(normalizedPath[i]) + } + } + normalizedPath = buf.String() + } + + // If the config tells Gitea to use a sub-url path directly without reverse proxy, + // then we need to remove the sub-url path from the request URL path. + // But "/v2" is special for OCI container registry, it should always be in the root of the site. + if setting.UseSubURLPath { + remainingPath, ok := strings.CutPrefix(normalizedPath, setting.AppSubURL+"/") + if ok { + normalizedPath = "/" + remainingPath + } else if normalizedPath == setting.AppSubURL { + normalizedPath = "/" + } else if !strings.HasPrefix(normalizedPath+"/", "/v2/") { + // do not respond to other requests, to simulate a real sub-path environment + http.Error(resp, "404 page not found, sub-path is: "+setting.AppSubURL, http.StatusNotFound) + return + } + normalized = true + } + + // if the path is normalized, then fill it back to the request + if normalized { + decodedPath, err := url.PathUnescape(normalizedPath) + if err != nil { + http.Error(resp, "400 Bad Request: unable to unescape path "+normalizedPath, http.StatusBadRequest) + return + } + req.URL.RawPath = normalizedPath + req.URL.Path = decodedPath + } + + next.ServeHTTP(resp, req) +} + // Combo delegates requests to Combo func (r *Route) Combo(pattern string, h ...any) *Combo { return &Combo{r, pattern, h} diff --git a/modules/web/route_test.go b/modules/web/route_test.go index cc0e26a12e..c5595a02a3 100644 --- a/modules/web/route_test.go +++ b/modules/web/route_test.go @@ -10,6 +10,9 @@ import ( "strconv" "testing" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" + chi "github.com/go-chi/chi/v5" "github.com/stretchr/testify/assert" ) @@ -176,3 +179,44 @@ func TestRoute3(t *testing.T) { assert.EqualValues(t, http.StatusOK, recorder.Code) assert.EqualValues(t, 4, hit) } + +func TestRouteNormalizePath(t *testing.T) { + type paths struct { + EscapedPath, RawPath, Path string + } + testPath := func(reqPath string, expectedPaths paths) { + recorder := httptest.NewRecorder() + recorder.Body = bytes.NewBuffer(nil) + + actualPaths := paths{EscapedPath: "(none)", RawPath: "(none)", Path: "(none)"} + r := NewRoute() + r.Get("/*", func(resp http.ResponseWriter, req *http.Request) { + actualPaths.EscapedPath = req.URL.EscapedPath() + actualPaths.RawPath = req.URL.RawPath + actualPaths.Path = req.URL.Path + }) + + req, err := http.NewRequest("GET", reqPath, nil) + assert.NoError(t, err) + r.ServeHTTP(recorder, req) + assert.Equal(t, expectedPaths, actualPaths, "req path = %q", reqPath) + } + + // RawPath could be empty if the EscapedPath is the same as escape(Path) and it is already normalized + testPath("/", paths{EscapedPath: "/", RawPath: "", Path: "/"}) + testPath("//", paths{EscapedPath: "/", RawPath: "/", Path: "/"}) + testPath("/%2f", paths{EscapedPath: "/%2f", RawPath: "/%2f", Path: "//"}) + testPath("///a//b/", paths{EscapedPath: "/a/b", RawPath: "/a/b", Path: "/a/b"}) + + defer test.MockVariableValue(&setting.UseSubURLPath, true)() + defer test.MockVariableValue(&setting.AppSubURL, "/sub-path")() + testPath("/", paths{EscapedPath: "(none)", RawPath: "(none)", Path: "(none)"}) // 404 + testPath("/sub-path", paths{EscapedPath: "/", RawPath: "/", Path: "/"}) + testPath("/sub-path/", paths{EscapedPath: "/", RawPath: "/", Path: "/"}) + testPath("/sub-path//a/b///", paths{EscapedPath: "/a/b", RawPath: "/a/b", Path: "/a/b"}) + testPath("/sub-path/%2f/", paths{EscapedPath: "/%2f", RawPath: "/%2f", Path: "//"}) + // "/v2" is special for OCI container registry, it should always be in the root of the site + testPath("/v2", paths{EscapedPath: "/v2", RawPath: "/v2", Path: "/v2"}) + testPath("/v2/", paths{EscapedPath: "/v2", RawPath: "/v2", Path: "/v2"}) + testPath("/v2/%2f", paths{EscapedPath: "/v2/%2f", RawPath: "/v2/%2f", Path: "/v2//"}) +} |