fix: restore missing blob file when re-publishing a package

[Huang-404-Q]

Problem

After publishing a NuGet package (the reporter's package contains a zero-byte lib/netstandard2.0/_._ placeholder entry), the package metadata is present but downloads fail with:

OpenFileForDownload, open /var/lib/gitea/packages/bb/d9/...: no such file or directory

Re-publishing the package — even after deleting it — never fixes it (issue #39215, reproduced on 1.26.4 through 1.27.3).

Root cause

Not the zero-byte entry: a .nupkg is stored as one opaque, content-addressed blob, and NuGet metadata parsing never inspects individual zip entries. The bug is in addFileToPackageVersionUnchecked (services/packages/packages.go): when GetOrInsertBlob reports the blob row already exists, the code skips contentStore.Save.

So once the state "blob row exists, storage file missing" is reached (a storage-level loss — the reporter's file was gone from both the container and the host volume), every re-publish of the same content only inserts new package_file rows and never rewrites the file. Deleting the package does not help because blob rows/files are removed by the cleanup cron (deferred deletion), not by package removal — the row keeps existing, so re-publishing keeps being skipped, and downloads fail forever.

Notably, the container registry path already carries this exact consistency check as a workaround for the same bug (#19586, FIXME in routers/api/packages/container/blob.go); the generic/NuGet path was simply missing it.

Fix

In addFileToPackageVersionUnchecked, when the blob row exists, verify the file is actually in the content store via contentStore.Has(...); if it reports not-exist, treat the blob as missing and Save it. The file is then restored on the next re-publish. Covers every registry type that goes through this function (nuget/generic/conda/cran/rpm/terraform/...), not just NuGet. Cost: one extra Stat/Head per re-publish with duplicate content — the container path already does this on every push.

This is a self-healing fix consistent with the upstream container-registry strategy; the original cause of the file loss is not identifiable from the in-repo code paths (all of them remove row + file together).

Test (1 new, red → green)

TestCreatePackageAndAddFileRestoresMissingBlobFile in services/packages/packages_test.go (service-level, unittest.MainTest + sqlite): builds a nupkg with test.WriteZipArchive mirroring the issue's package shape (nuspec + zero-byte lib/netstandard2.0/_._), uploads it, simulates the storage inconsistency by deleting the blob file while the row survives (contentStore.Delete), then re-publishes identical content under a second package name.

Before the fix: fails — the second package's download hits the issue's exact error (open .../data/packages/f3/61/...: no such file or directory). After the fix: passes — the blob file is restored and both packages download byte-identical content through OpenFileForDownload.

Regression: go test ./models/packages/... ./services/packages/... — all packages pass at baseline and after the fix (0 failures, no diff); gofmt/go vet clean on the changed files. The integration suite (tests/integration/api_packages_nuget_test.go) was not run locally (requires the compiled-binary integration environment); the service-level test exercises the same OpenFileForDownload entry point.

Fixes #39215

[wxiaoguang]

Please review and fix this line by human.

[Huang-404-Q]

Thanks for the review. This line mirrors the check that already exists on the container registry path for the same inconsistency (the FIXME workaround for #19586 in routers/api/packages/container/blob.go):

err = contentStore.Has(packages_module.BlobHash256Key(pb.HashSHA256))
if err != nil && (errors.Is(err, util.ErrNotExist) || errors.Is(err, os.ErrNotExist)) {
    exists = false
}

The intent: GetOrInsertBlob returning exists=true only proves the row exists, not the file. When the file is missing (Has reports not-exist), treat the blob as missing so the Save below rewrites it — otherwise, after a "row present, file lost" storage inconsistency, every re-publish of the same content inserts new package_file rows and never restores the file, and downloads fail forever (the failure mode in #39215).

One deliberate choice to flag: if Has fails with any error other than not-exist (e.g. an I/O error), we keep exists=true and skip the Save. That matches the container path exactly — a transient storage error should not silently turn into "file missing" and risk dropping the blob from further consideration; and in the common case (local file system, S3) a read error on an existing file is far less likely than a plain not-exist.

Happy to adjust if you'd rather handle the error differently (e.g. propagate it, or treat any Has error as missing).

[wxiaoguang]

https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md#ai-contribution-policy

No time to read AI response.

Reply by human. Just tell people what's the right thing to do

[wxiaoguang]

contentStore.Has only returns os.ErrNotExist, it never returns util.ErrNotExist

Old code is not right either.

[Huang-404-Q]

Fixed in 4590db1 — you're right, the storage layer only ever returns os.ErrNotExist, so the util.ErrNotExist check was dead code. Dropped it, kept the os.ErrNotExist check.

[wxiaoguang]

Thank you very much. I will try to refactor the legacy code together.