diff --git a/modules/web/router_path.go b/modules/web/router_path.go index 0d17c953af..7278241b6d 100644 --- a/modules/web/router_path.go +++ b/modules/web/router_path.go @@ -55,7 +55,7 @@ func (g *RouterPathGroup) MatchPattern(methods string, pattern *RouterPathGroupP type routerPathParam struct { name string - patSepEnd bool + pathSepEnd bool captureGroup int } @@ -99,7 +99,7 @@ func (p *routerPathMatcher) matchPath(chiCtx *chi.Context, path string) bool { continue } val := path[pm[groupIdx]:pm[groupIdx+1]] - if p.params[i].patSepEnd { + if p.params[i].pathSepEnd { val = strings.TrimSuffix(val, "/") } chiCtx.URLParams.Add(p.params[i].name, val) @@ -154,16 +154,19 @@ func patternRegexp(pattern string, h ...any) *RouterPathGroupPattern { // it is not used so no need to implement it now param := routerPathParam{} if partExp == "*" { + // "" is a shorthand for optionally matching any string (but not greedy) partExp = ".*?" if lastEnd < len(pattern) && pattern[lastEnd] == '/' { + // if this param part ends with path separator "/", then consider it together: "(.*?/)" partExp += "/" - param.patSepEnd = true + param.pathSepEnd = true lastEnd++ } re = append(re, '(') re = append(re, partExp...) - re = append(re, ')', '?') + re = append(re, ')', '?') // the wildcard matching is optional } else { + // the pattern is user-provided regexp, defaults to a path part (separated by "/") partExp = util.IfZero(partExp, "[^/]+") re = append(re, '(') re = append(re, partExp...) diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index eb8febdb17..ec5326130e 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -473,12 +473,6 @@ func CommonRoutes() *web.Router { g.MatchPath("GET", "//repodata/", rpm.GetRepositoryFile) g.MatchPath("PUT", "//upload", reqPackageAccess(perm.AccessModeWrite), rpm.UploadPackageFile) // this URL pattern is only used internally in the RPM index, it is generated by us, the filename part is not really used (can be anything) - // ~~Order is important here: the more specific long route (with filename) must come first.~~ - // ~~Otherwise, when a package name contains "package",~~ - // ~~* request path: ".../rpm/package/gitea-package/1.0.2-1/x86_64/any-file-name"~~ - // ~~* it also matches "/rpm//package///",~~ - // TODO: the MatchPath has been fixed, the order is not important anymore - // As long as the CI passes, we can remove the comments and the new test case (not needed anymore) g.MatchPath("HEAD,GET", "//package////", rpm.DownloadPackageFile) g.MatchPath("HEAD,GET", "//package///", rpm.DownloadPackageFile) g.MatchPath("DELETE", "//package///", reqPackageAccess(perm.AccessModeWrite), rpm.DeletePackageFile) diff --git a/tests/integration/api_packages_rpm_test.go b/tests/integration/api_packages_rpm_test.go index 59f6c4a1f1..61bd8ffc63 100644 --- a/tests/integration/api_packages_rpm_test.go +++ b/tests/integration/api_packages_rpm_test.go @@ -15,7 +15,6 @@ import ( "strings" "testing" - "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/packages" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -165,29 +164,6 @@ gpgkey=%sapi/packages/%s/rpm/repository.key`, req = NewRequest(t, "GET", fmt.Sprintf("%s/package/%s/%s/%s/any-file-name", groupURL, packageName, packageVersion, packageArchitecture)) resp = MakeRequest(t, req, http.StatusOK) assert.Equal(t, content, resp.Body.Bytes()) - - // Test with "package" in the name (reproduces bug if fix not applied) - pvs, err := packages.GetVersionsByPackageType(t.Context(), user.ID, packages.TypeRpm) - assert.NoError(t, err) - require.NotEmpty(t, pvs) - - // Rename package to "gitea-package" in DB to test collision - _, err = db.GetEngine(t.Context()).Exec("UPDATE package SET name = ?, lower_name = ? WHERE id = ?", "gitea-package", "gitea-package", pvs[0].PackageID) - assert.NoError(t, err) - - // Rename package_file to match new name - _, err = db.GetEngine(t.Context()).Exec("UPDATE package_file SET name = ?, lower_name = ? WHERE version_id = ?", "gitea-package-1.0.2-1.x86_64.rpm", "gitea-package-1.0.2-1.x86_64.rpm", pvs[0].ID) - assert.NoError(t, err) - - defer func() { - // Restore name - db.GetEngine(t.Context()).Exec("UPDATE package SET name = ?, lower_name = ? WHERE id = ?", packageName, strings.ToLower(packageName), pvs[0].PackageID) - db.GetEngine(t.Context()).Exec("UPDATE package_file SET name = ?, lower_name = ? WHERE version_id = ?", fmt.Sprintf("%s-%s.%s.rpm", packageName, packageVersion, packageArchitecture), strings.ToLower(fmt.Sprintf("%s-%s.%s.rpm", packageName, packageVersion, packageArchitecture)), pvs[0].ID) - }() - - req = NewRequest(t, "GET", fmt.Sprintf("%s/package/%s/%s/%s/any-file-name", groupURL, "gitea-package", packageVersion, packageArchitecture)) - resp = MakeRequest(t, req, http.StatusOK) - assert.Equal(t, content, resp.Body.Bytes()) }) t.Run("Repository", func(t *testing.T) {