diff --git a/modules/web/router_path.go b/modules/web/router_path.go index 5f356fff005..46cc0d7c0a9 100644 --- a/modules/web/router_path.go +++ b/modules/web/router_path.go @@ -5,6 +5,7 @@ package web import ( "net/http" + "net/url" "regexp" "slices" "strings" @@ -19,13 +20,17 @@ type RouterPathGroup struct { r *Router pathParam string matchers []*routerPathMatcher + unescape bool } func (g *RouterPathGroup) ServeHTTP(resp http.ResponseWriter, req *http.Request) { chiCtx := chi.RouteContext(req.Context()) path := chiCtx.URLParam(g.pathParam) + if g.unescape { + path, _ = url.PathUnescape(path) + } for _, m := range g.matchers { - if m.matchPath(chiCtx, path) { + if m.matchPath(chiCtx, path, g.unescape) { chiCtx.RoutePatterns = append(chiCtx.RoutePatterns, m.pattern) executeMiddlewaresHandler(resp, req, m.middlewares, m.handlerFunc) return @@ -53,6 +58,10 @@ func (g *RouterPathGroup) MatchPattern(methods string, pattern *RouterPathGroupP g.matchers = append(g.matchers, newRouterPathMatcher(methods, pattern, h...)) } +func (g *RouterPathGroup) UseUnescapedPath() { + g.unescape = true +} + type routerPathParam struct { name string pathSepEnd bool @@ -68,7 +77,7 @@ type routerPathMatcher struct { handlerFunc http.HandlerFunc } -func (p *routerPathMatcher) matchPath(chiCtx *chi.Context, path string) bool { +func (p *routerPathMatcher) matchPath(chiCtx *chi.Context, path string, unescaped bool) bool { if !p.methods.Contains(chiCtx.RouteMethod) { return false } @@ -102,6 +111,9 @@ func (p *routerPathMatcher) matchPath(chiCtx *chi.Context, path string) bool { if p.params[i].pathSepEnd { val = strings.TrimSuffix(val, "/") } + if unescaped { + val = url.PathEscape(val) + } chiCtx.URLParams.Add(p.params[i].name, val) } return true diff --git a/modules/web/router_test.go b/modules/web/router_test.go index fc1995773dd..2dceedbb276 100644 --- a/modules/web/router_test.go +++ b/modules/web/router_test.go @@ -7,6 +7,7 @@ import ( "bytes" "net/http" "net/http/httptest" + "net/url" "strings" "testing" @@ -97,12 +98,16 @@ func (r *testRecorder) test(t *testing.T, rt *Router, methodPath string, expecte } func TestPathProcessor(t *testing.T) { + unescape := false testProcess := func(pattern, uri string, expectedPathParams map[string]string) { chiCtx := chi.NewRouteContext() chiCtx.RouteMethod = "GET" p := newRouterPathMatcher("GET", patternRegexp(pattern), http.NotFound) shouldProcess := expectedPathParams != nil - assert.Equal(t, shouldProcess, p.matchPath(chiCtx, uri), "use pattern %s to process uri %s", pattern, uri) + if unescape { + uri, _ = url.PathUnescape(uri) + } + assert.Equal(t, shouldProcess, p.matchPath(chiCtx, uri, unescape), "use pattern %s to process uri %s", pattern, uri) assert.Equal(t, expectedPathParams, chiURLParamsToMap(chiCtx), "use pattern %s to process uri %s", pattern, uri) } @@ -119,6 +124,9 @@ func TestPathProcessor(t *testing.T) { testProcess("//part/", "/part/c", map[string]string{"p1": "", "p2": "c"}) testProcess("//part/", "/a/other-part/c", nil) testProcess("/-part/", "/a-other-part/c", map[string]string{"p1": "a-other", "p2": "c"}) + + unescape = true + testProcess("/", "/%40%2fx", map[string]string{"p1": "@%2Fx"}) } func TestRouter(t *testing.T) { diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index 3fe70135b29..67e64a66471 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -405,37 +405,24 @@ func CommonRoutes() *web.Router { }, reqPackageAccess(perm.AccessModeRead)) }) r.Group("/npm", func() { - // HINT: NPM-ROUTE-PATH-PATTERN: search this keyword to see more details - scopeRegexp := `^@` + npm_module.RegexpNamePart + `$` - idRegexp := `^(@` + npm_module.RegexpNamePart + `%2[fF])?` + npm_module.RegexpNamePart + `$` - addPackageHandlers := func() { - r.Get("", npm.PackageMetadata) - r.Put("", reqPackageAccess(perm.AccessModeWrite), npm.UploadPackage) - r.Get("/{version}", npm.PackageVersionMetadata) - r.Group("/-/{version}/{filename}", func() { - r.Get("", npm.DownloadPackageFile) - r.Delete("/-rev/{revision}", reqPackageAccess(perm.AccessModeWrite), npm.DeletePackageVersion) - }) - r.Get("/-/{filename}", npm.DownloadPackageFileByName) - r.Group("/-rev/{revision}", func() { - r.Delete("", npm.DeletePackage) - r.Put("", npm.DeletePreview) - }, reqPackageAccess(perm.AccessModeWrite)) - } - r.Group("/{scope:"+scopeRegexp+"}/{id:"+idRegexp+"}", addPackageHandlers) - r.Group("/{id:"+idRegexp+"}", addPackageHandlers) + r.Get("/-/v1/search", npm.PackageSearch) + r.PathGroup("/*", func(g *web.RouterPathGroup) { + // HINT: NPM-ROUTE-PATH-PATTERN: search this keyword to see more details + packageId := `/" + g.UseUnescapedPath() + g.MatchPath("DELETE", packageId+"/-///-rev/", reqPackageAccess(perm.AccessModeWrite), npm.DeletePackageVersion) + g.MatchPath("GET", packageId+"/-//", npm.DownloadPackageFile) + g.MatchPath("GET", packageId+"/-/", npm.DownloadPackageFileByName) + g.MatchPath("DELETE", packageId+"/-rev/", reqPackageAccess(perm.AccessModeWrite), npm.DeletePackage) + g.MatchPath("PUT", packageId+"/-rev/", reqPackageAccess(perm.AccessModeWrite), npm.DeletePreview) + g.MatchPath("GET", packageId+"/", npm.PackageVersionMetadata) + g.MatchPath("GET", packageId, npm.PackageMetadata) + g.MatchPath("PUT", packageId, reqPackageAccess(perm.AccessModeWrite), npm.UploadPackage) - addPackageDistTagsHandlers := func() { - r.Get("", npm.ListPackageTags) - r.Group("/{tag}", func() { - r.Put("", npm.AddPackageTag) - r.Delete("", npm.DeletePackageTag) - }, reqPackageAccess(perm.AccessModeWrite)) - } - r.Group("/-/package/{scope:"+scopeRegexp+"}/{id:"+idRegexp+"}/dist-tags", addPackageDistTagsHandlers) - r.Group("/-/package/{id:"+idRegexp+"}/dist-tags", addPackageDistTagsHandlers) - r.Group("/-/v1/search", func() { - r.Get("", npm.PackageSearch) + packageDistTags := "/-/package" + packageId + "/dist-tags" + g.MatchPath("GET", packageDistTags, npm.ListPackageTags) + g.MatchPath("PUT", packageDistTags+"/", reqPackageAccess(perm.AccessModeWrite), npm.AddPackageTag) + g.MatchPath("DELETE", packageDistTags+"/", reqPackageAccess(perm.AccessModeWrite), npm.DeletePackageTag) }) }, reqPackageAccess(perm.AccessModeRead)) r.Group("/pub", func() { diff --git a/routers/api/packages/npm/npm.go b/routers/api/packages/npm/npm.go index 0202f8abccf..55489fc9301 100644 --- a/routers/api/packages/npm/npm.go +++ b/routers/api/packages/npm/npm.go @@ -7,7 +7,6 @@ import ( "bytes" std_ctx "context" "errors" - "fmt" "io" "net/http" "net/url" @@ -44,21 +43,12 @@ func apiError(ctx *context.Context, status int, obj any) { // packageNameFromParams gets the package name from the url parameters func packageNameFromParams(ctx *context.Context) string { - // Real examples: these 2 both should work: + // HINT: NPM-ROUTE-PATH-PATTERN: real examples: these cases all should work: // * "https://registry.npmjs.org/@angular/core" // * "https://registry.npmjs.org/@angular%2Fcore" + // * "https://registry.npmjs.org/%40angular%2Fcore" // - // HINT: NPM-ROUTE-PATH-PATTERN: The cases for the path parameters: - // * ".../TheName/...": id="TheName" - // * ".../@TheScope/TheName/...": scope="@TheScope", id="TheName" - // * ".../@TheScope%2FTheName/...": id="@TheScope/TheName" - scope := ctx.PathParam("scope") - fullOrSub := ctx.PathParam("id") // may be a full name or a subpath of the full package name - if scope != "" { - // now id is the subpath of the full package name, e.g. "core" in "@angular/core" - return fmt.Sprintf("%s/%s", scope, fullOrSub) - } - return fullOrSub // id is the full package name, e.g.: "@angular/core" or "lodash" + return ctx.PathParam("id") // id is the full package name, e.g.: "@angular/core" or "lodash" } func buildNpmRegistryURL(ctx std_ctx.Context, owner *user_model.User) string { diff --git a/tests/integration/api_packages_npm_test.go b/tests/integration/api_packages_npm_test.go index 6b7bd2c0171..a13f053fdb6 100644 --- a/tests/integration/api_packages_npm_test.go +++ b/tests/integration/api_packages_npm_test.go @@ -170,8 +170,9 @@ func TestPackageNpm(t *testing.T) { defer tests.PrintCurrentTest(t)() rootPaths := []string{ - fmt.Sprintf("/api/packages/%s/npm/@scope/test-package", user.Name), - fmt.Sprintf("/api/packages/%s/npm/@scope%%2ftest-package", user.Name), + "/api/packages/user2/npm/@scope/test-package", + "/api/packages/user2/npm/@scope%2Ftest-package", + "/api/packages/user2/npm/%40scope%2ftest-package", } for _, root := range rootPaths { req := NewRequest(t, "GET", fmt.Sprintf("%s/-/%s/%s", root, packageVersion, filename)).AddTokenAuth(token) @@ -186,7 +187,7 @@ func TestPackageNpm(t *testing.T) { pvs, err := packages.GetVersionsByPackageType(t.Context(), user.ID, packages.TypeNpm) assert.NoError(t, err) assert.Len(t, pvs, 1) - assert.Equal(t, int64(4), pvs[0].DownloadCount) + assert.EqualValues(t, 6, pvs[0].DownloadCount) }) t.Run("PackageMetadata", func(t *testing.T) {