diff --git a/modules/actions/artifacts.go b/modules/actions/artifacts.go index d28726e8993..e8bf70ec310 100644 --- a/modules/actions/artifacts.go +++ b/modules/actions/artifacts.go @@ -20,7 +20,7 @@ func IsArtifactV4(art *actions_model.ActionArtifact) bool { func DownloadArtifactV4ServeDirectOnly(ctx *context.Base, art *actions_model.ActionArtifact) (bool, error) { if setting.Actions.ArtifactStorage.ServeDirect() { - u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, ctx.Req.Method, nil) + u, err := storage.ActionsArtifacts.ServeDirectURL(art.StoragePath, art.ArtifactPath, ctx.Req.Method, nil) if u != nil && err == nil { ctx.Redirect(u.String(), http.StatusFound) return true, nil diff --git a/modules/httplib/serve.go b/modules/httplib/serve.go index 2d66a86a8b0..fc7edc36c43 100644 --- a/modules/httplib/serve.go +++ b/modules/httplib/serve.go @@ -112,21 +112,20 @@ func setServeHeadersByFile(r *http.Request, w http.ResponseWriter, mineBuf []byt opts.ContentTypeCharset = strings.ToLower(charset) } - isSVG := sniffedType.IsSvgImage() - // serve types that can present a security risk with CSP - if isSVG { - w.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'; sandbox") - } else if sniffedType.IsPDF() { - // no sandbox attribute for pdf as it breaks rendering in at least safari. this + w.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'; sandbox") + + if sniffedType.IsPDF() { + // no sandbox attribute for PDF as it breaks rendering in at least safari. this // should generally be safe as scripts inside PDF can not escape the PDF document // see https://bugs.chromium.org/p/chromium/issues/detail?id=413851 for more discussion // HINT: PDF-RENDER-SANDBOX: PDF won't render in sandboxed context w.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'") } + // TODO: UNIFY-CONTENT-DISPOSITION-FROM-STORAGE opts.Disposition = "inline" - if isSVG && !setting.UI.SVG.Enabled { + if sniffedType.IsSvgImage() && !setting.UI.SVG.Enabled { opts.Disposition = "attachment" } diff --git a/modules/packages/content_store.go b/modules/packages/content_store.go index 57974515e2c..4c61233b5e7 100644 --- a/modules/packages/content_store.go +++ b/modules/packages/content_store.go @@ -36,8 +36,8 @@ func (s *ContentStore) ShouldServeDirect() bool { return setting.Packages.Storage.ServeDirect() } -func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename, method string, reqParams url.Values) (*url.URL, error) { - return s.store.URL(KeyToRelativePath(key), filename, method, reqParams) +func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename, method string, reqParams *storage.ServeDirectOptions) (*url.URL, error) { + return s.store.ServeDirectURL(KeyToRelativePath(key), filename, method, reqParams) } // FIXME: Workaround to be removed in v1.20 diff --git a/modules/public/mime_types.go b/modules/public/mime_types.go index 32bdf3bfa25..fef85d77cbe 100644 --- a/modules/public/mime_types.go +++ b/modules/public/mime_types.go @@ -3,9 +3,11 @@ package public -import "strings" +import ( + "strings" +) -// wellKnownMimeTypesLower comes from Golang's builtin mime package: `builtinTypesLower`, see the comment of detectWellKnownMimeType +// wellKnownMimeTypesLower comes from Golang's builtin mime package: `builtinTypesLower`, see the comment of DetectWellKnownMimeType var wellKnownMimeTypesLower = map[string]string{ ".avif": "image/avif", ".css": "text/css; charset=utf-8", @@ -28,13 +30,13 @@ var wellKnownMimeTypesLower = map[string]string{ ".txt": "text/plain; charset=utf-8", } -// detectWellKnownMimeType will return the mime-type for a well-known file ext name +// DetectWellKnownMimeType will return the mime-type for a well-known file ext name // The purpose of this function is to bypass the unstable behavior of Golang's mime.TypeByExtension // mime.TypeByExtension would use OS's mime-type config to overwrite the well-known types (see its document). // If the user's OS has incorrect mime-type config, it would make Gitea can not respond a correct Content-Type to browsers. // For example, if Gitea returns `text/plain` for a `.js` file, the browser couldn't run the JS due to security reasons. -// detectWellKnownMimeType makes the Content-Type for well-known files stable. -func detectWellKnownMimeType(ext string) string { +// DetectWellKnownMimeType makes the Content-Type for well-known files stable. +func DetectWellKnownMimeType(ext string) string { ext = strings.ToLower(ext) return wellKnownMimeTypesLower[ext] } diff --git a/modules/public/public.go b/modules/public/public.go index 3a5a76637e7..004aad5f3b1 100644 --- a/modules/public/public.go +++ b/modules/public/public.go @@ -51,9 +51,9 @@ func parseAcceptEncoding(val string) container.Set[string] { } // setWellKnownContentType will set the Content-Type if the file is a well-known type. -// See the comments of detectWellKnownMimeType +// See the comments of DetectWellKnownMimeType func setWellKnownContentType(w http.ResponseWriter, file string) { - mimeType := detectWellKnownMimeType(path.Ext(file)) + mimeType := DetectWellKnownMimeType(path.Ext(file)) if mimeType != "" { w.Header().Set("Content-Type", mimeType) } diff --git a/modules/storage/azureblob.go b/modules/storage/azureblob.go index e7297cec77a..a273a7770d0 100644 --- a/modules/storage/azureblob.go +++ b/modules/storage/azureblob.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "net/http" "net/url" "os" "path" @@ -246,16 +247,53 @@ func (a *AzureBlobStorage) Delete(path string) error { return convertAzureBlobErr(err) } -// URL gets the redirect URL to a file. The presigned link is valid for 5 minutes. -func (a *AzureBlobStorage) URL(path, name, _ string, reqParams url.Values) (*url.URL, error) { - blobClient := a.getBlobClient(path) +func (a *AzureBlobStorage) getSasURL(b *blob.Client, template sas.BlobSignatureValues) (string, error) { + urlParts, err := blob.ParseURL(b.URL()) + if err != nil { + return "", err + } - // TODO: OBJECT-STORAGE-CONTENT-TYPE: "browser inline rendering images/PDF" needs proper Content-Type header from storage - startTime := time.Now() - u, err := blobClient.GetSASURL(sas.BlobPermissions{ - Read: true, - }, time.Now().Add(5*time.Minute), &blob.GetSASURLOptions{ - StartTime: &startTime, + var t time.Time + if urlParts.Snapshot == "" { + t = time.Time{} + } else { + t, err = time.Parse(blob.SnapshotTimeFormat, urlParts.Snapshot) + if err != nil { + return "", err + } + } + + template.ContainerName = urlParts.ContainerName + template.BlobName = urlParts.BlobName + template.SnapshotTime = t + template.Version = sas.Version + + qps, err := template.SignWithSharedKey(a.credential) + if err != nil { + return "", err + } + + endpoint := b.URL() + "?" + qps.Encode() + + return endpoint, nil +} + +func (a *AzureBlobStorage) ServeDirectURL(storePath, name, method string, reqParams *ServeDirectOptions) (*url.URL, error) { + blobClient := a.getBlobClient(storePath) + + startTime := time.Now().UTC() + + param := prepareServeDirectOptions(reqParams, name) + + u, err := a.getSasURL(blobClient, sas.BlobSignatureValues{ + Permissions: (&sas.BlobPermissions{ + Read: method == http.MethodGet || method == http.MethodHead, + Write: method == http.MethodPut, + }).String(), + StartTime: startTime, + ExpiryTime: startTime.Add(5 * time.Minute), + ContentDisposition: param.ContentDisposition, + ContentType: param.ContentType, }) if err != nil { return nil, convertAzureBlobErr(err) diff --git a/modules/storage/azureblob_test.go b/modules/storage/azureblob_test.go index b3791b49164..b5d5d0fecc0 100644 --- a/modules/storage/azureblob_test.go +++ b/modules/storage/azureblob_test.go @@ -14,12 +14,13 @@ import ( "github.com/stretchr/testify/assert" ) -func TestAzureBlobStorageIterator(t *testing.T) { +func TestAzureBlobStorage(t *testing.T) { if os.Getenv("CI") == "" { t.Skip("azureBlobStorage not present outside of CI") return } - testStorageIterator(t, setting.AzureBlobStorageType, &setting.Storage{ + storageType := setting.AzureBlobStorageType + config := &setting.Storage{ AzureBlobConfig: setting.AzureBlobStorageConfig{ // https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#ip-style-url Endpoint: "http://devstoreaccount1.azurite.local:10000", @@ -28,7 +29,25 @@ func TestAzureBlobStorageIterator(t *testing.T) { AccountKey: "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==", Container: "test", }, - }) + } + table := []struct { + name string + test func(t *testing.T, typStr Type, cfg *setting.Storage) + }{ + { + name: "iterator", + test: testStorageIterator, + }, + { + name: "testBlobStorageURLContentTypeAndDisposition", + test: testBlobStorageURLContentTypeAndDisposition, + }, + } + for _, entry := range table { + t.Run(entry.name, func(t *testing.T) { + entry.test(t, storageType, config) + }) + } } func TestAzureBlobStoragePath(t *testing.T) { diff --git a/modules/storage/helper.go b/modules/storage/helper.go index f6c3d5eebbc..30d96a33add 100644 --- a/modules/storage/helper.go +++ b/modules/storage/helper.go @@ -30,7 +30,7 @@ func (s discardStorage) Delete(_ string) error { return fmt.Errorf("%s", s) } -func (s discardStorage) URL(_, _, _ string, _ url.Values) (*url.URL, error) { +func (s discardStorage) ServeDirectURL(_, _, _ string, _ *ServeDirectOptions) (*url.URL, error) { return nil, fmt.Errorf("%s", s) } diff --git a/modules/storage/helper_test.go b/modules/storage/helper_test.go index 3cba1e13c01..ae64c8aca49 100644 --- a/modules/storage/helper_test.go +++ b/modules/storage/helper_test.go @@ -37,7 +37,7 @@ func Test_discardStorage(t *testing.T) { assert.Error(t, err, string(tt)) } { - got, err := tt.URL("path", "name", "GET", nil) + got, err := tt.ServeDirectURL("path", "name", "GET", nil) assert.Nil(t, got) assert.Errorf(t, err, string(tt)) } diff --git a/modules/storage/local.go b/modules/storage/local.go index 5ea6f055ce7..04e3e05f950 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -133,8 +133,7 @@ func (l *LocalStorage) Delete(path string) error { return err } -// URL gets the redirect URL to a file -func (l *LocalStorage) URL(path, name, _ string, reqParams url.Values) (*url.URL, error) { +func (l *LocalStorage) ServeDirectURL(path, name, _ string, reqParams *ServeDirectOptions) (*url.URL, error) { return nil, ErrURLNotSupported } diff --git a/modules/storage/minio.go b/modules/storage/minio.go index 6993ac2d922..1355280f367 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -278,37 +278,16 @@ func (m *MinioStorage) Delete(path string) error { return convertMinioErr(err) } -// URL gets the redirect URL to a file. The presigned link is valid for 5 minutes. -func (m *MinioStorage) URL(storePath, name, method string, serveDirectReqParams url.Values) (*url.URL, error) { - // copy serveDirectReqParams - reqParams, err := url.ParseQuery(serveDirectReqParams.Encode()) - if err != nil { - return nil, err - } +func (m *MinioStorage) ServeDirectURL(storePath, name, method string, opt *ServeDirectOptions) (*url.URL, error) { + reqParams := url.Values{} - // Here we might not know the real filename, and it's quite inefficient to detect the mine type by pre-fetching the object head. - // So we just do a quick detection by extension name, at least if works for the "View Raw File" for an LFS file on the Web UI. - // Detect content type by extension name, only support the well-known safe types for inline rendering. - // TODO: OBJECT-STORAGE-CONTENT-TYPE: need a complete solution and refactor for Azure in the future - ext := path.Ext(name) - inlineExtMimeTypes := map[string]string{ - ".png": "image/png", - ".jpg": "image/jpeg", - ".jpeg": "image/jpeg", - ".gif": "image/gif", - ".webp": "image/webp", - ".avif": "image/avif", - // ATTENTION! Don't support unsafe types like HTML/SVG due to security concerns: they can contain JS code, and maybe they need proper Content-Security-Policy - // HINT: PDF-RENDER-SANDBOX: PDF won't render in sandboxed context, it seems fine to render it inline - ".pdf": "application/pdf", - - // TODO: refactor with "modules/public/mime_types.go", for example: "DetectWellKnownSafeInlineMimeType" + param := prepareServeDirectOptions(opt, name) + // minio does not ignore empty params + if param.ContentType != "" { + reqParams.Set("response-content-type", param.ContentType) } - if mimeType, ok := inlineExtMimeTypes[ext]; ok { - reqParams.Set("response-content-type", mimeType) - reqParams.Set("response-content-disposition", "inline") - } else { - reqParams.Set("response-content-disposition", fmt.Sprintf(`attachment; filename="%s"`, quoteEscaper.Replace(name))) + if param.ContentDisposition != "" { + reqParams.Set("response-content-disposition", param.ContentDisposition) } expires := 5 * time.Minute @@ -323,6 +302,7 @@ func (m *MinioStorage) URL(storePath, name, method string, serveDirectReqParams // IterateObjects iterates across the objects in the miniostorage func (m *MinioStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error { opts := minio.GetObjectOptions{} + // FIXME: this loop is not right and causes resource leaking, see the comment of ListObjects for mObjInfo := range m.client.ListObjects(m.ctx, m.bucket, minio.ListObjectsOptions{ Prefix: m.buildMinioDirPrefix(dirName), Recursive: true, diff --git a/modules/storage/minio_test.go b/modules/storage/minio_test.go index 2726d765ddf..5c15ee1ed6d 100644 --- a/modules/storage/minio_test.go +++ b/modules/storage/minio_test.go @@ -16,12 +16,13 @@ import ( "github.com/stretchr/testify/assert" ) -func TestMinioStorageIterator(t *testing.T) { +func TestMinioStorage(t *testing.T) { if os.Getenv("CI") == "" { t.Skip("minioStorage not present outside of CI") return } - testStorageIterator(t, setting.MinioStorageType, &setting.Storage{ + storageType := setting.MinioStorageType + config := &setting.Storage{ MinioConfig: setting.MinioStorageConfig{ Endpoint: "minio:9000", AccessKeyID: "123456", @@ -29,7 +30,25 @@ func TestMinioStorageIterator(t *testing.T) { Bucket: "gitea", Location: "us-east-1", }, - }) + } + table := []struct { + name string + test func(t *testing.T, typStr Type, cfg *setting.Storage) + }{ + { + name: "iterator", + test: testStorageIterator, + }, + { + name: "testBlobStorageURLContentTypeAndDisposition", + test: testBlobStorageURLContentTypeAndDisposition, + }, + } + for _, entry := range table { + t.Run(entry.name, func(t *testing.T) { + entry.test(t, storageType, config) + }) + } } func TestMinioStoragePath(t *testing.T) { diff --git a/modules/storage/storage.go b/modules/storage/storage.go index 74d0cd47c87..2491c77a3e0 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -10,8 +10,10 @@ import ( "io" "net/url" "os" + "path" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/public" "code.gitea.io/gitea/modules/setting" ) @@ -56,6 +58,38 @@ type Object interface { Stat() (os.FileInfo, error) } +// ServeDirectOptions customizes HTTP headers for a generated signed URL. +type ServeDirectOptions struct { + // Overrides the automatically detected MIME type. + ContentType string + // Overrides the default Content-Disposition header, which is `inline; filename="name"`. + ContentDisposition string +} + +// Safe defaults are applied only when not explicitly overridden by the caller. +func prepareServeDirectOptions(optsOptional *ServeDirectOptions, name string) (ret ServeDirectOptions) { + // Here we might not know the real filename, and it's quite inefficient to detect the MIME type by pre-fetching the object head. + // So we just do a quick detection by extension name, at least it works for the "View Raw File" for an LFS file on the Web UI. + // TODO: OBJECT-STORAGE-CONTENT-TYPE: need a complete solution and refactor for Azure in the future + + if optsOptional != nil { + ret = *optsOptional + } + + // TODO: UNIFY-CONTENT-DISPOSITION-FROM-STORAGE + if ret.ContentType == "" { + ext := path.Ext(name) + ret.ContentType = public.DetectWellKnownMimeType(ext) + } + if ret.ContentDisposition == "" { + // When using ServeDirect, the URL is from the object storage's web server, + // it is not the same origin as Gitea server, so it should be safe enough to use "inline" to render the content directly. + // If a browser doesn't support the content type to be displayed inline, browser will download with the filename. + ret.ContentDisposition = fmt.Sprintf(`inline; filename="%s"`, quoteEscaper.Replace(name)) + } + return ret +} + // ObjectStorage represents an object storage to handle a bucket and files type ObjectStorage interface { Open(path string) (Object, error) @@ -67,7 +101,15 @@ type ObjectStorage interface { Stat(path string) (os.FileInfo, error) Delete(path string) error - URL(path, name, method string, reqParams url.Values) (*url.URL, error) + + // ServeDirectURL generates a "serve-direct" URL for the specified blob storage file, + // end user (browser) will use this URL to access the file directly from the object storage, bypassing Gitea server. + // Usually the link is time-limited (a few minutes) and contains a signature to ensure security. + // The generated URL must NOT use the same origin as Gitea server, otherwise it will cause security issues. + // * method defines which HTTP method is permitted for certain storage providers (e.g., MinIO). + // * opt allows customizing the Content-Type and Content-Disposition headers. + // TODO: need to merge "ServeDirect()" check into this function, avoid duplicate code and potential inconsistency. + ServeDirectURL(path, name, method string, opt *ServeDirectOptions) (*url.URL, error) // IterateObjects calls the iterator function for each object in the storage with the given path as prefix // The "fullPath" argument in callback is the full path in this storage. @@ -136,7 +178,7 @@ var ( // Actions represents actions storage Actions ObjectStorage = uninitializedStorage - // Actions Artifacts represents actions artifacts storage + // ActionsArtifacts Artifacts represents actions artifacts storage ActionsArtifacts ObjectStorage = uninitializedStorage ) diff --git a/modules/storage/storage_test.go b/modules/storage/storage_test.go index 08f274e74b4..4156723c364 100644 --- a/modules/storage/storage_test.go +++ b/modules/storage/storage_test.go @@ -4,12 +4,14 @@ package storage import ( + "net/http" "strings" "testing" "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) { @@ -50,3 +52,55 @@ func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) { assert.Len(t, expected, count) } } + +func testSingleBlobStorageURLContentTypeAndDisposition(t *testing.T, s ObjectStorage, path, name string, expected ServeDirectOptions, reqParams *ServeDirectOptions) { + u, err := s.ServeDirectURL(path, name, http.MethodGet, reqParams) + require.NoError(t, err) + resp, err := http.Get(u.String()) + require.NoError(t, err) + defer resp.Body.Close() + if expected.ContentType != "" { + assert.Equal(t, expected.ContentType, resp.Header.Get("Content-Type")) + } + if expected.ContentDisposition != "" { + assert.Equal(t, expected.ContentDisposition, resp.Header.Get("Content-Disposition")) + } +} + +func testBlobStorageURLContentTypeAndDisposition(t *testing.T, typStr Type, cfg *setting.Storage) { + s, err := NewStorage(typStr, cfg) + assert.NoError(t, err) + + data := "Q2xTckt6Y1hDOWh0" // arbitrary test content; specific value is irrelevant to this test + testfilename := "test.txt" // arbitrary file name; specific value is irrelevant to this test + _, err = s.Save(testfilename, strings.NewReader(data), int64(len(data))) + assert.NoError(t, err) + + testSingleBlobStorageURLContentTypeAndDisposition(t, s, testfilename, "test.txt", ServeDirectOptions{ + ContentType: "text/plain; charset=utf-8", + ContentDisposition: `inline; filename="test.txt"`, + }, nil) + + testSingleBlobStorageURLContentTypeAndDisposition(t, s, testfilename, "test.pdf", ServeDirectOptions{ + ContentType: "application/pdf", + ContentDisposition: `inline; filename="test.pdf"`, + }, nil) + + testSingleBlobStorageURLContentTypeAndDisposition(t, s, testfilename, "test.wasm", ServeDirectOptions{ + ContentDisposition: `inline; filename="test.wasm"`, + }, nil) + + testSingleBlobStorageURLContentTypeAndDisposition(t, s, testfilename, "test.wasm", ServeDirectOptions{ + ContentDisposition: `inline; filename="test.wasm"`, + }, &ServeDirectOptions{}) + + testSingleBlobStorageURLContentTypeAndDisposition(t, s, testfilename, "test.txt", ServeDirectOptions{ + ContentType: "application/octet-stream", + ContentDisposition: `inline; filename="test.xml"`, + }, &ServeDirectOptions{ + ContentType: "application/octet-stream", + ContentDisposition: `inline; filename="test.xml"`, + }) + + assert.NoError(t, s.Delete(testfilename)) +} diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index d7e7203a857..76facd769f2 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -428,7 +428,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) { for _, artifact := range artifacts { var downloadURL string if setting.Actions.ArtifactStorage.ServeDirect() { - u, err := ar.fs.URL(artifact.StoragePath, artifact.ArtifactName, ctx.Req.Method, nil) + u, err := ar.fs.ServeDirectURL(artifact.StoragePath, artifact.ArtifactName, ctx.Req.Method, nil) if err != nil && !errors.Is(err, storage.ErrURLNotSupported) { log.Error("Error getting serve direct url: %v", err) } diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index d208785f638..62605f27022 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -562,7 +562,8 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) { respData := GetSignedArtifactURLResponse{} if setting.Actions.ArtifactStorage.ServeDirect() { - u, err := storage.ActionsArtifacts.URL(artifact.StoragePath, artifact.ArtifactPath, ctx.Req.Method, nil) + // DO NOT USE the http POST method coming from the getSignedArtifactURL endpoint + u, err := storage.ActionsArtifacts.ServeDirectURL(artifact.StoragePath, artifact.ArtifactPath, http.MethodGet, nil) if u != nil && err == nil { respData.SignedUrl = u.String() } diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go index a6512181e06..0726302d41f 100644 --- a/routers/api/packages/container/container.go +++ b/routers/api/packages/container/container.go @@ -26,6 +26,7 @@ import ( packages_module "code.gitea.io/gitea/modules/packages" container_module "code.gitea.io/gitea/modules/packages/container" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/routers/api/packages/helper" auth_service "code.gitea.io/gitea/services/auth" @@ -706,9 +707,9 @@ func DeleteManifest(ctx *context.Context) { } func serveBlob(ctx *context.Context, pfd *packages_model.PackageFileDescriptor) { - serveDirectReqParams := make(url.Values) - serveDirectReqParams.Set("response-content-type", pfd.Properties.GetByName(container_module.PropertyMediaType)) - s, u, _, err := packages_service.OpenBlobForDownload(ctx, pfd.File, pfd.Blob, ctx.Req.Method, serveDirectReqParams) + s, u, _, err := packages_service.OpenBlobForDownload(ctx, pfd.File, pfd.Blob, ctx.Req.Method, &storage.ServeDirectOptions{ + ContentType: pfd.Properties.GetByName(container_module.PropertyMediaType), + }) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index e31bc24eef6..d0596d778b7 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -207,7 +207,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) { if setting.LFS.Storage.ServeDirect() { // If we have a signed url (S3, object storage), redirect to this directly. - u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name(), ctx.Req.Method, nil) + u, err := storage.LFS.ServeDirectURL(pointer.RelativePath(), blob.Name(), ctx.Req.Method, nil) if u != nil && err == nil { ctx.Redirect(u.String()) return diff --git a/routers/web/base.go b/routers/web/base.go index db96432bedc..6afcac62e23 100644 --- a/routers/web/base.go +++ b/routers/web/base.go @@ -69,7 +69,7 @@ func avatarStorageHandler(storageSetting *setting.Storage, prefix string, objSto // So in theory, it doesn't work with the non-existing avatar fallback, it just gets the URL and redirects to it. // Checking "stat" requires one more request to the storage, which is inefficient. // Workaround: disable "SERVE_DIRECT". Leave the problem to the future. - u, err := objStore.URL(avatarPath, path.Base(avatarPath), req.Method, nil) + u, err := objStore.ServeDirectURL(avatarPath, path.Base(avatarPath), req.Method, nil) if handleError(w, req, avatarPath, err) { return } diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index be711afe6dd..19d533f3624 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -179,7 +179,7 @@ func ServeAttachment(ctx *context.Context, uuid string) { if setting.Attachment.Storage.ServeDirect() { // If we have a signed url (S3, object storage), redirect to this directly. - u, err := storage.Attachments.URL(attach.RelativePath(), attach.Name, ctx.Req.Method, nil) + u, err := storage.Attachments.ServeDirectURL(attach.RelativePath(), attach.Name, ctx.Req.Method, nil) if u != nil && err == nil { ctx.Redirect(u.String()) diff --git a/routers/web/repo/download.go b/routers/web/repo/download.go index 9412d2045d4..073d3d74208 100644 --- a/routers/web/repo/download.go +++ b/routers/web/repo/download.go @@ -54,7 +54,7 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified *time.Tim if setting.LFS.Storage.ServeDirect() { // If we have a signed url (S3, object storage, blob storage), redirect to this directly. - u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name(), ctx.Req.Method, nil) + u, err := storage.LFS.ServeDirectURL(pointer.RelativePath(), blob.Name(), ctx.Req.Method, nil) if u != nil && err == nil { ctx.Redirect(u.String()) return nil diff --git a/services/lfs/server.go b/services/lfs/server.go index 10b4dba222c..fc09eb58ca4 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -42,7 +42,6 @@ type requestContext struct { User string Repo string Authorization string - Method string RepoGitURL string } @@ -427,7 +426,6 @@ func getRequestContext(ctx *context.Context) *requestContext { User: ownerName, Repo: repoName, Authorization: ctx.Req.Header.Get("Authorization"), - Method: ctx.Req.Method, RepoGitURL: httplib.GuessCurrentAppURL(ctx) + url.PathEscape(ownerName) + "/" + url.PathEscape(repoName+".git"), } } @@ -490,7 +488,8 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa var link *lfs_module.Link if setting.LFS.Storage.ServeDirect() { // If we have a signed url (S3, object storage), redirect to this directly. - u, err := storage.LFS.URL(pointer.RelativePath(), pointer.Oid, rc.Method, nil) + // DO NOT USE the http POST method coming from the lfs batch endpoint + u, err := storage.LFS.ServeDirectURL(pointer.RelativePath(), pointer.Oid, http.MethodGet, nil) if u != nil && err == nil { link = lfs_module.NewLink(u.String()) // Presigned url does not need the Authorization header } diff --git a/services/packages/packages.go b/services/packages/packages.go index 22b26b65637..3b4e11e0410 100644 --- a/services/packages/packages.go +++ b/services/packages/packages.go @@ -599,7 +599,7 @@ func OpenBlobStream(pb *packages_model.PackageBlob) (io.ReadSeekCloser, error) { // OpenBlobForDownload returns the content of the specific package blob and increases the download counter. // If the storage supports direct serving and it's enabled, only the direct serving url is returned. -func OpenBlobForDownload(ctx context.Context, pf *packages_model.PackageFile, pb *packages_model.PackageBlob, method string, serveDirectReqParams url.Values) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { +func OpenBlobForDownload(ctx context.Context, pf *packages_model.PackageFile, pb *packages_model.PackageBlob, method string, serveDirectReqParams *storage.ServeDirectOptions) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { key := packages_module.BlobHash256Key(pb.HashSHA256) cs := packages_module.NewContentStore() diff --git a/services/repository/archiver/archiver.go b/services/repository/archiver/archiver.go index 07214d0bfa9..1d28e00655c 100644 --- a/services/repository/archiver/archiver.go +++ b/services/repository/archiver/archiver.go @@ -346,7 +346,7 @@ func ServeRepoArchive(ctx *gitea_context.Base, archiveReq *ArchiveRequest) error rPath := archiver.RelativePath() if setting.RepoArchive.Storage.ServeDirect() { // If we have a signed url (S3, object storage), redirect to this directly. - u, err := storage.RepoArchives.URL(rPath, downloadName, ctx.Req.Method, nil) + u, err := storage.RepoArchives.ServeDirectURL(rPath, downloadName, ctx.Req.Method, nil) if u != nil && err == nil { ctx.Redirect(u.String()) return nil