From 59a43c8733a89d59cd945024516446af4a949661 Mon Sep 17 00:00:00 2001 From: chudnyi Date: Mon, 24 Aug 2026 21:00:44 +0300 Subject: [PATCH] fix(packages/npm): use PathEscape for package name in tarball URL (#39061) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Replace `url.QueryEscape` with `url.PathEscape` when building `dist.tarball` in the npm package registry. `QueryEscape` leaves `@` unescaped, producing `dist.tarball` URLs like `@scope%2Fname` for scoped packages — which npm clients cannot resolve. `PathEscape` produces the RFC 3986 path-segment-safe encoding (`%40scope%2Fname`) that the npm registry URL format requires. ## Reproduction 1. Publish a scoped npm package (`@scope/name@1.0.0`) to a Gitea package registry. 2. Inspect the `dist.tarball` field in the metadata response. 3. Observe that the package name in the URL is `@scope%2Fname` instead of `%40scope%2Fname`. 4. `npm install @scope/name` fails because npm rejects the malformed tarball URL. ## Fix One-line change in `routers/api/packages/npm/api.go`: ```diff -Tarball: fmt.Sprintf("%s/%s/-/%s/%s", registryURL, url.QueryEscape(pd.Package.Name), url.PathEscape(pd.Version.Version), url.PathEscape(pd.Files[0].File.LowerName)), +Tarball: fmt.Sprintf("%s/%s/-/%s/%s", registryURL, url.PathEscape(pd.Package.Name), url.PathEscape(pd.Version.Version), url.PathEscape(pd.Files[0].File.LowerName)), ``` ## Tests - `routers/api/packages/npm/api_test.go`: extended `TestCreatePackageMetadataResponse` to use `Package.Name: "@scope/test"` and added an `assert.Equal` on `Dist.Tarball` (per review feedback to consolidate the test instead of adding a new one). - `tests/integration/api_packages_npm_test.go`: switched three `url.QueryEscape(packageName)` to `url.PathEscape(packageName)` to match the new production encoding (lines 125, 126, 446). The `TestPackageNpm` assert at line 219 against `pmv.Dist.Tarball` now passes for scoped packages. The unit test fails on `main` (excluding the `QueryEscape` → `PathEscape` swap) and passes with the fix. ## Related Closes #39060. ## Disclosure This contribution was prepared with assistance from an AI coding assistant (limited to language polishing in maintainer-facing messages). The contributor reviewed and validated all changes, including the test cases. --------- Signed-off-by: Dmitriy Chudnyi Signed-off-by: wxiaoguang Co-authored-by: wxiaoguang --- routers/api/packages/npm/api.go | 2 +- routers/api/packages/npm/api_test.go | 11 +++++++++-- tests/integration/api_packages_npm_test.go | 6 +++--- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/routers/api/packages/npm/api.go b/routers/api/packages/npm/api.go index 20da866034a..a2768646cfd 100644 --- a/routers/api/packages/npm/api.go +++ b/routers/api/packages/npm/api.go @@ -99,7 +99,7 @@ func createPackageMetadataVersion(registryURL string, pd *packages_model.Package Dist: npm_module.PackageDistribution{ Shasum: pd.Files[0].Blob.HashSHA1, Integrity: "sha512-" + base64.StdEncoding.EncodeToString(hashBytes), - Tarball: fmt.Sprintf("%s/%s/-/%s/%s", registryURL, url.QueryEscape(pd.Package.Name), url.PathEscape(pd.Version.Version), url.PathEscape(pd.Files[0].File.LowerName)), + Tarball: fmt.Sprintf("%s/%s/-/%s/%s", registryURL, url.PathEscape(pd.Package.Name), url.PathEscape(pd.Version.Version), url.PathEscape(pd.Files[0].File.LowerName)), }, } } diff --git a/routers/api/packages/npm/api_test.go b/routers/api/packages/npm/api_test.go index ab3a5821453..09364a2bdda 100644 --- a/routers/api/packages/npm/api_test.go +++ b/routers/api/packages/npm/api_test.go @@ -19,12 +19,15 @@ import ( func TestCreatePackageMetadataResponse(t *testing.T) { descriptor := func(v string, publishedUnix int64) *packages_model.PackageDescriptor { return &packages_model.PackageDescriptor{ - Package: &packages_model.Package{Name: "test"}, + Package: &packages_model.Package{Name: "@scope/test"}, Owner: &user_model.User{Name: "alice"}, Version: &packages_model.PackageVersion{Version: v, CreatedUnix: timeutil.TimeStamp(publishedUnix)}, SemVer: version.Must(version.NewVersion(v)), Metadata: &npm_module.Metadata{Keywords: []string{"gitea"}}, - Files: []*packages_model.PackageFileDescriptor{{File: &packages_model.PackageFile{}, Blob: &packages_model.PackageBlob{}}}, + Files: []*packages_model.PackageFileDescriptor{{ + File: &packages_model.PackageFile{LowerName: "test-" + v + ".tgz"}, + Blob: &packages_model.PackageBlob{}, + }}, } } @@ -43,4 +46,8 @@ func TestCreatePackageMetadataResponse(t *testing.T) { assert.Equal(t, []string{"gitea"}, result.Keywords) assert.Equal(t, []string{"gitea"}, result.Versions["1.0.0"].Keywords) assert.Equal(t, []npm_module.User{{Name: "alice"}}, result.Versions["1.0.0"].Maintainers) + assert.Equal(t, + "https://gitea.dev/api/packages/alice/npm/@scope%2Ftest/-/1.0.0/test-1.0.0.tgz", + result.Versions["1.0.0"].Dist.Tarball, + ) } diff --git a/tests/integration/api_packages_npm_test.go b/tests/integration/api_packages_npm_test.go index ab8671a0953..55a3ea70724 100644 --- a/tests/integration/api_packages_npm_test.go +++ b/tests/integration/api_packages_npm_test.go @@ -122,8 +122,8 @@ func TestPackageNpm(t *testing.T) { }` } - root := fmt.Sprintf("/api/packages/%s/npm/%s", user.Name, url.QueryEscape(packageName)) - tagsRoot := fmt.Sprintf("/api/packages/%s/npm/-/package/%s/dist-tags", user.Name, url.QueryEscape(packageName)) + root := fmt.Sprintf("/api/packages/%s/npm/%s", user.Name, url.PathEscape(packageName)) + tagsRoot := fmt.Sprintf("/api/packages/%s/npm/-/package/%s/dist-tags", user.Name, url.PathEscape(packageName)) filename := fmt.Sprintf("%s-%s.tgz", strings.Split(packageName, "/")[1], packageVersion) t.Run("Upload", func(t *testing.T) { @@ -443,7 +443,7 @@ func TestPackageNpm(t *testing.T) { // authoritative tarball scan must overrule the claim. claimPackageName := "@scope/test-shrinkwrap-claim" claimVersion := "1.0.0" - claimRoot := fmt.Sprintf("/api/packages/%s/npm/%s", user.Name, url.QueryEscape(claimPackageName)) + claimRoot := fmt.Sprintf("/api/packages/%s/npm/%s", user.Name, url.PathEscape(claimPackageName)) body := `{ "_id": "` + claimPackageName + `", "name": "` + claimPackageName + `",