Skip to content

Commit 9d55c37

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 376bf01 commit 9d55c37

File tree

6 files changed

+87
-123
lines changed

6 files changed

+87
-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: 15 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,13 @@ 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+
ret.imageNamePattern = regexp.MustCompile(`\A[a-z0-9]+([._-][a-z0-9]+)*(/[a-z0-9]+([._-][a-z0-9]+)*)*\z`)
47+
ret.referencePattern = regexp.MustCompile(`\A[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}\z`)
48+
return ret
49+
})
4650

4751
type containerHeaders struct {
4852
Status int
@@ -84,9 +88,7 @@ func jsonResponse(ctx *context.Context, status int, obj any) {
8488
Status: status,
8589
ContentType: "application/json",
8690
})
87-
if err := json.NewEncoder(ctx.Resp).Encode(obj); err != nil {
88-
log.Error("JSON encode: %v", err)
89-
}
91+
_ = json.NewEncoder(ctx.Resp).Encode(obj) // ignore network errors
9092
}
9193

9294
func apiError(ctx *context.Context, status int, err error) {
@@ -134,7 +136,7 @@ func ReqContainerAccess(ctx *context.Context) {
134136

135137
// VerifyImageName is a middleware which checks if the image name is allowed
136138
func VerifyImageName(ctx *context.Context) {
137-
if !imageNamePattern.MatchString(ctx.PathParam("image")) {
139+
if !globalVars().imageNamePattern.MatchString(ctx.PathParam("image")) {
138140
apiErrorDefined(ctx, errNameInvalid)
139141
}
140142
}
@@ -216,7 +218,7 @@ func GetRepositoryList(ctx *context.Context) {
216218
if len(repositories) == n {
217219
v := url.Values{}
218220
if n > 0 {
219-
v.Add("n", strconv.Itoa(n))
221+
v.Add("n", strconv.Itoa(n)) // FIXME: "n" can't be zero here, the logic is inconsistent with GetTagsList
220222
}
221223
v.Add("last", repositories[len(repositories)-1])
222224

@@ -565,7 +567,7 @@ func PutManifest(ctx *context.Context) {
565567
IsTagged: digest.Digest(reference).Validate() != nil,
566568
}
567569

568-
if mci.IsTagged && !referencePattern.MatchString(reference) {
570+
if mci.IsTagged && !globalVars().referencePattern.MatchString(reference) {
569571
apiErrorDefined(ctx, errManifestInvalid.WithMessage("Tag is invalid"))
570572
return
571573
}
@@ -618,7 +620,7 @@ func getBlobSearchOptionsFromContext(ctx *context.Context) (*container_model.Blo
618620
reference := ctx.PathParam("reference")
619621
if d := digest.Digest(reference); d.Validate() == nil {
620622
opts.Digest = string(d)
621-
} else if referencePattern.MatchString(reference) {
623+
} else if globalVars().referencePattern.MatchString(reference) {
622624
opts.Tag = reference
623625
opts.OnlyLead = true
624626
} else {
@@ -782,7 +784,8 @@ func GetTagsList(ctx *context.Context) {
782784
})
783785
}
784786

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

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

Lines changed: 56 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,
@@ -120,14 +129,13 @@ func processOciImageManifest(ctx context.Context, mci *manifestCreationInfo, buf
120129
ExpectedSize: layer.Size,
121130
})
122131
}
123-
124132
pv, err := createPackageAndVersion(ctx, mci, metadata)
125133
if err != nil {
126134
return err
127135
}
128136

129137
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 {
138+
if err != nil && !errors.Is(err, packages_model.ErrPackageNotExist) {
131139
return err
132140
}
133141

@@ -136,61 +144,38 @@ func processOciImageManifest(ctx context.Context, mci *manifestCreationInfo, buf
136144
return err
137145
}
138146
}
147+
txRet.pb, txRet.created, txRet.digest, err = createManifestBlob(ctx, mci, pv, buf)
148+
return err
149+
})
139150

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-
}()
168151
if err != nil {
169152
return "", err
170153
}
171154

172-
return manifestDigest, nil
155+
notifyPackageCreate(ctx, mci.Creator, txRet.pv)
156+
return txRet.digest, nil
173157
}
174158

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-
}
159+
func processOciImageIndex(ctx context.Context, mci *manifestCreationInfo, buf *packages_module.HashedBuffer) (manifestDigest string, errRet error) {
160+
var index oci.Index
161+
if err := json.NewDecoder(buf).Decode(&index); err != nil {
162+
return "", err
163+
}
164+
if _, err := buf.Seek(0, io.SeekStart); err != nil {
165+
return "", err
166+
}
187167

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

178+
err := db.WithTx(ctx, func(ctx context.Context) (err error) {
194179
metadata := &container_module.Metadata{
195180
Type: container_module.TypeOCI,
196181
Manifests: make([]*container_module.Manifest, 0, len(index.Manifests)),
@@ -241,50 +226,24 @@ func processOciImageIndex(ctx context.Context, mci *manifestCreationInfo, buf *p
241226
return err
242227
}
243228

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-
}()
229+
txRet.pb, txRet.created, txRet.digest, err = createManifestBlob(ctx, mci, pv, buf)
230+
return err
231+
})
272232
if err != nil {
273233
return "", err
274234
}
275235

276-
return manifestDigest, nil
236+
notifyPackageCreate(ctx, mci.Creator, txRet.pv)
237+
return txRet.digest, nil
277238
}
278239

279-
func notifyPackageCreate(ctx context.Context, doer *user_model.User, pv *packages_model.PackageVersion) error {
240+
func notifyPackageCreate(ctx context.Context, doer *user_model.User, pv *packages_model.PackageVersion) {
280241
pd, err := packages_model.GetPackageDescriptor(ctx, pv)
281242
if err != nil {
282-
return err
243+
log.Error("Error getting package descriptor: %v", err)
244+
return
283245
}
284-
285246
notify_service.PackageCreate(ctx, doer, pd)
286-
287-
return nil
288247
}
289248

290249
func createPackageAndVersion(ctx context.Context, mci *manifestCreationInfo, metadata *container_module.Metadata) (*packages_model.PackageVersion, error) {
@@ -437,7 +396,7 @@ func createFileFromBlobReference(ctx context.Context, pv, uploadVersion *package
437396
return pf, nil
438397
}
439398

440-
func createManifestBlob(ctx context.Context, mci *manifestCreationInfo, pv *packages_model.PackageVersion, buf *packages_module.HashedBuffer) (*packages_model.PackageBlob, bool, string, error) {
399+
func createManifestBlob(ctx context.Context, mci *manifestCreationInfo, pv *packages_model.PackageVersion, buf *packages_module.HashedBuffer) (_ *packages_model.PackageBlob, created bool, manifestDigest string, _ error) {
441400
pb, exists, err := packages_model.GetOrInsertBlob(ctx, packages_service.NewPackageBlob(buf))
442401
if err != nil {
443402
log.Error("Error inserting package blob: %v", err)
@@ -460,7 +419,7 @@ func createManifestBlob(ctx context.Context, mci *manifestCreationInfo, pv *pack
460419
}
461420
}
462421

463-
manifestDigest := digestFromHashSummer(buf)
422+
manifestDigest = digestFromHashSummer(buf)
464423
pf, err := createFileFromBlobReference(ctx, pv, nil, &blobReference{
465424
Digest: digest.Digest(manifestDigest),
466425
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)