Skip to content

Commit 0690cb0

Browse files
authored
Fix missing signature key error when pulling Docker images with SERVE_DIRECT enabled (#32365)
Fix #28121 I did some tests and found that the `missing signature key` error is caused by an incorrect `Content-Type` header. Gitea correctly sets the `Content-Type` header when serving files. https://github.com/go-gitea/gitea/blob/348d1d0f322ca57c459acd902f54821d687ca804/routers/api/packages/container/container.go#L712-L717 However, when `SERVE_DIRECT` is enabled, the `Content-Type` header may be set to an incorrect value by the storage service. To fix this issue, we can use query parameters to override response header values. https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html <img width="600px" src="https://github.com/user-attachments/assets/f2ff90f0-f1df-46f9-9680-b8120222c555" /> In this PR, I introduced a new parameter to the `URL` method to support additional parameters. ``` URL(path, name string, reqParams url.Values) (*url.URL, error) ``` --- Most S3-like services support specifying the content type when storing objects. However, Gitea always use `application/octet-stream`. Therefore, I believe we also need to improve the `Save` method to support storing objects with the correct content type. https://github.com/go-gitea/gitea/blob/b7fb20e73e63b8edc9b90c52073e248bef428fcc/modules/storage/minio.go#L214-L221
1 parent 8107823 commit 0690cb0

File tree

19 files changed

+30
-24
lines changed

19 files changed

+30
-24
lines changed

Diff for: modules/packages/content_store.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ func (s *ContentStore) ShouldServeDirect() bool {
3737
return setting.Packages.Storage.ServeDirect()
3838
}
3939

40-
func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename string) (*url.URL, error) {
41-
return s.store.URL(KeyToRelativePath(key), filename)
40+
func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename string, reqParams url.Values) (*url.URL, error) {
41+
return s.store.URL(KeyToRelativePath(key), filename, reqParams)
4242
}
4343

4444
// FIXME: Workaround to be removed in v1.20

Diff for: modules/storage/azureblob.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ func (a *AzureBlobStorage) Delete(path string) error {
247247
}
248248

249249
// URL gets the redirect URL to a file. The presigned link is valid for 5 minutes.
250-
func (a *AzureBlobStorage) URL(path, name string) (*url.URL, error) {
250+
func (a *AzureBlobStorage) URL(path, name string, reqParams url.Values) (*url.URL, error) {
251251
blobClient := a.getBlobClient(path)
252252

253253
startTime := time.Now()

Diff for: modules/storage/helper.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func (s discardStorage) Delete(_ string) error {
3030
return fmt.Errorf("%s", s)
3131
}
3232

33-
func (s discardStorage) URL(_, _ string) (*url.URL, error) {
33+
func (s discardStorage) URL(_, _ string, _ url.Values) (*url.URL, error) {
3434
return nil, fmt.Errorf("%s", s)
3535
}
3636

Diff for: modules/storage/helper_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func Test_discardStorage(t *testing.T) {
3737
assert.Error(t, err, string(tt))
3838
}
3939
{
40-
got, err := tt.URL("path", "name")
40+
got, err := tt.URL("path", "name", nil)
4141
assert.Nil(t, got)
4242
assert.Errorf(t, err, string(tt))
4343
}

Diff for: modules/storage/local.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func (l *LocalStorage) Delete(path string) error {
114114
}
115115

116116
// URL gets the redirect URL to a file
117-
func (l *LocalStorage) URL(path, name string) (*url.URL, error) {
117+
func (l *LocalStorage) URL(path, name string, reqParams url.Values) (*url.URL, error) {
118118
return nil, ErrURLNotSupported
119119
}
120120

Diff for: modules/storage/minio.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,12 @@ func (m *MinioStorage) Delete(path string) error {
276276
}
277277

278278
// URL gets the redirect URL to a file. The presigned link is valid for 5 minutes.
279-
func (m *MinioStorage) URL(path, name string) (*url.URL, error) {
280-
reqParams := make(url.Values)
279+
func (m *MinioStorage) URL(path, name string, serveDirectReqParams url.Values) (*url.URL, error) {
280+
// copy serveDirectReqParams
281+
reqParams, err := url.ParseQuery(serveDirectReqParams.Encode())
282+
if err != nil {
283+
return nil, err
284+
}
281285
// TODO it may be good to embed images with 'inline' like ServeData does, but we don't want to have to read the file, do we?
282286
reqParams.Set("response-content-disposition", "attachment; filename=\""+quoteEscaper.Replace(name)+"\"")
283287
u, err := m.client.PresignedGetObject(m.ctx, m.bucket, m.buildMinioPath(path), 5*time.Minute, reqParams)

Diff for: modules/storage/storage.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ type ObjectStorage interface {
6363
Save(path string, r io.Reader, size int64) (int64, error)
6464
Stat(path string) (os.FileInfo, error)
6565
Delete(path string) error
66-
URL(path, name string) (*url.URL, error)
66+
URL(path, name string, reqParams url.Values) (*url.URL, error)
6767
IterateObjects(path string, iterator func(path string, obj Object) error) error
6868
}
6969

Diff for: routers/api/actions/artifacts.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) {
425425
for _, artifact := range artifacts {
426426
var downloadURL string
427427
if setting.Actions.ArtifactStorage.ServeDirect() {
428-
u, err := ar.fs.URL(artifact.StoragePath, artifact.ArtifactName)
428+
u, err := ar.fs.URL(artifact.StoragePath, artifact.ArtifactName, nil)
429429
if err != nil && !errors.Is(err, storage.ErrURLNotSupported) {
430430
log.Error("Error getting serve direct url: %v", err)
431431
}

Diff for: routers/api/actions/artifactsv4.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) {
517517
respData := GetSignedArtifactURLResponse{}
518518

519519
if setting.Actions.ArtifactStorage.ServeDirect() {
520-
u, err := storage.ActionsArtifacts.URL(artifact.StoragePath, artifact.ArtifactPath)
520+
u, err := storage.ActionsArtifacts.URL(artifact.StoragePath, artifact.ArtifactPath, nil)
521521
if u != nil && err == nil {
522522
respData.SignedUrl = u.String()
523523
}

Diff for: routers/api/packages/container/container.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,9 @@ func DeleteManifest(ctx *context.Context) {
703703
}
704704

705705
func serveBlob(ctx *context.Context, pfd *packages_model.PackageFileDescriptor) {
706-
s, u, _, err := packages_service.GetPackageBlobStream(ctx, pfd.File, pfd.Blob)
706+
serveDirectReqParams := make(url.Values)
707+
serveDirectReqParams.Set("response-content-type", pfd.Properties.GetByName(container_module.PropertyMediaType))
708+
s, u, _, err := packages_service.GetPackageBlobStream(ctx, pfd.File, pfd.Blob, serveDirectReqParams)
707709
if err != nil {
708710
apiError(ctx, http.StatusInternalServerError, err)
709711
return

Diff for: routers/api/packages/maven/maven.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ func servePackageFile(ctx *context.Context, params parameters, serveContent bool
215215
return
216216
}
217217

218-
s, u, _, err := packages_service.GetPackageBlobStream(ctx, pf, pb)
218+
s, u, _, err := packages_service.GetPackageBlobStream(ctx, pf, pb, nil)
219219
if err != nil {
220220
apiError(ctx, http.StatusInternalServerError, err)
221221
return

Diff for: routers/api/v1/repo/file.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) {
209209

210210
if setting.LFS.Storage.ServeDirect() {
211211
// If we have a signed url (S3, object storage), redirect to this directly.
212-
u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name())
212+
u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name(), nil)
213213
if u != nil && err == nil {
214214
ctx.Redirect(u.String())
215215
return
@@ -334,7 +334,7 @@ func download(ctx *context.APIContext, archiveName string, archiver *repo_model.
334334
rPath := archiver.RelativePath()
335335
if setting.RepoArchive.Storage.ServeDirect() {
336336
// If we have a signed url (S3, object storage), redirect to this directly.
337-
u, err := storage.RepoArchives.URL(rPath, downloadName)
337+
u, err := storage.RepoArchives.URL(rPath, downloadName, nil)
338338
if u != nil && err == nil {
339339
ctx.Redirect(u.String())
340340
return

Diff for: routers/web/base.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func storageHandler(storageSetting *setting.Storage, prefix string, objStore sto
3939
rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/")
4040
rPath = util.PathJoinRelX(rPath)
4141

42-
u, err := objStore.URL(rPath, path.Base(rPath))
42+
u, err := objStore.URL(rPath, path.Base(rPath), nil)
4343
if err != nil {
4444
if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) {
4545
log.Warn("Unable to find %s %s", prefix, rPath)

Diff for: routers/web/repo/actions/view.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,7 @@ func ArtifactsDownloadView(ctx *context_module.Context) {
663663
if len(artifacts) == 1 && artifacts[0].ArtifactName+".zip" == artifacts[0].ArtifactPath && artifacts[0].ContentEncoding == "application/zip" {
664664
art := artifacts[0]
665665
if setting.Actions.ArtifactStorage.ServeDirect() {
666-
u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath)
666+
u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, nil)
667667
if u != nil && err == nil {
668668
ctx.Redirect(u.String())
669669
return

Diff for: routers/web/repo/attachment.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func ServeAttachment(ctx *context.Context, uuid string) {
129129

130130
if setting.Attachment.Storage.ServeDirect() {
131131
// If we have a signed url (S3, object storage), redirect to this directly.
132-
u, err := storage.Attachments.URL(attach.RelativePath(), attach.Name)
132+
u, err := storage.Attachments.URL(attach.RelativePath(), attach.Name, nil)
133133

134134
if u != nil && err == nil {
135135
ctx.Redirect(u.String())

Diff for: routers/web/repo/download.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified *time.Tim
5555

5656
if setting.LFS.Storage.ServeDirect() {
5757
// If we have a signed url (S3, object storage, blob storage), redirect to this directly.
58-
u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name())
58+
u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name(), nil)
5959
if u != nil && err == nil {
6060
ctx.Redirect(u.String())
6161
return nil

Diff for: routers/web/repo/repo.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ func download(ctx *context.Context, archiveName string, archiver *repo_model.Rep
494494
rPath := archiver.RelativePath()
495495
if setting.RepoArchive.Storage.ServeDirect() {
496496
// If we have a signed url (S3, object storage), redirect to this directly.
497-
u, err := storage.RepoArchives.URL(rPath, downloadName)
497+
u, err := storage.RepoArchives.URL(rPath, downloadName, nil)
498498
if u != nil && err == nil {
499499
ctx.Redirect(u.String())
500500
return

Diff for: services/lfs/server.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa
460460
var link *lfs_module.Link
461461
if setting.LFS.Storage.ServeDirect() {
462462
// If we have a signed url (S3, object storage), redirect to this directly.
463-
u, err := storage.LFS.URL(pointer.RelativePath(), pointer.Oid)
463+
u, err := storage.LFS.URL(pointer.RelativePath(), pointer.Oid, nil)
464464
if u != nil && err == nil {
465465
// Presigned url does not need the Authorization header
466466
// https://github.com/go-gitea/gitea/issues/21525

Diff for: services/packages/packages.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -596,12 +596,12 @@ func GetPackageFileStream(ctx context.Context, pf *packages_model.PackageFile) (
596596
return nil, nil, nil, err
597597
}
598598

599-
return GetPackageBlobStream(ctx, pf, pb)
599+
return GetPackageBlobStream(ctx, pf, pb, nil)
600600
}
601601

602602
// GetPackageBlobStream returns the content of the specific package blob
603603
// If the storage supports direct serving and it's enabled, only the direct serving url is returned.
604-
func GetPackageBlobStream(ctx context.Context, pf *packages_model.PackageFile, pb *packages_model.PackageBlob) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) {
604+
func GetPackageBlobStream(ctx context.Context, pf *packages_model.PackageFile, pb *packages_model.PackageBlob, serveDirectReqParams url.Values) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) {
605605
key := packages_module.BlobHash256Key(pb.HashSHA256)
606606

607607
cs := packages_module.NewContentStore()
@@ -611,7 +611,7 @@ func GetPackageBlobStream(ctx context.Context, pf *packages_model.PackageFile, p
611611
var err error
612612

613613
if cs.ShouldServeDirect() {
614-
u, err = cs.GetServeDirectURL(key, pf.Name)
614+
u, err = cs.GetServeDirectURL(key, pf.Name, serveDirectReqParams)
615615
if err != nil && !errors.Is(err, storage.ErrURLNotSupported) {
616616
log.Error("Error getting serve direct url: %v", err)
617617
}

0 commit comments

Comments
 (0)