improvements

Author: lunny

custom/conf/app.example.ini

@@ -1079,7 +1079,7 @@ LEVEL = Info
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;
-;; Path for local repository copy. Defaults to `local-repo` under `TEMP_PATH` except it's an absolute path (content gets deleted on gitea restart)
+;; Path for local repository copy. Defaults to  TEMP_PATH + `local-repo`
 ;LOCAL_COPY_PATH = local-repo
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@@ -1091,7 +1091,7 @@ LEVEL = Info
 ;; Whether repository file uploads are enabled. Defaults to `true`
 ;ENABLED = true
 ;;
-;; Path for uploads. Defaults to `uploads` under `TEMP_PATH` except it's an absolute path (content gets deleted on gitea restart)
+;; Path for uploads. Defaults to TEMP_PATH + `uploads`
 ;TEMP_PATH = uploads
 ;;
 ;; Comma-separated list of allowed file extensions (`.zip`), mime types (`text/plain`) or wildcard type (`image/*`, `audio/*`, `video/*`). Empty value or `*/*` allows all types.
@@ -2592,7 +2592,7 @@ LEVEL = Info
 ;; Currently, only `minio` and `azureblob` is supported.
 ;SERVE_DIRECT = false
 ;;
-;; Path for chunked uploads. Defaults to `package-upload` under `TEMP_PATH` except it's an absolute path.
+;; Path for chunked uploads. Defaults it's `package-upload` under `TEMP_PATH` unless it's an absolute path.
 ;CHUNKED_UPLOAD_PATH = package-upload
 ;;
 ;; Maximum count of package versions a single owner can have (`-1` means no limits)

modules/repository/temp.go

@@ -14,8 +14,8 @@ import (
 	"code.gitea.io/gitea/modules/util"
 )
 
-// LocalCopyPath returns the local repository temporary copy path.
-func LocalCopyPath() string {
+// localCopyPath returns the local repository temporary copy path.
+func localCopyPath() string {
 	if setting.Repository.Local.LocalCopyPath == "" {
 		return filepath.Join(setting.TempPath, "local-repo")
 	} else if !filepath.IsAbs(setting.Repository.Local.LocalCopyPath) {
@@ -26,14 +26,14 @@ func LocalCopyPath() string {
 
 // CreateTemporaryPath creates a temporary path
 func CreateTemporaryPath(prefix string) (string, context.CancelFunc, error) {
-	if err := os.MkdirAll(LocalCopyPath(), os.ModePerm); err != nil {
-		log.Error("Unable to create localcopypath directory: %s (%v)", LocalCopyPath(), err)
-		return "", func() {}, fmt.Errorf("failed to create localcopypath directory %s: %w", LocalCopyPath(), err)
+	if err := os.MkdirAll(localCopyPath(), os.ModePerm); err != nil {
+		log.Error("Unable to create localcopypath directory: %s (%v)", localCopyPath(), err)
+		return "", nil, fmt.Errorf("failed to create localcopypath directory %s: %w", localCopyPath(), err)
 	}
-	basePath, err := os.MkdirTemp(LocalCopyPath(), prefix+".git")
+	basePath, err := os.MkdirTemp(localCopyPath(), prefix+".git")
 	if err != nil {
 		log.Error("Unable to create temporary directory: %s-*.git (%v)", prefix, err)
-		return "", func() {}, fmt.Errorf("failed to create dir %s-*.git: %w", prefix, err)
+		return "", nil, fmt.Errorf("failed to create dir %s-*.git: %w", prefix, err)
 	}
 	return basePath, func() {
 		if err := util.RemoveAll(basePath); err != nil {

modules/setting/setting.go

@@ -7,6 +7,7 @@ package setting
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"time"
@@ -187,12 +188,18 @@ func loadTempDir(rootCfg ConfigProvider) {
 	rootSec := rootCfg.Section("")
 	tempPath := rootSec.Key("TEMP_PATH").String()
 	if tempPath != "" {
+		if !filepath.IsAbs(tempPath) {
+			tempPath = filepath.Join(os.TempDir(), tempPath)
+		}
 		TempPath = tempPath
 	}
-	if TempPath != "" {
-		if err := os.MkdirAll(TempPath, os.ModePerm); err != nil {
-			log.Fatal("Failed to create temp directory %s: %v", TempPath, err)
-		}
+	// TempPath has been initialized in init function of global.go
+	if TempPath == "" {
+		log.Fatal("It's impossible that TEMP_PATH is empty")
+	}
+
+	if err := os.MkdirAll(TempPath, os.ModePerm); err != nil {
+		log.Fatal("Failed to create temp directory %s: %v", TempPath, err)
 	}
 }
 

modules/storage/local.go

@@ -36,7 +36,7 @@ func NewLocalStorage(ctx context.Context, config *setting.Storage) (ObjectStorag
 	}
 
 	if config.TemporaryPath == "" {
-		config.TemporaryPath = filepath.Join(setting.TempPath, filepath.Base(config.Path))
+		config.TemporaryPath = filepath.Join(setting.TempPath, "storage", util.SanitizeDirName(config.Path))
 	}
 	if !filepath.IsAbs(config.TemporaryPath) {
 		return nil, fmt.Errorf("LocalStorageConfig.TemporaryPath should be an absolute path, but not: %q", config.TemporaryPath)

modules/util/path.go

@@ -291,3 +291,11 @@ func IsReadmeFileExtension(name string, ext ...string) (int, bool) {
 
 	return 0, false
 }
+
+// SanitizeDirName replaces illegal characters in a directory name with "_"
+func SanitizeDirName(path string) string {
+	p := filepath.Clean(path)
+	re := regexp.MustCompile(`[<>:"/\\|?*]`)
+	sanitized := re.ReplaceAllString(p, "_")
+	return sanitized
+}

modules/util/path_test.go

@@ -230,3 +230,20 @@ func TestListDirRecursively(t *testing.T) {
 	require.NoError(t, err)
 	assert.ElementsMatch(t, []string{"d1/f-d1", "d1/s1/f-d1s1"}, res)
 }
+
+func TestSanitizeDirName(t *testing.T) {
+	cases := []struct {
+		path     string
+		expected string
+	}{
+		{"a", "a"},
+		{"a/b", "a_b"},
+		{"/a/b", "_a_b"},
+		{"a/b/c", "a_b_c"},
+		{"c:\\a\\b", "c__a_b"},
+		{"c:\\a/b", "c__a_b"},
+	}
+	for _, c := range cases {
+		assert.Equal(t, c.expected, SanitizeDirName(c.path))
+	}
+}

services/repository/repository.go

@@ -12,7 +12,6 @@ import (
 	issues_model "code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/models/organization"
 	repo_model "code.gitea.io/gitea/models/repo"
-	system_model "code.gitea.io/gitea/models/system"
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/graceful"
@@ -101,8 +100,6 @@ func Init(ctx context.Context) error {
 	if err := repo_module.LoadRepoConfig(); err != nil {
 		return err
 	}
-	system_model.RemoveAllWithNotice(ctx, "Clean up temporary repository uploads", setting.Repository.Upload.TempPath)
-	system_model.RemoveAllWithNotice(ctx, "Clean up temporary repositories", repo_module.LocalCopyPath())
 	if err := initPushQueue(); err != nil {
 		return err
 	}

tests/test_utils.go

@@ -18,7 +18,6 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/graceful"
 	"code.gitea.io/gitea/modules/log"
-	repo_module "code.gitea.io/gitea/modules/repository"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/test"
@@ -69,7 +68,6 @@ func InitTest(requireGitea bool) {
 
 	unittest.InitSettings()
 	setting.Repository.DefaultBranch = "master" // many test code still assume that default branch is called "master"
-	_ = util.RemoveAll(repo_module.LocalCopyPath())
 
 	if err := git.InitFull(context.Background()); err != nil {
 		log.Fatal("git.InitOnceWithSync: %v", err)