Skip to content

Commit 7880aed

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 376bf01 commit 7880aed

File tree

6 files changed

+90
-123
lines changed

6 files changed

+90
-123
lines changed

‎models/db/context.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,15 @@ func WithTx(parentCtx context.Context, f func(ctx context.Context) error) error
178178
return txWithNoCheck(parentCtx, f)
179179
}
180180

181+
// WithTx2 is similar to WithTx, but it has two return values: result and error.
182+
func WithTx2[T any](parentCtx context.Context, f func(ctx context.Context) (T, error)) (ret T, errRet error) {
183+
errRet = WithTx(parentCtx, func(ctx context.Context) (errInner error) {
184+
ret, errInner = f(ctx)
185+
return errInner
186+
})
187+
return ret, errRet
188+
}
189+
181190
func txWithNoCheck(parentCtx context.Context, f func(ctx context.Context) error) error {
182191
sess := xormEngine.NewSession()
183192
defer sess.Close()

‎routers/api/packages/container/auth.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func (a *Auth) Name() string {
2121
}
2222

2323
// Verify extracts the user from the Bearer token
24-
// If it's an anonymous session a ghost user is returned
24+
// If it's an anonymous session, a ghost user is returned
2525
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
2626
packageMeta, err := packages.ParseAuthorizationRequest(req)
2727
if err != nil {

‎routers/api/packages/container/blob.go‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -95,15 +95,13 @@ func containerGlobalLockKey(piOwnerID int64, piName, usage string) string {
9595
}
9696

9797
func getOrCreateUploadVersion(ctx context.Context, pi *packages_service.PackageInfo) (*packages_model.PackageVersion, error) {
98-
var uploadVersion *packages_model.PackageVersion
99-
10098
releaser, err := globallock.Lock(ctx, containerGlobalLockKey(pi.Owner.ID, pi.Name, "package"))
10199
if err != nil {
102100
return nil, err
103101
}
104102
defer releaser()
105103

106-
err = db.WithTx(ctx, func(ctx context.Context) error {
104+
return db.WithTx2(ctx, func(ctx context.Context) (*packages_model.PackageVersion, error) {
107105
created := true
108106
p := &packages_model.Package{
109107
OwnerID: pi.Owner.ID,
@@ -115,15 +113,15 @@ func getOrCreateUploadVersion(ctx context.Context, pi *packages_service.PackageI
115113
if p, err = packages_model.TryInsertPackage(ctx, p); err != nil {
116114
if !errors.Is(err, packages_model.ErrDuplicatePackage) {
117115
log.Error("Error inserting package: %v", err)
118-
return err
116+
return nil, err
119117
}
120118
created = false
121119
}
122120

123121
if created {
124122
if _, err := packages_model.InsertProperty(ctx, packages_model.PropertyTypePackage, p.ID, container_module.PropertyRepository, strings.ToLower(pi.Owner.LowerName+"/"+pi.Name)); err != nil {
125123
log.Error("Error setting package property: %v", err)
126-
return err
124+
return nil, err
127125
}
128126
}
129127

@@ -138,16 +136,11 @@ func getOrCreateUploadVersion(ctx context.Context, pi *packages_service.PackageI
138136
if pv, err = packages_model.GetOrInsertVersion(ctx, pv); err != nil {
139137
if !errors.Is(err, packages_model.ErrDuplicatePackageVersion) {
140138
log.Error("Error inserting package: %v", err)
141-
return err
139+
return nil, err
142140
}
143141
}
144-
145-
uploadVersion = pv
146-
147-
return nil
142+
return pv, nil
148143
})
149-
150-
return uploadVersion, err
151144
}
152145

153146
func createFileForBlob(ctx context.Context, pv *packages_model.PackageVersion, pb *packages_model.PackageBlob) error {

‎routers/api/packages/container/container.go‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"regexp"
1414
"strconv"
1515
"strings"
16+
"sync"
1617

1718
auth_model "code.gitea.io/gitea/models/auth"
1819
packages_model "code.gitea.io/gitea/models/packages"
@@ -39,10 +40,14 @@ import (
3940
// https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pushing-manifests
4041
const maxManifestSize = 10 * 1024 * 1024
4142

42-
var (
43-
imageNamePattern = regexp.MustCompile(`\A[a-z0-9]+([._-][a-z0-9]+)*(/[a-z0-9]+([._-][a-z0-9]+)*)*\z`)
44-
referencePattern = regexp.MustCompile(`\A[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}\z`)
45-
)
43+
var globalVars = sync.OnceValue(func() (ret struct {
44+
imageNamePattern, referencePattern *regexp.Regexp
45+
},
46+
) {
47+
ret.imageNamePattern = regexp.MustCompile(`\A[a-z0-9]+([._-][a-z0-9]+)*(/[a-z0-9]+([._-][a-z0-9]+)*)*\z`)
48+
ret.referencePattern = regexp.MustCompile(`\A[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}\z`)
49+
return ret
50+
})
4651

4752
type containerHeaders struct {
4853
Status int
@@ -84,9 +89,7 @@ func jsonResponse(ctx *context.Context, status int, obj any) {
8489
Status: status,
8590
ContentType: "application/json",
8691
})
87-
if err := json.NewEncoder(ctx.Resp).Encode(obj); err != nil {
88-
log.Error("JSON encode: %v", err)
89-
}
92+
_ = json.NewEncoder(ctx.Resp).Encode(obj) // ignore network errors
9093
}
9194

9295
func apiError(ctx *context.Context, status int, err error) {
@@ -134,7 +137,7 @@ func ReqContainerAccess(ctx *context.Context) {
134137

135138
// VerifyImageName is a middleware which checks if the image name is allowed
136139
func VerifyImageName(ctx *context.Context) {
137-
if !imageNamePattern.MatchString(ctx.PathParam("image")) {
140+
if !globalVars().imageNamePattern.MatchString(ctx.PathParam("image")) {
138141
apiErrorDefined(ctx, errNameInvalid)
139142
}
140143
}
@@ -216,7 +219,7 @@ func GetRepositoryList(ctx *context.Context) {
216219
if len(repositories) == n {
217220
v := url.Values{}
218221
if n > 0 {
219-
v.Add("n", strconv.Itoa(n))
222+
v.Add("n", strconv.Itoa(n)) // FIXME: "n" can't be zero here, the logic is inconsistent with GetTagsList
220223
}
221224
v.Add("last", repositories[len(repositories)-1])
222225

@@ -565,7 +568,7 @@ func PutManifest(ctx *context.Context) {
565568
IsTagged: digest.Digest(reference).Validate() != nil,
566569
}
567570

568-
if mci.IsTagged && !referencePattern.MatchString(reference) {
571+
if mci.IsTagged && !globalVars().referencePattern.MatchString(reference) {
569572
apiErrorDefined(ctx, errManifestInvalid.WithMessage("Tag is invalid"))
570573
return
571574
}
@@ -618,7 +621,7 @@ func getBlobSearchOptionsFromContext(ctx *context.Context) (*container_model.Blo
618621
reference := ctx.PathParam("reference")
619622
if d := digest.Digest(reference); d.Validate() == nil {
620623
opts.Digest = string(d)
621-
} else if referencePattern.MatchString(reference) {
624+
} else if globalVars().referencePattern.MatchString(reference) {
622625
opts.Tag = reference
623626
opts.OnlyLead = true
624627
} else {
@@ -782,7 +785,8 @@ func GetTagsList(ctx *context.Context) {
782785
})
783786
}
784787

785-
// FIXME: Workaround to be removed in v1.20
788+
// FIXME: Workaround to be removed in v1.20.
789+
// Update maybe we should never really remote it, as long as there is legacy data?
786790
// https://github.com/go-gitea/gitea/issues/19586
787791
func workaroundGetContainerBlob(ctx *context.Context, opts *container_model.BlobSearchOptions) (*packages_model.PackageFileDescriptor, error) {
788792
blob, err := container_model.GetContainerBlob(ctx, opts)

‎routers/api/packages/container/manifest.go‎

Lines changed: 58 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -77,24 +77,33 @@ func processManifest(ctx context.Context, mci *manifestCreationInfo, buf *packag
7777
return "", errManifestInvalid
7878
}
7979

80-
func processOciImageManifest(ctx context.Context, mci *manifestCreationInfo, buf *packages_module.HashedBuffer) (string, error) {
81-
manifestDigest := ""
80+
type processManifestTxRet struct {
81+
pv *packages_model.PackageVersion
82+
pb *packages_model.PackageBlob
83+
created bool
84+
digest string
85+
}
8286

83-
err := func() error {
84-
manifest, configDescriptor, metadata, err := container_service.ParseManifestMetadata(ctx, buf, mci.Owner.ID, mci.Image)
85-
if err != nil {
86-
return err
87-
}
88-
if _, err = buf.Seek(0, io.SeekStart); err != nil {
89-
return err
90-
}
87+
func processOciImageManifest(ctx context.Context, mci *manifestCreationInfo, buf *packages_module.HashedBuffer) (manifestDigest string, errRet error) {
88+
manifest, configDescriptor, metadata, err := container_service.ParseManifestMetadata(ctx, buf, mci.Owner.ID, mci.Image)
89+
if err != nil {
90+
return "", err
91+
}
92+
if _, err = buf.Seek(0, io.SeekStart); err != nil {
93+
return "", err
94+
}
9195

92-
ctx, committer, err := db.TxContext(ctx)
93-
if err != nil {
94-
return err
96+
var txRet processManifestTxRet
97+
defer func() {
98+
if errRet != nil && txRet.created && txRet.pb != nil {
99+
contentStore := packages_module.NewContentStore()
100+
if err := contentStore.Delete(packages_module.BlobHash256Key(txRet.pb.HashSHA256)); err != nil {
101+
log.Error("Error deleting package blob from content store: %v", err)
102+
}
95103
}
96-
defer committer.Close()
104+
}()
97105

106+
err = db.WithTx(ctx, func(ctx context.Context) (err error) {
98107
blobReferences := make([]*blobReference, 0, 1+len(manifest.Layers))
99108
blobReferences = append(blobReferences, &blobReference{
100109
Digest: manifest.Config.Digest,
@@ -127,7 +136,7 @@ func processOciImageManifest(ctx context.Context, mci *manifestCreationInfo, buf
127136
}
128137

129138
uploadVersion, err := packages_model.GetInternalVersionByNameAndVersion(ctx, mci.Owner.ID, packages_model.TypeContainer, mci.Image, container_module.UploadVersion)
130-
if err != nil && err != packages_model.ErrPackageNotExist {
139+
if err != nil && !errors.Is(err, packages_model.ErrPackageNotExist) {
131140
return err
132141
}
133142

@@ -136,61 +145,38 @@ func processOciImageManifest(ctx context.Context, mci *manifestCreationInfo, buf
136145
return err
137146
}
138147
}
139-
140-
pb, created, digest, err := createManifestBlob(ctx, mci, pv, buf)
141-
removeBlob := false
142-
defer func() {
143-
if removeBlob {
144-
contentStore := packages_module.NewContentStore()
145-
if err := contentStore.Delete(packages_module.BlobHash256Key(pb.HashSHA256)); err != nil {
146-
log.Error("Error deleting package blob from content store: %v", err)
147-
}
148-
}
149-
}()
150-
if err != nil {
151-
removeBlob = created
152-
return err
153-
}
154-
155-
if err := committer.Commit(); err != nil {
156-
removeBlob = created
157-
return err
158-
}
159-
160-
if err := notifyPackageCreate(ctx, mci.Creator, pv); err != nil {
161-
return err
162-
}
163-
164-
manifestDigest = digest
165-
166-
return nil
167-
}()
148+
txRet.pv = pv
149+
txRet.pb, txRet.created, txRet.digest, err = createManifestBlob(ctx, mci, pv, buf)
150+
return err
151+
})
168152
if err != nil {
169153
return "", err
170154
}
171155

172-
return manifestDigest, nil
156+
notifyPackageCreate(ctx, mci.Creator, txRet.pv)
157+
return txRet.digest, nil
173158
}
174159

175-
func processOciImageIndex(ctx context.Context, mci *manifestCreationInfo, buf *packages_module.HashedBuffer) (string, error) {
176-
manifestDigest := ""
177-
178-
err := func() error {
179-
var index oci.Index
180-
if err := json.NewDecoder(buf).Decode(&index); err != nil {
181-
return err
182-
}
183-
184-
if _, err := buf.Seek(0, io.SeekStart); err != nil {
185-
return err
186-
}
160+
func processOciImageIndex(ctx context.Context, mci *manifestCreationInfo, buf *packages_module.HashedBuffer) (manifestDigest string, errRet error) {
161+
var index oci.Index
162+
if err := json.NewDecoder(buf).Decode(&index); err != nil {
163+
return "", err
164+
}
165+
if _, err := buf.Seek(0, io.SeekStart); err != nil {
166+
return "", err
167+
}
187168

188-
ctx, committer, err := db.TxContext(ctx)
189-
if err != nil {
190-
return err
169+
var txRet processManifestTxRet
170+
defer func() {
171+
if errRet != nil && txRet.created && txRet.pb != nil {
172+
contentStore := packages_module.NewContentStore()
173+
if err := contentStore.Delete(packages_module.BlobHash256Key(txRet.pb.HashSHA256)); err != nil {
174+
log.Error("Error deleting package blob from content store: %v", err)
175+
}
191176
}
192-
defer committer.Close()
177+
}()
193178

179+
err := db.WithTx(ctx, func(ctx context.Context) (err error) {
194180
metadata := &container_module.Metadata{
195181
Type: container_module.TypeOCI,
196182
Manifests: make([]*container_module.Manifest, 0, len(index.Manifests)),
@@ -241,50 +227,25 @@ func processOciImageIndex(ctx context.Context, mci *manifestCreationInfo, buf *p
241227
return err
242228
}
243229

244-
pb, created, digest, err := createManifestBlob(ctx, mci, pv, buf)
245-
removeBlob := false
246-
defer func() {
247-
if removeBlob {
248-
contentStore := packages_module.NewContentStore()
249-
if err := contentStore.Delete(packages_module.BlobHash256Key(pb.HashSHA256)); err != nil {
250-
log.Error("Error deleting package blob from content store: %v", err)
251-
}
252-
}
253-
}()
254-
if err != nil {
255-
removeBlob = created
256-
return err
257-
}
258-
259-
if err := committer.Commit(); err != nil {
260-
removeBlob = created
261-
return err
262-
}
263-
264-
if err := notifyPackageCreate(ctx, mci.Creator, pv); err != nil {
265-
return err
266-
}
267-
268-
manifestDigest = digest
269-
270-
return nil
271-
}()
230+
txRet.pv = pv
231+
txRet.pb, txRet.created, txRet.digest, err = createManifestBlob(ctx, mci, pv, buf)
232+
return err
233+
})
272234
if err != nil {
273235
return "", err
274236
}
275237

276-
return manifestDigest, nil
238+
notifyPackageCreate(ctx, mci.Creator, txRet.pv)
239+
return txRet.digest, nil
277240
}
278241

279-
func notifyPackageCreate(ctx context.Context, doer *user_model.User, pv *packages_model.PackageVersion) error {
242+
func notifyPackageCreate(ctx context.Context, doer *user_model.User, pv *packages_model.PackageVersion) {
280243
pd, err := packages_model.GetPackageDescriptor(ctx, pv)
281244
if err != nil {
282-
return err
245+
log.Error("Error getting package descriptor: %v", err)
246+
return
283247
}
284-
285248
notify_service.PackageCreate(ctx, doer, pd)
286-
287-
return nil
288249
}
289250

290251
func createPackageAndVersion(ctx context.Context, mci *manifestCreationInfo, metadata *container_module.Metadata) (*packages_model.PackageVersion, error) {
@@ -437,7 +398,7 @@ func createFileFromBlobReference(ctx context.Context, pv, uploadVersion *package
437398
return pf, nil
438399
}
439400

440-
func createManifestBlob(ctx context.Context, mci *manifestCreationInfo, pv *packages_model.PackageVersion, buf *packages_module.HashedBuffer) (*packages_model.PackageBlob, bool, string, error) {
401+
func createManifestBlob(ctx context.Context, mci *manifestCreationInfo, pv *packages_model.PackageVersion, buf *packages_module.HashedBuffer) (_ *packages_model.PackageBlob, created bool, manifestDigest string, _ error) {
441402
pb, exists, err := packages_model.GetOrInsertBlob(ctx, packages_service.NewPackageBlob(buf))
442403
if err != nil {
443404
log.Error("Error inserting package blob: %v", err)
@@ -460,7 +421,7 @@ func createManifestBlob(ctx context.Context, mci *manifestCreationInfo, pv *pack
460421
}
461422
}
462423

463-
manifestDigest := digestFromHashSummer(buf)
424+
manifestDigest = digestFromHashSummer(buf)
464425
pf, err := createFileFromBlobReference(ctx, pv, nil, &blobReference{
465426
Digest: digest.Digest(manifestDigest),
466427
MediaType: mci.MediaType,

‎services/packages/container/common.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func ParseManifestMetadata(ctx context.Context, rd io.Reader, ownerID int64, ima
4848
configDescriptor, err := container_service.GetContainerBlob(ctx, &container_service.BlobSearchOptions{
4949
OwnerID: ownerID,
5050
Image: imageName,
51-
Digest: string(manifest.Config.Digest),
51+
Digest: manifest.Config.Digest.String(),
5252
})
5353
if err != nil {
5454
return nil, nil, nil, err

0 commit comments

Comments
 (0)