feat: vendor gitea 1.16.2

package/gitea/source/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)

package/gitea/source/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) {

package/gitea/source/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)
 }
 

package/gitea/source/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))
 			}

package/gitea/source/modules/storage/local.go

@@ -5,6 +5,7 @@ package storage
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/url"
@@ -27,25 +28,32 @@ type LocalStorage struct {
 
 // NewLocalStorage returns a local files
 func NewLocalStorage(ctx context.Context, config *setting.Storage) (ObjectStorage, error) {
+	// prepare storage root path
 	if !filepath.IsAbs(config.Path) {
-		return nil, fmt.Errorf("LocalStorageConfig.Path should have been prepared by setting/storage.go and should be an absolute path, but not: %q", config.Path)
-	}
-	log.Info("Creating new Local Storage at %s", config.Path)
-	if err := os.MkdirAll(config.Path, os.ModePerm); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("LocalStorage config.Path should have been prepared by setting/storage.go and should be an absolute path, but not: %q", config.Path)
 	}
+	storageRoot := util.FilePathJoinAbs(config.Path)
 
-	if config.TemporaryPath == "" {
-		config.TemporaryPath = filepath.Join(config.Path, "tmp")
+	// prepare storage temporary path
+	storageTmp := config.TemporaryPath
+	if storageTmp == "" {
+		storageTmp = filepath.Join(storageRoot, "tmp")
 	}
-	if !filepath.IsAbs(config.TemporaryPath) {
-		return nil, fmt.Errorf("LocalStorageConfig.TemporaryPath should be an absolute path, but not: %q", config.TemporaryPath)
+	if !filepath.IsAbs(storageTmp) {
+		return nil, fmt.Errorf("LocalStorage config.TemporaryPath should be an absolute path, but not: %q", config.TemporaryPath)
+	}
+	storageTmp = util.FilePathJoinAbs(storageTmp)
+
+	// create the storage root if not exist
+	log.Info("Creating new Local Storage at %s", storageRoot)
+	if err := os.MkdirAll(storageRoot, os.ModePerm); err != nil {
+		return nil, err
 	}
 
 	return &LocalStorage{
 		ctx:    ctx,
-		dir:    config.Path,
-		tmpdir: config.TemporaryPath,
+		dir:    storageRoot,
+		tmpdir: storageTmp,
 	}, nil
 }
 
@@ -108,44 +116,59 @@ func (l *LocalStorage) Stat(path string) (os.FileInfo, error) {
 	return os.Stat(l.buildLocalPath(path))
 }
 
-// Delete delete a file
-func (l *LocalStorage) Delete(path string) error {
-	return util.Remove(l.buildLocalPath(path))
+func (l *LocalStorage) deleteEmptyParentDirs(localFullPath string) {
+	for parent := filepath.Dir(localFullPath); len(parent) > len(l.dir); parent = filepath.Dir(parent) {
+		if err := os.Remove(parent); err != nil {
+			// since the target file has been deleted, parent dir error is not related to the file deletion itself.
+			break
+		}
+	}
 }
 
-// URL gets the redirect URL to a file
-func (l *LocalStorage) URL(path, name, _ string, reqParams url.Values) (*url.URL, error) {
+// Delete deletes the file in storage and removes the empty parent directories (if possible)
+func (l *LocalStorage) Delete(path string) error {
+	localFullPath := l.buildLocalPath(path)
+	err := util.Remove(localFullPath)
+	l.deleteEmptyParentDirs(localFullPath)
+	return err
+}
+
+func (l *LocalStorage) ServeDirectURL(path, name, _ string, reqParams *ServeDirectOptions) (*url.URL, error) {
 	return nil, ErrURLNotSupported
 }
 
+func (l *LocalStorage) normalizeWalkError(err error) error {
+	if errors.Is(err, os.ErrNotExist) {
+		// ignore it because the file may be deleted during the walk, and we don't care about it
+		return nil
+	}
+	return err
+}
+
 // IterateObjects iterates across the objects in the local storage
 func (l *LocalStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error {
 	dir := l.buildLocalPath(dirName)
-	return filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error {
-		if err != nil {
+	return filepath.WalkDir(dir, func(path string, d os.DirEntry, errWalk error) error {
+		if err := l.ctx.Err(); err != nil {
 			return err
 		}
-		select {
-		case <-l.ctx.Done():
-			return l.ctx.Err()
-		default:
+		if errWalk != nil {
+			return l.normalizeWalkError(errWalk)
 		}
-		if path == l.dir {
-			return nil
-		}
-		if d.IsDir() {
+		if path == l.dir || d.IsDir() {
 			return nil
 		}
+
 		relPath, err := filepath.Rel(l.dir, path)
 		if err != nil {
-			return err
+			return l.normalizeWalkError(err)
 		}
 		obj, err := os.Open(path)
 		if err != nil {
-			return err
+			return l.normalizeWalkError(err)
 		}
 		defer obj.Close()
-		return fn(relPath, obj)
+		return fn(filepath.ToSlash(relPath), obj)
 	})
 }
 

package/gitea/source/modules/storage/local_test.go

@@ -4,11 +4,14 @@
 package storage
 
 import (
+	"os"
+	"strings"
 	"testing"
 
 	"code.gitea.io/gitea/modules/setting"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestBuildLocalPath(t *testing.T) {
@@ -53,6 +56,49 @@ func TestBuildLocalPath(t *testing.T) {
 	}
 }
 
+func TestLocalStorageDelete(t *testing.T) {
+	rootDir := t.TempDir()
+	st, err := NewLocalStorage(t.Context(), &setting.Storage{Path: rootDir})
+	require.NoError(t, err)
+
+	assertExists := func(t *testing.T, path string, exists bool) {
+		_, err = os.Stat(rootDir + "/" + path)
+		if exists {
+			require.NoError(t, err)
+		} else {
+			require.ErrorIs(t, err, os.ErrNotExist)
+		}
+	}
+
+	_, err = st.Save("dir/sub1/1-a.txt", strings.NewReader(""), -1)
+	require.NoError(t, err)
+	_, err = st.Save("dir/sub1/1-b.txt", strings.NewReader(""), -1)
+	require.NoError(t, err)
+	_, err = st.Save("dir/sub2/2-a.txt", strings.NewReader(""), -1)
+	require.NoError(t, err)
+
+	assertExists(t, "dir/sub1/1-a.txt", true)
+	assertExists(t, "dir/sub1/1-b.txt", true)
+	assertExists(t, "dir/sub2/2-a.txt", true)
+
+	require.NoError(t, st.Delete("dir/sub1/1-a.txt"))
+	assertExists(t, "dir/sub1", true)
+	assertExists(t, "dir/sub1/1-a.txt", false)
+	assertExists(t, "dir/sub1/1-b.txt", true)
+	assertExists(t, "dir/sub2/2-a.txt", true)
+
+	require.NoError(t, st.Delete("dir/sub1/1-b.txt"))
+	assertExists(t, ".", true)
+	assertExists(t, "dir/sub1", false)
+	assertExists(t, "dir/sub1/1-a.txt", false)
+	assertExists(t, "dir/sub1/1-b.txt", false)
+	assertExists(t, "dir/sub2/2-a.txt", true)
+
+	require.NoError(t, st.Delete("dir/sub2/2-a.txt"))
+	assertExists(t, ".", true)
+	assertExists(t, "dir", false)
+}
+
 func TestLocalStorageIterator(t *testing.T) {
 	testStorageIterator(t, setting.LocalStorageType, &setting.Storage{Path: t.TempDir()})
 }

package/gitea/source/modules/storage/minio.go

@@ -23,11 +23,7 @@ import (
 	"github.com/minio/minio-go/v7/pkg/credentials"
 )
 
-var (
-	_ ObjectStorage = &MinioStorage{}
-
-	quoteEscaper = strings.NewReplacer("\\", "\\\\", `"`, "\\\"")
-)
+var _ ObjectStorage = &MinioStorage{}
 
 type minioObject struct {
 	*minio.Object
@@ -278,37 +274,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 +298,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,

package/gitea/source/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) {

package/gitea/source/modules/storage/storage.go

@@ -10,8 +10,11 @@ import (
 	"io"
 	"net/url"
 	"os"
+	"path"
 
+	"code.gitea.io/gitea/modules/httplib"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/public"
 	"code.gitea.io/gitea/modules/setting"
 )
 
@@ -56,6 +59,37 @@ 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
+}
+
+// Safe defaults are applied only when not explicitly overridden by the caller.
+func prepareServeDirectOptions(optsOptional *ServeDirectOptions, name string) (ret struct {
+	ContentType        string
+	ContentDisposition string
+},
+) {
+	// 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.ContentType = optsOptional.ContentType
+	}
+	name = path.Base(name)
+	if ret.ContentType == "" {
+		ext := path.Ext(name)
+		ret.ContentType = public.DetectWellKnownMimeType(ext)
+	}
+	// 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 = httplib.EncodeContentDispositionInline(name)
+	return ret
+}
+
 // ObjectStorage represents an object storage to handle a bucket and files
 type ObjectStorage interface {
 	Open(path string) (Object, error)
@@ -67,8 +101,21 @@ type ObjectStorage interface {
 
 	Stat(path string) (os.FileInfo, error)
 	Delete(path string) error
-	URL(path, name, method string, reqParams url.Values) (*url.URL, error)
-	IterateObjects(path string, iterator func(path string, obj Object) error) 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.
+	// * IterateObjects("", ...): iterate all objects in this storage
+	// * IterateObjects("sub-path", ...): iterate all objects with "sub-path" as prefix in this storage, the "fullPath" will be like "sub-path/xxx"
+	IterateObjects(basePath string, iterator func(fullPath string, obj Object) error) error
 }
 
 // Copy copies a file from source ObjectStorage to dest ObjectStorage
@@ -131,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
 )
 

package/gitea/source/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,53 @@ func testStorageIterator(t *testing.T, typStr Type, cfg *setting.Storage) {
 		assert.Len(t, expected, count)
 	}
 }
+
+type expectedServeDirectHeaders struct {
+	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 != "" {
+		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"
+	_, err = s.Save(testFilename, strings.NewReader("dummy-content"), -1)
+	assert.NoError(t, err)
+
+	testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.txt", expectedServeDirectHeaders{
+		ContentType:        "text/plain; charset=utf-8",
+		ContentDisposition: `inline; filename=test.txt`,
+	}, nil)
+
+	testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.pdf", expectedServeDirectHeaders{
+		ContentType:        "application/pdf",
+		ContentDisposition: `inline; filename=test.pdf`,
+	}, nil)
+
+	testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.wasm", expectedServeDirectHeaders{
+		ContentDisposition: `inline; filename=test.wasm`,
+	}, nil)
+
+	testSingleBlobStorageURLContentTypeAndDisposition(t, s, testFilename, "test.wasm", expectedServeDirectHeaders{
+		ContentType:        "application/wasm",
+		ContentDisposition: `inline; filename=test.wasm`,
+	}, &ServeDirectOptions{
+		ContentType: "application/wasm",
+	})
+	assert.NoError(t, s.Delete(testFilename))
+}