diff --git a/modules/packages/content_store.go b/modules/packages/content_store.go index 7554c4900e3..3df9e34a263 100644 --- a/modules/packages/content_store.go +++ b/modules/packages/content_store.go @@ -4,11 +4,14 @@ package packages import ( + "errors" "io" + "io/fs" "net/url" "path" "strings" + "gitea.dev/modules/optional" "gitea.dev/modules/setting" "gitea.dev/modules/storage" "gitea.dev/modules/util" @@ -47,6 +50,17 @@ func (s *ContentStore) Has(key BlobHash256Key) error { return err } +func (s *ContentStore) OptionalSize(key BlobHash256Key) (sz optional.Option[int64], _ error) { + st, err := s.store.Stat(KeyToRelativePath(key)) + if errors.Is(err, fs.ErrNotExist) { + return sz, nil + } + if err != nil { + return sz, err + } + return optional.Some(st.Size()), nil +} + // Save stores a package blob func (s *ContentStore) Save(key BlobHash256Key, r io.Reader, size int64) error { _, err := s.store.Save(KeyToRelativePath(key), r, size) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index b8486acbe60..529b330d9a2 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1617,7 +1617,7 @@ func GetPullRequestFiles(ctx *context.APIContext) { limit = max(limit, 0) apiFiles := make([]*api.ChangedFile, 0, limit) - for i := start; i < start+limit; i++ { + for i := start; i < start+limit && i < len(diff.Files); i++ { // refs/pull/1/head stores the HEAD commit ID, allowing all related commits to be found in the base repository. // The head repository might have been deleted, so we should not rely on it here. apiFiles = append(apiFiles, convert.ToChangedFile(diff.Files[i], pr.BaseRepo, endCommitID)) diff --git a/services/context/context_template.go b/services/context/context_template.go index b1c213ca9e1..2010432e3c5 100644 --- a/services/context/context_template.go +++ b/services/context/context_template.go @@ -12,7 +12,6 @@ import ( "strings" "time" - "gitea.dev/modules/htmlutil" "gitea.dev/modules/httplib" "gitea.dev/modules/public" "gitea.dev/modules/reqctx" @@ -139,5 +138,5 @@ func (c TemplateContext) HeadMetaContentSecurityPolicy() template.HTML { if csp == "" { return "" } - return htmlutil.HTMLFormat(``, csp) + return template.HTML(``) } diff --git a/services/packages/packages.go b/services/packages/packages.go index abd4b4c54db..1e669a6b2c7 100644 --- a/services/packages/packages.go +++ b/services/packages/packages.go @@ -262,6 +262,42 @@ func NewPackageBlob(hsr packages_module.HashedSizeReader) *packages_model.Packag } } +func GetOrSavePackageBlob(ctx context.Context, contentStore *packages_module.ContentStore, blob *packages_model.PackageBlob, data packages_module.HashedSizeReader) (_ *packages_model.PackageBlob, _ bool, retErr error) { + if blob.Size != data.Size() { + return nil, false, fmt.Errorf("size mismatch: blob size %d, data size %d", blob.Size, data.Size()) + } + pb, existsInDatabase, err := packages_model.GetOrInsertBlob(ctx, blob) + if err != nil { + return nil, false, fmt.Errorf("unable to get or insert blob: %w", err) + } + + defer func() { + if retErr != nil && !existsInDatabase { + if errDelete := packages_model.DeleteBlobByID(ctx, pb.ID); errDelete != nil { + log.Error("unable to delete blob from database after failed save in content store: %v", errDelete) + } + } + }() + + var objSize optional.Option[int64] + storeKey := packages_module.BlobHash256Key(pb.HashSHA256) + if existsInDatabase { + // check if the blob file actually is valid in the content store + objSize, err = contentStore.OptionalSize(storeKey) + if err != nil { + return nil, false, fmt.Errorf("unable to check object size in content store: %w", err) + } + } + if objSize.ValueOrDefault(-1) != blob.Size { + if err := contentStore.Save(storeKey, data, data.Size()); err != nil { + return nil, false, fmt.Errorf("unable to save object in content store: %w", err) + } + } + // existsInDatabase controls the "roll back", if other errors happen later, + // the "non-existing (newly created)" blob will be deleted from the content store, but not if it already existed in the database. + return pb, existsInDatabase, nil +} + func addFileToPackageVersion(ctx context.Context, pv *packages_model.PackageVersion, pvi *PackageInfo, pfci *PackageFileCreationInfo) (*packages_model.PackageFile, *packages_model.PackageBlob, bool, error) { if err := CheckSizeQuotaExceeded(ctx, pfci.Creator, pvi.Owner, pvi.PackageType, pfci.Data.Size()); err != nil { return nil, nil, false, err @@ -273,18 +309,11 @@ func addFileToPackageVersion(ctx context.Context, pv *packages_model.PackageVers func addFileToPackageVersionUnchecked(ctx context.Context, pv *packages_model.PackageVersion, pfci *PackageFileCreationInfo) (*packages_model.PackageFile, *packages_model.PackageBlob, bool, error) { log.Trace("Adding package file: %v, %s", pv.ID, pfci.Filename) - pb, exists, err := packages_model.GetOrInsertBlob(ctx, NewPackageBlob(pfci.Data)) + pb, exists, err := GetOrSavePackageBlob(ctx, packages_module.NewContentStore(), NewPackageBlob(pfci.Data), pfci.Data) if err != nil { log.Error("Error inserting package blob: %v", err) return nil, nil, false, err } - if !exists { - contentStore := packages_module.NewContentStore() - if err := contentStore.Save(packages_module.BlobHash256Key(pb.HashSHA256), pfci.Data, pfci.Data.Size()); err != nil { - log.Error("Error saving package blob in content store: %v", err) - return nil, nil, false, err - } - } if pfci.OverwriteExisting { pf, err := packages_model.GetFileForVersionByName(ctx, pv.ID, pfci.Filename, pfci.CompositeKey) diff --git a/services/packages/packages_test.go b/services/packages/packages_test.go new file mode 100644 index 00000000000..4908fd4778e --- /dev/null +++ b/services/packages/packages_test.go @@ -0,0 +1,96 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package packages + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "io" + "net/http" + "testing" + + packages_model "gitea.dev/models/packages" + "gitea.dev/models/unittest" + user_model "gitea.dev/models/user" + packages_module "gitea.dev/modules/packages" + "gitea.dev/modules/test" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m) +} + +func TestCreatePackageAndAddFileRestoresMissingBlobFile(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + uploadPackage := func(t *testing.T, user *user_model.User, name, filename string, data []byte) (*packages_model.PackageFile, error) { + buf, err := packages_module.CreateHashedBufferFromReader(bytes.NewReader(data)) + require.NoError(t, err) + _, pf, err := CreatePackageAndAddFile(t.Context(), + &PackageCreationInfo{ + PackageInfo: PackageInfo{ + Owner: user, + PackageType: packages_model.TypeNuGet, + Name: name, + Version: "1.0.0", + }, + SemverCompatible: true, + Creator: user, + }, + &PackageFileCreationInfo{ + PackageFileInfo: PackageFileInfo{ + Filename: filename, + }, + Creator: user, + Data: buf, + IsLead: true, + }) + return pf, err + } + + // This test data is from https://github.com/go-gitea/gitea/issues/39215, it doesn't really matter, actually. + // The key point is that if the blob object is missing in the content storage, it must be restored when uploaded again. + pkgData := test.WriteZipArchive(map[string]string{ + "package.nuspec": "nuget.repro1.0.0", + "lib/netstandard2.0/_._": "", + }).Bytes() + pkgDataSum := sha256.Sum256(pkgData) + key := packages_module.BlobHash256Key(hex.EncodeToString(pkgDataSum[:])) + contentStore := packages_module.NewContentStore() + + // The initial upload writes the blob row and its file + pf1, err := uploadPackage(t, user, "nuget.repro", "nuget.repro.1.0.0.nupkg", pkgData) + require.NoError(t, err) + sz, err := contentStore.OptionalSize(key) + assert.NoError(t, err) + assert.EqualValues(t, len(pkgData), sz.ValueOrDefault(-1)) + + // Simulate the storage inconsistency: the blob row survives but its file is missing + require.NoError(t, contentStore.Delete(key)) + sz, err = contentStore.OptionalSize(key) + assert.NoError(t, err) + assert.EqualValues(t, -1, sz.ValueOrDefault(-1)) + + // Publishing a package with identical content must restore the blob file + pf2, err := uploadPackage(t, user, "nuget.repro-copy", "nuget.repro-copy.1.0.0.nupkg", pkgData) + require.NoError(t, err) + sz, err = contentStore.OptionalSize(key) + assert.NoError(t, err) + assert.EqualValues(t, len(pkgData), sz.ValueOrDefault(-1)) + + // The blob file must be present and both packages must be downloadable + for _, pf := range []*packages_model.PackageFile{pf1, pf2} { + s, _, _, err := OpenFileForDownload(t.Context(), pf, http.MethodGet) + require.NoError(t, err) + respData, err := io.ReadAll(s) + require.NoError(t, err) + assert.NoError(t, s.Close()) + assert.Equal(t, pkgData, respData) + } +} diff --git a/templates/admin/packages/list.tmpl b/templates/admin/packages/list.tmpl index bf71a12d1e6..d60a1597136 100644 --- a/templates/admin/packages/list.tmpl +++ b/templates/admin/packages/list.tmpl @@ -55,10 +55,12 @@ {{.Version.ID}} + {{if .Owner}} {{.Owner.Name}} {{if .Owner.Visibility.IsPrivate}} {{svg "octicon-lock"}} {{end}} + {{end}} {{.Package.Type.Name}} {{.Package.Name}} diff --git a/templates/admin/repo/list.tmpl b/templates/admin/repo/list.tmpl index 02205793c44..1f21ce90aab 100644 --- a/templates/admin/repo/list.tmpl +++ b/templates/admin/repo/list.tmpl @@ -47,10 +47,12 @@ {{.ID}} + {{if .Owner}} {{.Owner.Name}} {{if .Owner.Visibility.IsPrivate}} {{svg "octicon-lock"}} {{end}} + {{end}} {{.Name}} @@ -59,7 +61,7 @@ {{end}} {{if .IsPrivate}} {{ctx.Locale.Tr "repo.desc.private"}} - {{else}} + {{else if .Owner}} {{if .Owner.Visibility.IsPrivate}} {{ctx.Locale.Tr "repo.desc.internal"}} {{end}} diff --git a/web_src/js/features/dropzone.ts b/web_src/js/features/dropzone.ts index 2b9333263fb..6e74ddb7522 100644 --- a/web_src/js/features/dropzone.ts +++ b/web_src/js/features/dropzone.ts @@ -132,7 +132,7 @@ export async function initDropzone(dropzoneEl: HTMLElement) { const file = {name: attachment.name, uuid: attachment.uuid, size: attachment.size}; dzInst.emit('addedfile', file); dzInst.emit('complete', file); - if (isImageFile(file.name)) { + if (isImageFile(file)) { const imgSrc = `${attachmentBaseLinkUrl}/${file.uuid}`; dzInst.emit('thumbnail', file, imgSrc); } diff --git a/web_src/js/modules/fomantic/modal.ts b/web_src/js/modules/fomantic/modal.ts index d383b10ff4b..c32c3532672 100644 --- a/web_src/js/modules/fomantic/modal.ts +++ b/web_src/js/modules/fomantic/modal.ts @@ -66,9 +66,9 @@ function onModalBeforeHidden(this: any) { function onModalApproveDefault(this: any) { const $modal = $(this); const selectors = $modal.modal('setting', 'selector'); - const elModal = $modal[0]; - const elApprove = elModal.querySelector(selectors.approve); - const elForm = elApprove?.closest('form'); + const elModal = $modal[0] as HTMLElement; + const elApprove = elModal.querySelector(selectors.approve); + const elForm = elApprove?.closest('form'); if (!elForm) return true; // no form, just allow closing the modal // "form-fetch-action" can handle network errors gracefully, @@ -78,6 +78,7 @@ function onModalApproveDefault(this: any) { // There is an abuse for the "modal" + "form" combination, the "Approve" button is a traditional form submit button in the form. // Then "approve" and "submit" occur at the same time, the modal will be closed immediately before the form is submitted. // So here we prevent the modal from closing automatically by returning false, add the "is-loading" class to the form element. + if (!elForm.reportValidity()) return false; elForm.classList.add('is-loading'); return false; }