fix(storage): fix Azure Blob dump failing with file does not exist (#38814)

## Issue

Gitea fails to dump LFS (and other object-storage) files when Azure Blob
Storage is configured as the storage backend. The dump reports:

Failed to dump LFS objects: /file/path: copying contents: file does not
exist

This happens with any non-empty base path (the default for LFS storage),
which is why the user could only work around it by using `--skip-*`
flags.

The root cause is in `AzureBlobStorage.IterateObjects()`: Azure's list
API already returns each blob's name including the configured base path,
but the code was building the read client by running that name through
the base-path-prepending helper a second time. This doubled the base
path (e.g. `gitea-lfs/gitea-lfs/aa/bb/hash`), pointing at a blob that
doesn't exist. `Stat()` still succeeded because it doesn't touch the
network, so the failure only surfaced when the dumper actually tried to
read the object's contents.

## Solution

Add `getBlobClientByFullName()`, which builds a blob client from a name
that is already fully qualified, without re-applying
`buildAzureBlobPath()`. `IterateObjects()` now uses it for names
obtained from Azure's list API. `getBlobClient()` (used by `Open`,
`Stat`, `Delete`, `ServeDirectURL`, which take relative paths) is
unchanged in behavior.

Also add `TestAzureBlobStorageDumpArchive`, a regression test that
drives the real dump path (`IterateObjects` → `Stat` →
`dump.Dumper.AddFileByReader` → `mholt/archives` zip writer) against a
**non-empty** `BasePath`, and verifies the produced archive contains the
object with the correct content. The existing Azure tests use an empty
`BasePath` and never read object content via `IterateObjects`, which is
why they didn't catch this.

Fixes #35476

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Mitrahsoft
2026-08-07 16:35:30 +00:00
committed by GitHub
co-authored by wxiaoguang
parent 4c382cea59
commit 7733f1953f
7 changed files with 198 additions and 226 deletions
+34 -41
View File
@@ -32,21 +32,21 @@ var _ Object = &azureBlobObject{}
type azureBlobObject struct { type azureBlobObject struct {
blobClient *blob.Client blobClient *blob.Client
Context context.Context ctx context.Context
Name string name string
Size int64 size int64
ModTime *time.Time modTime *time.Time
offset int64 offset int64
} }
func (a *azureBlobObject) Read(p []byte) (int, error) { func (a *azureBlobObject) Read(p []byte) (int, error) {
// TODO: improve the performance, we can implement another interface, maybe implement io.WriteTo // TODO: improve the performance, we can implement another interface, maybe implement io.WriteTo
if a.offset >= a.Size { if a.offset >= a.size {
return 0, io.EOF return 0, io.EOF
} }
count := min(int64(len(p)), a.Size-a.offset) count := min(int64(len(p)), a.size-a.offset)
res, err := a.blobClient.DownloadBuffer(a.Context, p, &blob.DownloadBufferOptions{ res, err := a.blobClient.DownloadBuffer(a.ctx, p, &blob.DownloadBufferOptions{
Range: blob.HTTPRange{ Range: blob.HTTPRange{
Offset: a.offset, Offset: a.offset,
Count: count, Count: count,
@@ -71,12 +71,12 @@ func (a *azureBlobObject) Seek(offset int64, whence int) (int64, error) {
case io.SeekCurrent: case io.SeekCurrent:
offset += a.offset offset += a.offset
case io.SeekEnd: case io.SeekEnd:
offset = a.Size + offset offset = a.size + offset
default: default:
return 0, errors.New("Seek: invalid whence") return 0, errors.New("Seek: invalid whence")
} }
if offset > a.Size { if offset > a.size {
return 0, errors.New("Seek: invalid offset") return 0, errors.New("Seek: invalid offset")
} else if offset < 0 { } else if offset < 0 {
return 0, errors.New("Seek: invalid offset") return 0, errors.New("Seek: invalid offset")
@@ -87,15 +87,14 @@ func (a *azureBlobObject) Seek(offset int64, whence int) (int64, error) {
func (a *azureBlobObject) Stat() (os.FileInfo, error) { func (a *azureBlobObject) Stat() (os.FileInfo, error) {
return &azureBlobFileInfo{ return &azureBlobFileInfo{
a.Name, a.name,
a.Size, a.size,
*a.ModTime, *a.modTime,
}, nil }, nil
} }
var _ ObjectStorage = &AzureBlobStorage{} var _ ObjectStorage = &AzureBlobStorage{}
// AzureStorage returns a azure blob storage
type AzureBlobStorage struct { type AzureBlobStorage struct {
cfg *setting.AzureBlobStorageConfig cfg *setting.AzureBlobStorageConfig
ctx context.Context ctx context.Context
@@ -150,11 +149,7 @@ func NewAzureBlobStorage(ctx context.Context, cfg *setting.Storage) (ObjectStora
} }
func (a *AzureBlobStorage) buildAzureBlobPath(p string) string { func (a *AzureBlobStorage) buildAzureBlobPath(p string) string {
p = util.PathJoinRelX(a.cfg.BasePath, p) return buildObjectStorePath(a.cfg.BasePath, p)
if p == "." || p == "/" {
p = "" // azure uses prefix, so path should be empty as relative path
}
return p
} }
func (a *AzureBlobStorage) getObjectNameFromPath(path string) string { func (a *AzureBlobStorage) getObjectNameFromPath(path string) string {
@@ -170,11 +165,11 @@ func (a *AzureBlobStorage) Open(path string) (Object, error) {
return nil, convertAzureBlobErr(err) return nil, convertAzureBlobErr(err)
} }
return &azureBlobObject{ return &azureBlobObject{
Context: a.ctx, ctx: a.ctx,
blobClient: blobClient, blobClient: blobClient,
Name: a.getObjectNameFromPath(path), name: a.getObjectNameFromPath(path),
Size: *res.ContentLength, size: *res.ContentLength,
ModTime: res.LastModified, modTime: res.LastModified,
}, nil }, nil
} }
@@ -302,33 +297,32 @@ func (a *AzureBlobStorage) ServeDirectURL(storePath, name, method string, reqPar
return url.Parse(u) return url.Parse(u)
} }
// IterateObjects iterates across the objects in the azureblobstorage
func (a *AzureBlobStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error { func (a *AzureBlobStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error {
dirName = a.buildAzureBlobPath(dirName) basePrefix := buildObjectStorePathPrefix(a.cfg.BasePath, "")
if dirName != "" { dirPrefix := buildObjectStorePathPrefix(a.cfg.BasePath, dirName)
dirName += "/"
}
pager := a.client.NewListBlobsFlatPager(a.cfg.Container, &container.ListBlobsFlatOptions{ pager := a.client.NewListBlobsFlatPager(a.cfg.Container, &container.ListBlobsFlatOptions{
Prefix: &dirName, Prefix: &dirPrefix,
}) })
callback := func(object *azureBlobObject, objPath string) error {
defer object.Close()
return fn(objPath, object)
}
for pager.More() { for pager.More() {
resp, err := pager.NextPage(a.ctx) resp, err := pager.NextPage(a.ctx)
if err != nil { if err != nil {
return convertAzureBlobErr(err) return convertAzureBlobErr(err)
} }
for _, object := range resp.Segment.BlobItems { for _, azureObj := range resp.Segment.BlobItems {
blobClient := a.getBlobClient(*object.Name) objPath := strings.TrimPrefix(*azureObj.Name, basePrefix)
object := &azureBlobObject{ objWrap := &azureBlobObject{
Context: a.ctx, ctx: a.ctx,
blobClient: blobClient, blobClient: a.getBlobClient(objPath),
Name: *object.Name, name: *azureObj.Name,
Size: *object.Properties.ContentLength, size: *azureObj.Properties.ContentLength,
ModTime: object.Properties.LastModified, modTime: azureObj.Properties.LastModified,
} }
if err := func(object *azureBlobObject, fn func(path string, obj Object) error) error { if err := callback(objWrap, objPath); err != nil {
defer object.Close()
return fn(strings.TrimPrefix(object.Name, a.cfg.BasePath), object)
}(object, fn); err != nil {
return convertAzureBlobErr(err) return convertAzureBlobErr(err)
} }
} }
@@ -336,7 +330,6 @@ func (a *AzureBlobStorage) IterateObjects(dirName string, fn func(path string, o
return nil return nil
} }
// Delete delete a file
func (a *AzureBlobStorage) getBlobClient(path string) *blob.Client { func (a *AzureBlobStorage) getBlobClient(path string) *blob.Client {
return a.client.ServiceClient().NewContainerClient(a.cfg.Container).NewBlobClient(a.buildAzureBlobPath(path)) return a.client.ServiceClient().NewContainerClient(a.cfg.Container).NewBlobClient(a.buildAzureBlobPath(path))
} }
+21 -58
View File
@@ -10,82 +10,45 @@ import (
"gitea.dev/modules/setting" "gitea.dev/modules/setting"
"gitea.dev/modules/test" "gitea.dev/modules/test"
"gitea.dev/modules/util"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
) )
func TestAzureBlobStorage(t *testing.T) { func prepareAzureStorageConfig(t *testing.T, basePath ...string) *setting.Storage {
endpoint := test.ExternalServiceHTTP(t, "TEST_AZURESTORAGE_ENDPOINT", "http://devstoreaccount1.azurite.local:10000") endpoint := test.ExternalServiceHTTP(t, "TEST_AZURESTORAGE_ENDPOINT", "http://devstoreaccount1.azurite.local:10000")
storageType := setting.AzureBlobStorageType return &setting.Storage{
config := &setting.Storage{
AzureBlobConfig: setting.AzureBlobStorageConfig{ AzureBlobConfig: setting.AzureBlobStorageConfig{
// https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#ip-style-url // https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#ip-style-url
Endpoint: endpoint, Endpoint: endpoint,
// https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#well-known-storage-account-and-key // https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#well-known-storage-account-and-key
AccountName: "devstoreaccount1", AccountName: "devstoreaccount1",
AccountKey: "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==", AccountKey: "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==",
Container: "test", Container: "test-container",
BasePath: util.OptionalArg(basePath),
}, },
} }
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) { func TestAzureBlobStorage(t *testing.T) {
m := &AzureBlobStorage{cfg: &setting.AzureBlobStorageConfig{BasePath: ""}} t.Run("NoBasePath", func(t *testing.T) {
assert.Empty(t, m.buildAzureBlobPath("/")) config := prepareAzureStorageConfig(t)
assert.Empty(t, m.buildAzureBlobPath(".")) objStore, err := NewStorage(setting.AzureBlobStorageType, config)
assert.Equal(t, "a", m.buildAzureBlobPath("/a")) require.NoError(t, err)
assert.Equal(t, "a/b", m.buildAzureBlobPath("/a/b/")) testStorageGeneral(t, objStore)
})
m = &AzureBlobStorage{cfg: &setting.AzureBlobStorageConfig{BasePath: "/"}} t.Run("WithBasePath", func(t *testing.T) {
assert.Empty(t, m.buildAzureBlobPath("/")) config := prepareAzureStorageConfig(t, "test-base-path")
assert.Empty(t, m.buildAzureBlobPath(".")) objStore, err := NewStorage(setting.AzureBlobStorageType, config)
assert.Equal(t, "a", m.buildAzureBlobPath("/a")) require.NoError(t, err)
assert.Equal(t, "a/b", m.buildAzureBlobPath("/a/b/")) testStorageGeneral(t, objStore)
})
m = &AzureBlobStorage{cfg: &setting.AzureBlobStorageConfig{BasePath: "/base"}}
assert.Equal(t, "base", m.buildAzureBlobPath("/"))
assert.Equal(t, "base", m.buildAzureBlobPath("."))
assert.Equal(t, "base/a", m.buildAzureBlobPath("/a"))
assert.Equal(t, "base/a/b", m.buildAzureBlobPath("/a/b/"))
m = &AzureBlobStorage{cfg: &setting.AzureBlobStorageConfig{BasePath: "/base/"}}
assert.Equal(t, "base", m.buildAzureBlobPath("/"))
assert.Equal(t, "base", m.buildAzureBlobPath("."))
assert.Equal(t, "base/a", m.buildAzureBlobPath("/a"))
assert.Equal(t, "base/a/b", m.buildAzureBlobPath("/a/b/"))
} }
func Test_azureBlobObject(t *testing.T) { func Test_azureBlobObject(t *testing.T) {
endpoint := test.ExternalServiceHTTP(t, "TEST_AZURESTORAGE_ENDPOINT", "http://devstoreaccount1.azurite.local:10000") s, err := NewStorage(setting.AzureBlobStorageType, prepareAzureStorageConfig(t))
s, err := NewStorage(setting.AzureBlobStorageType, &setting.Storage{ require.NoError(t, err)
AzureBlobConfig: setting.AzureBlobStorageConfig{
// https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#ip-style-url
Endpoint: endpoint,
// https://learn.microsoft.com/azure/storage/common/storage-use-azurite?tabs=visual-studio-code#well-known-storage-account-and-key
AccountName: "devstoreaccount1",
AccountKey: "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==",
Container: "test",
},
})
assert.NoError(t, err)
data := "Q2xTckt6Y1hDOWh0" data := "Q2xTckt6Y1hDOWh0"
_, err = s.Save("test.txt", strings.NewReader(data), int64(len(data))) _, err = s.Save("test.txt", strings.NewReader(data), int64(len(data)))
+6 -4
View File
@@ -14,6 +14,12 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
func TestLocalStorage(t *testing.T) {
objStore, err := NewStorage(setting.LocalStorageType, &setting.Storage{Path: t.TempDir()})
require.NoError(t, err)
testStorageGeneral(t, objStore)
}
func TestBuildLocalPath(t *testing.T) { func TestBuildLocalPath(t *testing.T) {
kases := []struct { kases := []struct {
localDir string localDir string
@@ -98,7 +104,3 @@ func TestLocalStorageDelete(t *testing.T) {
assertExists(t, ".", true) assertExists(t, ".", true)
assertExists(t, "dir", false) assertExists(t, "dir", false)
} }
func TestLocalStorageIterator(t *testing.T) {
testStorageIterator(t, setting.LocalStorageType, &setting.Storage{Path: t.TempDir()})
}
+18 -23
View File
@@ -158,20 +158,11 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage,
} }
func (m *MinioStorage) buildMinioPath(p string) string { func (m *MinioStorage) buildMinioPath(p string) string {
p = strings.TrimPrefix(util.PathJoinRelX(m.basePath, p), "/") // object store doesn't use slash for root path return buildObjectStorePath(m.basePath, p)
if p == "." {
p = "" // object store doesn't use dot as relative path
}
return p
} }
func (m *MinioStorage) buildMinioDirPrefix(p string) string { func (m *MinioStorage) buildMinioDirPrefix(p string) string {
// ending slash is required for avoiding matching like "foo/" and "foobar/" with prefix "foo" return buildObjectStorePathPrefix(m.basePath, p)
p = m.buildMinioPath(p) + "/"
if p == "/" {
p = "" // object store doesn't use slash for root path
}
return p
} }
func buildMinioCredentials(config setting.MinioStorageConfig) *credentials.Credentials { func buildMinioCredentials(config setting.MinioStorageConfig) *credentials.Credentials {
@@ -312,22 +303,26 @@ func (m *MinioStorage) ServeDirectURL(storePath, name, method string, opt *Serve
return u, convertMinioErr(err) return u, convertMinioErr(err)
} }
// IterateObjects iterates across the objects in the miniostorage
func (m *MinioStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error { func (m *MinioStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error {
opts := minio.GetObjectOptions{} basePrefix := m.buildMinioDirPrefix("")
// FIXME: this loop is not right and causes resource leaking, see the comment of ListObjects dirPrefix := m.buildMinioDirPrefix(dirName)
for mObjInfo := range m.client.ListObjects(m.ctx, m.bucket, minio.ListObjectsOptions{ callback := func(object *minio.Object, objPath string) error {
Prefix: m.buildMinioDirPrefix(dirName), defer object.Close()
Recursive: true, return fn(objPath, &minioObject{object})
}) { }
object, err := m.client.GetObject(m.ctx, m.bucket, mObjInfo.Key, opts)
ctxList, ctxListCancel := context.WithCancel(m.ctx)
defer ctxListCancel() // ListObjectsIter: make sure to cancel the passed context, without that you might leak coroutines
getOpts := minio.GetObjectOptions{}
listOpts := minio.ListObjectsOptions{Prefix: dirPrefix, Recursive: true}
for mObjInfo := range m.client.ListObjectsIter(ctxList, m.bucket, listOpts) {
object, err := m.client.GetObject(m.ctx, m.bucket, mObjInfo.Key, getOpts)
if err != nil { if err != nil {
return convertMinioErr(err) return convertMinioErr(err)
} }
if err := func(object *minio.Object, fn func(path string, obj Object) error) error { objPath := strings.TrimPrefix(mObjInfo.Key, basePrefix)
defer object.Close() if err := callback(object, objPath); err != nil {
return fn(strings.TrimPrefix(mObjInfo.Key, m.basePath), &minioObject{object})
}(object, fn); err != nil {
return convertMinioErr(err) return convertMinioErr(err)
} }
} }
+22 -66
View File
@@ -10,89 +10,45 @@ import (
"gitea.dev/modules/setting" "gitea.dev/modules/setting"
"gitea.dev/modules/test" "gitea.dev/modules/test"
"gitea.dev/modules/util"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
) )
func TestMinioStorage(t *testing.T) { func prepareMinioStorageConfig(t *testing.T, basePath ...string) *setting.Storage {
endpoint := test.ExternalServiceHTTP(t, "TEST_MINIO_ENDPOINT", "minio:9000") return &setting.Storage{
storageType := setting.MinioStorageType
config := &setting.Storage{
MinioConfig: setting.MinioStorageConfig{ MinioConfig: setting.MinioStorageConfig{
Endpoint: endpoint, Endpoint: test.ExternalServiceHTTP(t, "TEST_MINIO_ENDPOINT", "minio:9000"),
AccessKeyID: "123456", AccessKeyID: "123456",
SecretAccessKey: "12345678", SecretAccessKey: "12345678",
Bucket: "gitea", Bucket: "gitea",
Location: "us-east-1", Location: "us-east-1",
BasePath: util.OptionalArg(basePath),
}, },
} }
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) { func TestMinioStorage(t *testing.T) {
m := &MinioStorage{basePath: ""} t.Run("NoBasePath", func(t *testing.T) {
assert.Empty(t, m.buildMinioPath("/")) config := prepareMinioStorageConfig(t)
assert.Empty(t, m.buildMinioPath(".")) objStore, err := NewStorage(setting.MinioStorageType, config)
assert.Equal(t, "a", m.buildMinioPath("/a")) require.NoError(t, err)
assert.Equal(t, "a/b", m.buildMinioPath("/a/b/")) testStorageGeneral(t, objStore)
assert.Empty(t, m.buildMinioDirPrefix("")) })
assert.Equal(t, "a/", m.buildMinioDirPrefix("/a/")) t.Run("WithBasePath", func(t *testing.T) {
config := prepareMinioStorageConfig(t, "test-base-path")
m = &MinioStorage{basePath: "/"} objStore, err := NewStorage(setting.MinioStorageType, config)
assert.Empty(t, m.buildMinioPath("/")) require.NoError(t, err)
assert.Empty(t, m.buildMinioPath(".")) testStorageGeneral(t, objStore)
assert.Equal(t, "a", m.buildMinioPath("/a")) })
assert.Equal(t, "a/b", m.buildMinioPath("/a/b/"))
assert.Empty(t, m.buildMinioDirPrefix(""))
assert.Equal(t, "a/", m.buildMinioDirPrefix("/a/"))
m = &MinioStorage{basePath: "/base"}
assert.Equal(t, "base", m.buildMinioPath("/"))
assert.Equal(t, "base", m.buildMinioPath("."))
assert.Equal(t, "base/a", m.buildMinioPath("/a"))
assert.Equal(t, "base/a/b", m.buildMinioPath("/a/b/"))
assert.Equal(t, "base/", m.buildMinioDirPrefix(""))
assert.Equal(t, "base/a/", m.buildMinioDirPrefix("/a/"))
m = &MinioStorage{basePath: "/base/"}
assert.Equal(t, "base", m.buildMinioPath("/"))
assert.Equal(t, "base", m.buildMinioPath("."))
assert.Equal(t, "base/a", m.buildMinioPath("/a"))
assert.Equal(t, "base/a/b", m.buildMinioPath("/a/b/"))
assert.Equal(t, "base/", m.buildMinioDirPrefix(""))
assert.Equal(t, "base/a/", m.buildMinioDirPrefix("/a/"))
} }
func TestS3StorageBadRequest(t *testing.T) { func TestS3StorageBadRequest(t *testing.T) {
endpoint := test.ExternalServiceHTTP(t, "TEST_MINIO_ENDPOINT", "minio:9000") cfg := prepareMinioStorageConfig(t)
cfg := &setting.Storage{ cfg.MinioConfig.SecretAccessKey = "invalid-secret"
MinioConfig: setting.MinioStorageConfig{
Endpoint: endpoint,
AccessKeyID: "123456",
SecretAccessKey: "invalid-secret",
Bucket: "bucket",
Location: "us-east-1",
},
}
_, err := NewStorage(setting.MinioStorageType, cfg) _, err := NewStorage(setting.MinioStorageType, cfg)
assert.ErrorContains(t, err, "ObjectStorage.BucketExists: endpoint="+endpoint) assert.ErrorContains(t, err, "ObjectStorage.BucketExists: endpoint="+cfg.MinioConfig.Endpoint)
} }
func TestMinioCredentials(t *testing.T) { func TestMinioCredentials(t *testing.T) {
+19
View File
@@ -11,11 +11,13 @@ import (
"net/url" "net/url"
"os" "os"
"path" "path"
"strings"
"gitea.dev/modules/httplib" "gitea.dev/modules/httplib"
"gitea.dev/modules/log" "gitea.dev/modules/log"
"gitea.dev/modules/public" "gitea.dev/modules/public"
"gitea.dev/modules/setting" "gitea.dev/modules/setting"
"gitea.dev/modules/util"
) )
// ErrURLNotSupported represents url is not supported // ErrURLNotSupported represents url is not supported
@@ -139,6 +141,23 @@ func SaveFrom(objStorage ObjectStorage, path string, callback func(w io.Writer)
return err return err
} }
func buildObjectStorePath(base, p string) string {
p = strings.TrimPrefix(util.PathJoinRelX(base, p), "/") // object store doesn't use slash for root path
if p == "." {
p = "" // object store doesn't use dot as relative path
}
return p
}
func buildObjectStorePathPrefix(base, p string) string {
// ending slash is required for avoiding matching like "foo/" and "foobar/" with prefix "foo"
p = buildObjectStorePath(base, p) + "/"
if p == "/" {
p = "" // object store doesn't use slash for root path
}
return p
}
var ( var (
// Attachments represents attachments storage // Attachments represents attachments storage
Attachments ObjectStorage = uninitializedStorage Attachments ObjectStorage = uninitializedStorage
+78 -34
View File
@@ -4,20 +4,50 @@
package storage package storage
import ( import (
"io"
"net/http" "net/http"
"strings" "strings"
"testing" "testing"
"gitea.dev/modules/setting"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) { func TestObjectStoragePath(t *testing.T) {
l, err := NewStorage(typStr, cfg) base := ""
assert.NoError(t, err) assert.Empty(t, buildObjectStorePath(base, "/"))
assert.Empty(t, buildObjectStorePath(base, "."))
assert.Equal(t, "a", buildObjectStorePath(base, "/a"))
assert.Equal(t, "a/b", buildObjectStorePath(base, "/a/b/"))
assert.Empty(t, buildObjectStorePathPrefix(base, ""))
assert.Equal(t, "a/", buildObjectStorePathPrefix(base, "/a/"))
base = "/"
assert.Empty(t, buildObjectStorePath(base, "/"))
assert.Empty(t, buildObjectStorePath(base, "."))
assert.Equal(t, "a", buildObjectStorePath(base, "/a"))
assert.Equal(t, "a/b", buildObjectStorePath(base, "/a/b/"))
assert.Empty(t, buildObjectStorePathPrefix(base, ""))
assert.Equal(t, "a/", buildObjectStorePathPrefix(base, "/a/"))
base = "/base"
assert.Equal(t, "base", buildObjectStorePath(base, "/"))
assert.Equal(t, "base", buildObjectStorePath(base, "."))
assert.Equal(t, "base/a", buildObjectStorePath(base, "/a"))
assert.Equal(t, "base/a/b", buildObjectStorePath(base, "/a/b/"))
assert.Equal(t, "base/", buildObjectStorePathPrefix(base, ""))
assert.Equal(t, "base/a/", buildObjectStorePathPrefix(base, "/a/"))
base = "/base/"
assert.Equal(t, "base", buildObjectStorePath(base, "/"))
assert.Equal(t, "base", buildObjectStorePath(base, "."))
assert.Equal(t, "base/a", buildObjectStorePath(base, "/a"))
assert.Equal(t, "base/a/b", buildObjectStorePath(base, "/a/b/"))
assert.Equal(t, "base/", buildObjectStorePathPrefix(base, ""))
assert.Equal(t, "base/a/", buildObjectStorePathPrefix(base, "/a/"))
}
func testStorageIterator(t *testing.T, objStore ObjectStorage) {
testFiles := [][]string{ testFiles := [][]string{
{"a/1.txt", "a1"}, {"a/1.txt", "a1"},
{"/a/1.txt", "aa1"}, // same as above, but with leading slash that will be trim {"/a/1.txt", "aa1"}, // same as above, but with leading slash that will be trim
@@ -28,12 +58,19 @@ func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) {
{"b/x 4.txt", "bx4"}, {"b/x 4.txt", "bx4"},
} }
for _, f := range testFiles { for _, f := range testFiles {
_, err = l.Save(f[0], strings.NewReader(f[1]), -1) _, err := objStore.Save(f[0], strings.NewReader(f[1]), -1)
assert.NoError(t, err) assert.NoError(t, err)
} }
defer func() {
for _, f := range testFiles {
_ = objStore.Delete(f[0])
}
}()
expectedList := map[string][]string{ expectedList := map[string][]string{
"a": {"a/1.txt"}, "a": {"a/1.txt"},
"a/": {"a/1.txt"},
"/a/": {"a/1.txt"},
"b": {"b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt"}, "b": {"b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt"},
"": {"a/1.txt", "b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt", "ab/1.txt"}, "": {"a/1.txt", "b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt", "ab/1.txt"},
"/": {"a/1.txt", "b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt", "ab/1.txt"}, "/": {"a/1.txt", "b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt", "ab/1.txt"},
@@ -42,8 +79,10 @@ func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) {
} }
for dir, expected := range expectedList { for dir, expected := range expectedList {
count := 0 count := 0
err = l.IterateObjects(dir, func(path string, f Object) error { err := objStore.IterateObjects(dir, func(path string, f Object) error {
defer f.Close() content, err := io.ReadAll(f)
assert.NoError(t, err)
assert.NotEmpty(t, content)
assert.Contains(t, expected, path) assert.Contains(t, expected, path)
count++ count++
return nil return nil
@@ -53,52 +92,57 @@ func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) {
} }
} }
type expectedServeDirectHeaders struct { func testStorageURLContentTypeAndDisposition(t *testing.T, objStore ObjectStorage) {
ContentType string type expectedServeDirectHeaders struct {
ContentDisposition string ContentType string
} ContentDisposition string
func testSingleBlobStorageURLContentTypeAndDisposition(t *testing.T, s ObjectStorage, path, name string, expected expectedServeDirectHeaders, 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 != "" { test := func(t *testing.T, s ObjectStorage, path, name string, expected expectedServeDirectHeaders, reqParams *ServeDirectOptions) {
assert.Equal(t, expected.ContentDisposition, resp.Header.Get("Content-Disposition")) 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)
testFilename := "test.txt" testFilename := "test.txt"
_, err = s.Save(testFilename, strings.NewReader("dummy-content"), -1) _, err := objStore.Save(testFilename, strings.NewReader("dummy-content"), -1)
assert.NoError(t, err) assert.NoError(t, err)
testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.txt", expectedServeDirectHeaders{ test(t, objStore, testFilename, "test.txt", expectedServeDirectHeaders{
ContentType: "text/plain; charset=utf-8", ContentType: "text/plain; charset=utf-8",
ContentDisposition: `inline; filename=test.txt`, ContentDisposition: `inline; filename=test.txt`,
}, nil) }, nil)
testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.pdf", expectedServeDirectHeaders{ test(t, objStore, testFilename, "test.pdf", expectedServeDirectHeaders{
ContentType: "application/pdf", ContentType: "application/pdf",
ContentDisposition: `inline; filename=test.pdf`, ContentDisposition: `inline; filename=test.pdf`,
}, nil) }, nil)
testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.wasm", expectedServeDirectHeaders{ test(t, objStore, testFilename, "test.wasm", expectedServeDirectHeaders{
ContentDisposition: `inline; filename=test.wasm`, ContentDisposition: `inline; filename=test.wasm`,
}, nil) }, nil)
testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.wasm", expectedServeDirectHeaders{ test(t, objStore, testFilename, "test.wasm", expectedServeDirectHeaders{
ContentType: "application/wasm", ContentType: "application/wasm",
ContentDisposition: `inline; filename=test.wasm`, ContentDisposition: `inline; filename=test.wasm`,
}, &ServeDirectOptions{ }, &ServeDirectOptions{
ContentType: "application/wasm", ContentType: "application/wasm",
}) })
assert.NoError(t, s.Delete(testFilename)) assert.NoError(t, objStore.Delete(testFilename))
}
func testStorageGeneral(t *testing.T, objStore ObjectStorage) {
t.Run("StorageIterator", func(t *testing.T) { testStorageIterator(t, objStore) })
if _, ok := objStore.(*LocalStorage); ok {
t.Skipf("Skipping tests for local storage")
}
t.Run("StorageURLContentTypeAndDisposition", func(t *testing.T) { testStorageURLContentTypeAndDisposition(t, objStore) })
} }