From 30c07c20e94551141cc1873ab14bdd4c104bba94 Mon Sep 17 00:00:00 2001 From: Rohan Guliani Date: Fri, 3 Apr 2026 02:12:04 -0400 Subject: [PATCH] Fix RPM Registry 404 when package name contains 'package' (#37087) Fixes #37086, fix the bug in MatchPath, and swap the order of overlapping routes in api.go to make it look better. --------- Signed-off-by: Rohan Guliani Co-authored-by: wxiaoguang --- modules/web/router_path.go | 23 ++++++++++++++++++++--- modules/web/router_test.go | 7 ++++++- routers/api/packages/api.go | 2 +- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/modules/web/router_path.go b/modules/web/router_path.go index 9e531346d1..7278241b6d 100644 --- a/modules/web/router_path.go +++ b/modules/web/router_path.go @@ -55,6 +55,7 @@ func (g *RouterPathGroup) MatchPattern(methods string, pattern *RouterPathGroupP type routerPathParam struct { name string + pathSepEnd bool captureGroup int } @@ -93,7 +94,15 @@ func (p *routerPathMatcher) matchPath(chiCtx *chi.Context, path string) bool { } for i, pm := range paramMatches { groupIdx := p.params[i].captureGroup * 2 - chiCtx.URLParams.Add(p.params[i].name, path[pm[groupIdx]:pm[groupIdx+1]]) + if pm[groupIdx] == -1 || pm[groupIdx+1] == -1 { + chiCtx.URLParams.Add(p.params[i].name, "") + continue + } + val := path[pm[groupIdx]:pm[groupIdx+1]] + if p.params[i].pathSepEnd { + val = strings.TrimSuffix(val, "/") + } + chiCtx.URLParams.Add(p.params[i].name, val) } return true } @@ -145,11 +154,19 @@ func patternRegexp(pattern string, h ...any) *RouterPathGroupPattern { // it is not used so no need to implement it now param := routerPathParam{} if partExp == "*" { - re = append(re, "(.*?)/?"...) + // "" is a shorthand for optionally matching any string (but not greedy) + partExp = ".*?" if lastEnd < len(pattern) && pattern[lastEnd] == '/' { - lastEnd++ // the "*" pattern is able to handle the last slash, so skip it + // if this param part ends with path separator "/", then consider it together: "(.*?/)" + partExp += "/" + param.pathSepEnd = true + lastEnd++ } + re = append(re, '(') + re = append(re, partExp...) + 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/modules/web/router_test.go b/modules/web/router_test.go index 645e70b869..d424f072e9 100644 --- a/modules/web/router_test.go +++ b/modules/web/router_test.go @@ -100,7 +100,8 @@ func TestPathProcessor(t *testing.T) { chiCtx := chi.NewRouteContext() chiCtx.RouteMethod = "GET" p := newRouterPathMatcher("GET", patternRegexp(pattern), http.NotFound) - assert.True(t, p.matchPath(chiCtx, uri), "use pattern %s to process uri %s", pattern, uri) + shouldProcess := expectedPathParams != nil + assert.Equal(t, shouldProcess, p.matchPath(chiCtx, uri), "use pattern %s to process uri %s", pattern, uri) assert.Equal(t, expectedPathParams, chiURLParamsToMap(chiCtx), "use pattern %s to process uri %s", pattern, uri) } @@ -113,6 +114,10 @@ func TestPathProcessor(t *testing.T) { testProcess("//", "/a", map[string]string{"p1": "", "p2": "a"}) testProcess("//", "/a/b", map[string]string{"p1": "a", "p2": "b"}) testProcess("//", "/a/b/c", map[string]string{"p1": "a/b", "p2": "c"}) + testProcess("//part/", "/a/part/c", map[string]string{"p1": "a", "p2": "c"}) + 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"}) } func TestRouter(t *testing.T) { diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index 44bb80018b..ec5326130e 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -473,8 +473,8 @@ 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) - g.MatchPath("HEAD,GET", "//package///", rpm.DownloadPackageFile) g.MatchPath("HEAD,GET", "//package////", rpm.DownloadPackageFile) + g.MatchPath("HEAD,GET", "//package///", rpm.DownloadPackageFile) g.MatchPath("DELETE", "//package///", reqPackageAccess(perm.AccessModeWrite), rpm.DeletePackageFile) }, reqPackageAccess(perm.AccessModeRead))