From d5c274359cff7b0f598065c65de3b451fc3f99e7 Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Wed, 15 Mar 2023 15:01:02 +0800
Subject: [PATCH] Revert "Use CleanPath instead of path.Clean (#23371)"

This reverts commit b116418f05b822481bba3613873eef876da73814.
---
 models/git/lfs_lock.go                       | 12 ++++++++----
 modules/options/base.go                      | 10 +++++-----
 modules/public/public.go                     |  4 ++--
 modules/storage/local.go                     |  3 ++-
 modules/storage/minio.go                     |  3 +--
 modules/util/path.go                         |  8 --------
 modules/util/path_test.go                    | 12 ------------
 routers/web/base.go                          |  5 ++---
 routers/web/repo/editor.go                   |  2 +-
 routers/web/repo/lfs.go                      |  2 +-
 services/migrations/gitea_uploader.go        |  4 ++--
 services/packages/container/blob_uploader.go |  4 ++--
 services/repository/files/file.go            |  4 ++--
 13 files changed, 28 insertions(+), 45 deletions(-)

diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go
index 178fa72f09bf6..25480f3f96541 100644
--- a/models/git/lfs_lock.go
+++ b/models/git/lfs_lock.go
@@ -6,6 +6,7 @@ package git
 import (
 	"context"
 	"fmt"
+	"path"
 	"strings"
 	"time"
 
@@ -16,7 +17,6 @@ import (
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/setting"
-	"code.gitea.io/gitea/modules/util"
 )
 
 // LFSLock represents a git lfs lock of repository.
@@ -34,7 +34,11 @@ func init() {
 
 // BeforeInsert is invoked from XORM before inserting an object of this type.
 func (l *LFSLock) BeforeInsert() {
-	l.Path = util.CleanPath(l.Path)
+	l.Path = cleanPath(l.Path)
+}
+
+func cleanPath(p string) string {
+	return path.Clean("/" + p)[1:]
 }
 
 // CreateLFSLock creates a new lock.
@@ -49,7 +53,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo
 		return nil, err
 	}
 
-	lock.Path = util.CleanPath(lock.Path)
+	lock.Path = cleanPath(lock.Path)
 	lock.RepoID = repo.ID
 
 	l, err := GetLFSLock(dbCtx, repo, lock.Path)
@@ -69,7 +73,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo
 
 // GetLFSLock returns release by given path.
 func GetLFSLock(ctx context.Context, repo *repo_model.Repository, path string) (*LFSLock, error) {
-	path = util.CleanPath(path)
+	path = cleanPath(path)
 	rel := &LFSLock{RepoID: repo.ID}
 	has, err := db.GetEngine(ctx).Where("lower(path) = ?", strings.ToLower(path)).Get(rel)
 	if err != nil {
diff --git a/modules/options/base.go b/modules/options/base.go
index e83e8df5d0945..3c140f64326be 100644
--- a/modules/options/base.go
+++ b/modules/options/base.go
@@ -16,27 +16,27 @@ import (
 
 // Locale reads the content of a specific locale from static/bindata or custom path.
 func Locale(name string) ([]byte, error) {
-	return fileFromDir(path.Join("locale", util.CleanPath(name)))
+	return fileFromDir(path.Join("locale", path.Clean("/"+name)))
 }
 
 // Readme reads the content of a specific readme from static/bindata or custom path.
 func Readme(name string) ([]byte, error) {
-	return fileFromDir(path.Join("readme", util.CleanPath(name)))
+	return fileFromDir(path.Join("readme", path.Clean("/"+name)))
 }
 
 // Gitignore reads the content of a gitignore locale from static/bindata or custom path.
 func Gitignore(name string) ([]byte, error) {
-	return fileFromDir(path.Join("gitignore", util.CleanPath(name)))
+	return fileFromDir(path.Join("gitignore", path.Clean("/"+name)))
 }
 
 // License reads the content of a specific license from static/bindata or custom path.
 func License(name string) ([]byte, error) {
-	return fileFromDir(path.Join("license", util.CleanPath(name)))
+	return fileFromDir(path.Join("license", path.Clean("/"+name)))
 }
 
 // Labels reads the content of a specific labels from static/bindata or custom path.
 func Labels(name string) ([]byte, error) {
-	return fileFromDir(path.Join("label", util.CleanPath(name)))
+	return fileFromDir(path.Join("label", path.Clean("/"+name)))
 }
 
 // WalkLocales reads the content of a specific locale
diff --git a/modules/public/public.go b/modules/public/public.go
index e1d60d89eb9f9..42026f9b10549 100644
--- a/modules/public/public.go
+++ b/modules/public/public.go
@@ -6,6 +6,7 @@ package public
 import (
 	"net/http"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -13,7 +14,6 @@ import (
 	"code.gitea.io/gitea/modules/httpcache"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
-	"code.gitea.io/gitea/modules/util"
 )
 
 // Options represents the available options to configure the handler.
@@ -103,7 +103,7 @@ func setWellKnownContentType(w http.ResponseWriter, file string) {
 
 func (opts *Options) handle(w http.ResponseWriter, req *http.Request, fs http.FileSystem, file string) bool {
 	// use clean to keep the file is a valid path with no . or ..
-	f, err := fs.Open(util.CleanPath(file))
+	f, err := fs.Open(path.Clean(file))
 	if err != nil {
 		if os.IsNotExist(err) {
 			return false
diff --git a/modules/storage/local.go b/modules/storage/local.go
index 15f5761e8f055..0e20d0ac70abd 100644
--- a/modules/storage/local.go
+++ b/modules/storage/local.go
@@ -8,6 +8,7 @@ import (
 	"io"
 	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -58,7 +59,7 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
 }
 
 func (l *LocalStorage) buildLocalPath(p string) string {
-	return filepath.Join(l.dir, util.CleanPath(strings.ReplaceAll(p, "\\", "/")))
+	return filepath.Join(l.dir, path.Clean("/" + strings.ReplaceAll(p, "\\", "/"))[1:])
 }
 
 // Open a file
diff --git a/modules/storage/minio.go b/modules/storage/minio.go
index 8cc06bcdd3df2..baaf93a8c0720 100644
--- a/modules/storage/minio.go
+++ b/modules/storage/minio.go
@@ -15,7 +15,6 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/modules/log"
-	"code.gitea.io/gitea/modules/util"
 
 	"github.com/minio/minio-go/v7"
 	"github.com/minio/minio-go/v7/pkg/credentials"
@@ -121,7 +120,7 @@ func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
 }
 
 func (m *MinioStorage) buildMinioPath(p string) string {
-	return strings.TrimPrefix(path.Join(m.basePath, util.CleanPath(strings.ReplaceAll(p, "\\", "/"))), "/")
+	return strings.TrimPrefix(path.Join(m.basePath, path.Clean("/" + strings.ReplaceAll(p, "\\", "/"))[1:]), "/")
 }
 
 // Open open a file
diff --git a/modules/util/path.go b/modules/util/path.go
index 5aa9e15f5c3e9..74acb7a85fd87 100644
--- a/modules/util/path.go
+++ b/modules/util/path.go
@@ -14,14 +14,6 @@ import (
 	"strings"
 )
 
-// CleanPath ensure to clean the path
-func CleanPath(p string) string {
-	if strings.HasPrefix(p, "/") {
-		return path.Clean(p)
-	}
-	return path.Clean("/" + p)[1:]
-}
-
 // EnsureAbsolutePath ensure that a path is absolute, making it
 // relative to absoluteBase if necessary
 func EnsureAbsolutePath(path, absoluteBase string) string {
diff --git a/modules/util/path_test.go b/modules/util/path_test.go
index 2f020f924dd2a..93f4f67cf6486 100644
--- a/modules/util/path_test.go
+++ b/modules/util/path_test.go
@@ -136,15 +136,3 @@ func TestMisc_IsReadmeFileName(t *testing.T) {
 		assert.Equal(t, testCase.idx, idx)
 	}
 }
-
-func TestCleanPath(t *testing.T) {
-	cases := map[string]string{
-		"../../test": "test",
-		"/test":      "/test",
-		"/../test":   "/test",
-	}
-
-	for k, v := range cases {
-		assert.Equal(t, v, CleanPath(k))
-	}
-}
diff --git a/routers/web/base.go b/routers/web/base.go
index 2eb0b6f39118a..e537382a638a5 100644
--- a/routers/web/base.go
+++ b/routers/web/base.go
@@ -19,7 +19,6 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/templates"
-	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web/middleware"
 	"code.gitea.io/gitea/modules/web/routing"
 	"code.gitea.io/gitea/services/auth"
@@ -45,7 +44,7 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
 				routing.UpdateFuncInfo(req.Context(), funcInfo)
 
 				rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/")
-				rPath = util.CleanPath(strings.ReplaceAll(rPath, "\\", "/"))
+				rPath = path.Clean("/" + strings.ReplaceAll(rPath, "\\", "/"))[1:]
 
 				u, err := objStore.URL(rPath, path.Base(rPath))
 				if err != nil {
@@ -81,7 +80,7 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
 			routing.UpdateFuncInfo(req.Context(), funcInfo)
 
 			rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/")
-			rPath = util.CleanPath(strings.ReplaceAll(rPath, "\\", "/"))
+			rPath = path.Clean("/" + strings.ReplaceAll(rPath, "\\", "/"))[1:]
 			if rPath == "" {
 				http.Error(w, "file not found", http.StatusNotFound)
 				return
diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go
index 4f208098e4766..e5ba4ad2c1398 100644
--- a/routers/web/repo/editor.go
+++ b/routers/web/repo/editor.go
@@ -726,7 +726,7 @@ func UploadFilePost(ctx *context.Context) {
 
 func cleanUploadFileName(name string) string {
 	// Rebase the filename
-	name = strings.Trim(util.CleanPath(name), "/")
+	name = strings.Trim(path.Clean("/"+name), "/")
 	// Git disallows any filenames to have a .git directory in them.
 	for _, part := range strings.Split(name, "/") {
 		if strings.ToLower(part) == ".git" {
diff --git a/routers/web/repo/lfs.go b/routers/web/repo/lfs.go
index 43f5527986b9b..869a69c377219 100644
--- a/routers/web/repo/lfs.go
+++ b/routers/web/repo/lfs.go
@@ -207,7 +207,7 @@ func LFSLockFile(ctx *context.Context) {
 		ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks")
 		return
 	}
-	lockPath = util.CleanPath(lockPath)
+	lockPath = path.Clean("/" + lockPath)[1:]
 	if len(lockPath) == 0 {
 		ctx.Flash.Error(ctx.Tr("repo.settings.lfs_invalid_locking_path", originalPath))
 		ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks")
diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go
index ca961524d12c2..8b259a362b1eb 100644
--- a/services/migrations/gitea_uploader.go
+++ b/services/migrations/gitea_uploader.go
@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -29,7 +30,6 @@ import (
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/uri"
-	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/services/pull"
 
 	"github.com/google/uuid"
@@ -866,7 +866,7 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error {
 			}
 
 			// SECURITY: The TreePath must be cleaned!
-			comment.TreePath = util.CleanPath(comment.TreePath)
+			comment.TreePath = path.Clean("/" + comment.TreePath)[1:]
 
 			var patch string
 			reader, writer := io.Pipe()
diff --git a/services/packages/container/blob_uploader.go b/services/packages/container/blob_uploader.go
index 860672587d2b4..ba92b0507343a 100644
--- a/services/packages/container/blob_uploader.go
+++ b/services/packages/container/blob_uploader.go
@@ -8,13 +8,13 @@ import (
 	"errors"
 	"io"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
 	packages_model "code.gitea.io/gitea/models/packages"
 	packages_module "code.gitea.io/gitea/modules/packages"
 	"code.gitea.io/gitea/modules/setting"
-	"code.gitea.io/gitea/modules/util"
 )
 
 var (
@@ -33,7 +33,7 @@ type BlobUploader struct {
 }
 
 func buildFilePath(id string) string {
-	return filepath.Join(setting.Packages.ChunkedUploadPath, util.CleanPath(strings.ReplaceAll(id, "\\", "/")))
+	return filepath.Join(setting.Packages.ChunkedUploadPath, path.Clean("/" + strings.ReplaceAll(id, "\\", "/"))[1:])
 }
 
 // NewBlobUploader creates a new blob uploader for the given id
diff --git a/services/repository/files/file.go b/services/repository/files/file.go
index 7939491aec624..2bac4372d378c 100644
--- a/services/repository/files/file.go
+++ b/services/repository/files/file.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"net/url"
+	"path"
 	"strings"
 	"time"
 
@@ -14,7 +15,6 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
 	api "code.gitea.io/gitea/modules/structs"
-	"code.gitea.io/gitea/modules/util"
 )
 
 // GetFileResponseFromCommit Constructs a FileResponse from a Commit object
@@ -129,7 +129,7 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *user_m
 // CleanUploadFileName Trims a filename and returns empty string if it is a .git directory
 func CleanUploadFileName(name string) string {
 	// Rebase the filename
-	name = strings.Trim(util.CleanPath(name), "/")
+	name = strings.Trim(path.Clean("/"+name), "/")
 	// Git disallows any filenames to have a .git directory in them.
 	for _, part := range strings.Split(name, "/") {
 		if strings.ToLower(part) == ".git" {