Introduce path Clean/Join helper functions (#23495)
Since #23493 has conflicts with latest commits, this PR is my proposal for fixing #23371 Details are in the comments And refactor the `modules/options` module, to make it always use "filepath" to access local files. Benefits: * No need to do `util.CleanPath(strings.ReplaceAll(p, "\\", "/"))), "/")` any more (not only one before) * The function behaviors are clearly defined
This commit is contained in:
		
					parent
					
						
							
								253a00aaac
							
						
					
				
			
			
				commit
				
					
						ce9dee5a1e
					
				
			
		
					 16 changed files with 261 additions and 152 deletions
				
			
		| 
						 | 
				
			
			@ -34,7 +34,7 @@ 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 = util.PathJoinRel(l.Path)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
// CreateLFSLock creates a new lock.
 | 
			
		||||
| 
						 | 
				
			
			@ -49,7 +49,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo
 | 
			
		|||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	lock.Path = util.CleanPath(lock.Path)
 | 
			
		||||
	lock.Path = util.PathJoinRel(lock.Path)
 | 
			
		||||
	lock.RepoID = repo.ID
 | 
			
		||||
 | 
			
		||||
	l, err := GetLFSLock(dbCtx, repo, lock.Path)
 | 
			
		||||
| 
						 | 
				
			
			@ -69,7 +69,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 = util.PathJoinRel(path)
 | 
			
		||||
	rel := &LFSLock{RepoID: repo.ID}
 | 
			
		||||
	has, err := db.GetEngine(ctx).Where("lower(path) = ?", strings.ToLower(path)).Get(rel)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -7,36 +7,38 @@ import (
 | 
			
		|||
	"fmt"
 | 
			
		||||
	"io/fs"
 | 
			
		||||
	"os"
 | 
			
		||||
	"path"
 | 
			
		||||
	"path/filepath"
 | 
			
		||||
 | 
			
		||||
	"code.gitea.io/gitea/modules/log"
 | 
			
		||||
	"code.gitea.io/gitea/modules/setting"
 | 
			
		||||
	"code.gitea.io/gitea/modules/util"
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
var directories = make(directorySet)
 | 
			
		||||
 | 
			
		||||
// 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 fileFromOptionsDir("locale", 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 fileFromOptionsDir("readme", 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 fileFromOptionsDir("gitignore", 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 fileFromOptionsDir("license", 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 fileFromOptionsDir("label", name)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
// WalkLocales reads the content of a specific locale
 | 
			
		||||
| 
						 | 
				
			
			@ -79,17 +81,54 @@ func walkAssetDir(root string, callback func(path, name string, d fs.DirEntry, e
 | 
			
		|||
	return nil
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func statDirIfExist(dir string) ([]string, error) {
 | 
			
		||||
	isDir, err := util.IsDir(dir)
 | 
			
		||||
// mustLocalPathAbs coverts a path to absolute path
 | 
			
		||||
// FIXME: the old behavior (StaticRootPath might not be absolute), not ideal, just keep the same as before
 | 
			
		||||
func mustLocalPathAbs(s string) string {
 | 
			
		||||
	abs, err := filepath.Abs(s)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return nil, fmt.Errorf("unable to check if static directory %s is a directory. %w", dir, err)
 | 
			
		||||
		// This should never happen in a real system. If it happens, the user must have already been in trouble: the system is not able to resolve its own paths.
 | 
			
		||||
		log.Fatal("Unable to get absolute path for %q: %v", s, err)
 | 
			
		||||
	}
 | 
			
		||||
	if !isDir {
 | 
			
		||||
		return nil, nil
 | 
			
		||||
	return abs
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func joinLocalPaths(baseDirs []string, subDir string, elems ...string) (paths []string) {
 | 
			
		||||
	abs := make([]string, len(elems)+2)
 | 
			
		||||
	abs[1] = subDir
 | 
			
		||||
	copy(abs[2:], elems)
 | 
			
		||||
	for _, baseDir := range baseDirs {
 | 
			
		||||
		abs[0] = mustLocalPathAbs(baseDir)
 | 
			
		||||
		paths = append(paths, util.FilePathJoinAbs(abs...))
 | 
			
		||||
	}
 | 
			
		||||
	files, err := util.StatDir(dir, true)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return nil, fmt.Errorf("unable to read directory %q. %w", dir, err)
 | 
			
		||||
	return paths
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func listLocalDirIfExist(baseDirs []string, subDir string, elems ...string) (files []string, err error) {
 | 
			
		||||
	for _, localPath := range joinLocalPaths(baseDirs, subDir, elems...) {
 | 
			
		||||
		isDir, err := util.IsDir(localPath)
 | 
			
		||||
		if err != nil {
 | 
			
		||||
			return nil, fmt.Errorf("unable to check if path %q is a directory. %w", localPath, err)
 | 
			
		||||
		} else if !isDir {
 | 
			
		||||
			continue
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
		dirFiles, err := util.StatDir(localPath, true)
 | 
			
		||||
		if err != nil {
 | 
			
		||||
			return nil, fmt.Errorf("unable to read directory %q. %w", localPath, err)
 | 
			
		||||
		}
 | 
			
		||||
		files = append(files, dirFiles...)
 | 
			
		||||
	}
 | 
			
		||||
	return files, nil
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
func readLocalFile(baseDirs []string, subDir string, elems ...string) ([]byte, error) {
 | 
			
		||||
	for _, localPath := range joinLocalPaths(baseDirs, subDir, elems...) {
 | 
			
		||||
		data, err := os.ReadFile(localPath)
 | 
			
		||||
		if err == nil {
 | 
			
		||||
			return data, nil
 | 
			
		||||
		} else if !os.IsNotExist(err) {
 | 
			
		||||
			log.Error("Unable to read file %q. Error: %v", localPath, err)
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
	return nil, os.ErrNotExist
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -6,62 +6,26 @@
 | 
			
		|||
package options
 | 
			
		||||
 | 
			
		||||
import (
 | 
			
		||||
	"fmt"
 | 
			
		||||
	"os"
 | 
			
		||||
	"path"
 | 
			
		||||
 | 
			
		||||
	"code.gitea.io/gitea/modules/log"
 | 
			
		||||
	"code.gitea.io/gitea/modules/setting"
 | 
			
		||||
	"code.gitea.io/gitea/modules/util"
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
var directories = make(directorySet)
 | 
			
		||||
 | 
			
		||||
// Dir returns all files from static or custom directory.
 | 
			
		||||
func Dir(name string) ([]string, error) {
 | 
			
		||||
	if directories.Filled(name) {
 | 
			
		||||
		return directories.Get(name), nil
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	var result []string
 | 
			
		||||
 | 
			
		||||
	for _, dir := range []string{
 | 
			
		||||
		path.Join(setting.CustomPath, "options", name),     // custom dir
 | 
			
		||||
		path.Join(setting.StaticRootPath, "options", name), // static dir
 | 
			
		||||
	} {
 | 
			
		||||
		files, err := statDirIfExist(dir)
 | 
			
		||||
		if err != nil {
 | 
			
		||||
			return nil, err
 | 
			
		||||
		}
 | 
			
		||||
		result = append(result, files...)
 | 
			
		||||
	result, err := listLocalDirIfExist([]string{setting.CustomPath, setting.StaticRootPath}, "options", name)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	return directories.AddAndGet(name, result), nil
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
// fileFromDir is a helper to read files from static or custom path.
 | 
			
		||||
func fileFromDir(name string) ([]byte, error) {
 | 
			
		||||
	customPath := path.Join(setting.CustomPath, "options", name)
 | 
			
		||||
 | 
			
		||||
	isFile, err := util.IsFile(customPath)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		log.Error("Unable to check if %s is a file. Error: %v", customPath, err)
 | 
			
		||||
	}
 | 
			
		||||
	if isFile {
 | 
			
		||||
		return os.ReadFile(customPath)
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	staticPath := path.Join(setting.StaticRootPath, "options", name)
 | 
			
		||||
 | 
			
		||||
	isFile, err = util.IsFile(staticPath)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		log.Error("Unable to check if %s is a file. Error: %v", staticPath, err)
 | 
			
		||||
	}
 | 
			
		||||
	if isFile {
 | 
			
		||||
		return os.ReadFile(staticPath)
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	return []byte{}, fmt.Errorf("Asset file does not exist: %s", name)
 | 
			
		||||
// fileFromOptionsDir is a helper to read files from custom or static path.
 | 
			
		||||
func fileFromOptionsDir(elems ...string) ([]byte, error) {
 | 
			
		||||
	return readLocalFile([]string{setting.CustomPath, setting.StaticRootPath}, "options", elems...)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
// IsDynamic will return false when using embedded data (-tags bindata)
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -8,33 +8,20 @@ package options
 | 
			
		|||
import (
 | 
			
		||||
	"fmt"
 | 
			
		||||
	"io"
 | 
			
		||||
	"os"
 | 
			
		||||
	"path"
 | 
			
		||||
 | 
			
		||||
	"code.gitea.io/gitea/modules/log"
 | 
			
		||||
	"code.gitea.io/gitea/modules/setting"
 | 
			
		||||
	"code.gitea.io/gitea/modules/util"
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
var directories = make(directorySet)
 | 
			
		||||
 | 
			
		||||
// Dir returns all files from bindata or custom directory.
 | 
			
		||||
// Dir returns all files from custom directory or bindata.
 | 
			
		||||
func Dir(name string) ([]string, error) {
 | 
			
		||||
	if directories.Filled(name) {
 | 
			
		||||
		return directories.Get(name), nil
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	var result []string
 | 
			
		||||
 | 
			
		||||
	for _, dir := range []string{
 | 
			
		||||
		path.Join(setting.CustomPath, "options", name), // custom dir
 | 
			
		||||
		// no static dir
 | 
			
		||||
	} {
 | 
			
		||||
		files, err := statDirIfExist(dir)
 | 
			
		||||
		if err != nil {
 | 
			
		||||
			return nil, err
 | 
			
		||||
		}
 | 
			
		||||
		result = append(result, files...)
 | 
			
		||||
	result, err := listLocalDirIfExist([]string{setting.CustomPath}, "options", name)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	files, err := AssetDir(name)
 | 
			
		||||
| 
						 | 
				
			
			@ -64,24 +51,18 @@ func AssetDir(dirName string) ([]string, error) {
 | 
			
		|||
	return results, nil
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
// fileFromDir is a helper to read files from bindata or custom path.
 | 
			
		||||
func fileFromDir(name string) ([]byte, error) {
 | 
			
		||||
	customPath := path.Join(setting.CustomPath, "options", name)
 | 
			
		||||
 | 
			
		||||
	isFile, err := util.IsFile(customPath)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		log.Error("Unable to check if %s is a file. Error: %v", customPath, err)
 | 
			
		||||
	}
 | 
			
		||||
	if isFile {
 | 
			
		||||
		return os.ReadFile(customPath)
 | 
			
		||||
// fileFromOptionsDir is a helper to read files from custom path or bindata.
 | 
			
		||||
func fileFromOptionsDir(elems ...string) ([]byte, error) {
 | 
			
		||||
	// only try custom dir, no static dir
 | 
			
		||||
	if data, err := readLocalFile([]string{setting.CustomPath}, "options", elems...); err == nil {
 | 
			
		||||
		return data, nil
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	f, err := Assets.Open(name)
 | 
			
		||||
	f, err := Assets.Open(util.PathJoinRelX(elems...))
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		return nil, err
 | 
			
		||||
	}
 | 
			
		||||
	defer f.Close()
 | 
			
		||||
 | 
			
		||||
	return io.ReadAll(f)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -45,29 +45,19 @@ func AssetsHandlerFunc(opts *Options) http.HandlerFunc {
 | 
			
		|||
			return
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
		file := req.URL.Path
 | 
			
		||||
		file = file[len(opts.Prefix):]
 | 
			
		||||
		if len(file) == 0 {
 | 
			
		||||
			resp.WriteHeader(http.StatusNotFound)
 | 
			
		||||
			return
 | 
			
		||||
		}
 | 
			
		||||
		if strings.Contains(file, "\\") {
 | 
			
		||||
			resp.WriteHeader(http.StatusBadRequest)
 | 
			
		||||
			return
 | 
			
		||||
		}
 | 
			
		||||
		file = "/" + file
 | 
			
		||||
 | 
			
		||||
		var written bool
 | 
			
		||||
		var corsSent bool
 | 
			
		||||
		if opts.CorsHandler != nil {
 | 
			
		||||
			written = true
 | 
			
		||||
			opts.CorsHandler(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
 | 
			
		||||
				written = false
 | 
			
		||||
				corsSent = true
 | 
			
		||||
			})).ServeHTTP(resp, req)
 | 
			
		||||
		}
 | 
			
		||||
		if written {
 | 
			
		||||
		// If CORS is not sent, the response must have been written by other handlers
 | 
			
		||||
		if !corsSent {
 | 
			
		||||
			return
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
		file := req.URL.Path[len(opts.Prefix):]
 | 
			
		||||
 | 
			
		||||
		// custom files
 | 
			
		||||
		if opts.handle(resp, req, http.Dir(custPath), file) {
 | 
			
		||||
			return
 | 
			
		||||
| 
						 | 
				
			
			@ -102,8 +92,8 @@ 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))
 | 
			
		||||
	// actually, fs (http.FileSystem) is designed to be a safe interface, relative paths won't bypass its parent directory, it's also fine to do a clean here
 | 
			
		||||
	f, err := fs.Open(util.PathJoinRelX(file))
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		if os.IsNotExist(err) {
 | 
			
		||||
			return false
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -5,11 +5,11 @@ package storage
 | 
			
		|||
 | 
			
		||||
import (
 | 
			
		||||
	"context"
 | 
			
		||||
	"fmt"
 | 
			
		||||
	"io"
 | 
			
		||||
	"net/url"
 | 
			
		||||
	"os"
 | 
			
		||||
	"path/filepath"
 | 
			
		||||
	"strings"
 | 
			
		||||
 | 
			
		||||
	"code.gitea.io/gitea/modules/log"
 | 
			
		||||
	"code.gitea.io/gitea/modules/util"
 | 
			
		||||
| 
						 | 
				
			
			@ -41,13 +41,19 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
 | 
			
		|||
	}
 | 
			
		||||
	config := configInterface.(LocalStorageConfig)
 | 
			
		||||
 | 
			
		||||
	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
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	if config.TemporaryPath == "" {
 | 
			
		||||
		config.TemporaryPath = config.Path + "/tmp"
 | 
			
		||||
		config.TemporaryPath = filepath.Join(config.Path, "tmp")
 | 
			
		||||
	}
 | 
			
		||||
	if !filepath.IsAbs(config.TemporaryPath) {
 | 
			
		||||
		return nil, fmt.Errorf("LocalStorageConfig.TemporaryPath should be an absolute path, but not: %q", config.TemporaryPath)
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	return &LocalStorage{
 | 
			
		||||
| 
						 | 
				
			
			@ -58,7 +64,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 util.FilePathJoinAbs(l.dir, p)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
// Open a file
 | 
			
		||||
| 
						 | 
				
			
			@ -128,10 +134,7 @@ func (l *LocalStorage) URL(path, name string) (*url.URL, error) {
 | 
			
		|||
 | 
			
		||||
// IterateObjects iterates across the objects in the local storage
 | 
			
		||||
func (l *LocalStorage) IterateObjects(prefix string, fn func(path string, obj Object) error) error {
 | 
			
		||||
	dir := l.dir
 | 
			
		||||
	if prefix != "" {
 | 
			
		||||
		dir = filepath.Join(l.dir, util.CleanPath(prefix))
 | 
			
		||||
	}
 | 
			
		||||
	dir := l.buildLocalPath(prefix)
 | 
			
		||||
	return filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error {
 | 
			
		||||
		if err != nil {
 | 
			
		||||
			return err
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -20,29 +20,29 @@ func TestBuildLocalPath(t *testing.T) {
 | 
			
		|||
		expected string
 | 
			
		||||
	}{
 | 
			
		||||
		{
 | 
			
		||||
			"a",
 | 
			
		||||
			"/a",
 | 
			
		||||
			"0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
			"a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
			"/a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
		},
 | 
			
		||||
		{
 | 
			
		||||
			"a",
 | 
			
		||||
			"/a",
 | 
			
		||||
			"../0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
			"a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
			"/a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
		},
 | 
			
		||||
		{
 | 
			
		||||
			"a",
 | 
			
		||||
			"/a",
 | 
			
		||||
			"0\\a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
			"a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
			"/a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
		},
 | 
			
		||||
		{
 | 
			
		||||
			"b",
 | 
			
		||||
			"/b",
 | 
			
		||||
			"a/../0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
			"b/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
			"/b/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
		},
 | 
			
		||||
		{
 | 
			
		||||
			"b",
 | 
			
		||||
			"/b",
 | 
			
		||||
			"a\\..\\0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
			"b/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
			"/b/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14",
 | 
			
		||||
		},
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -121,7 +121,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 util.PathJoinRelX(m.basePath, p)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
// Open open a file
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -5,6 +5,7 @@ package util
 | 
			
		|||
 | 
			
		||||
import (
 | 
			
		||||
	"errors"
 | 
			
		||||
	"fmt"
 | 
			
		||||
	"net/url"
 | 
			
		||||
	"os"
 | 
			
		||||
	"path"
 | 
			
		||||
| 
						 | 
				
			
			@ -14,21 +15,92 @@ import (
 | 
			
		|||
	"strings"
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
// CleanPath ensure to clean the path
 | 
			
		||||
func CleanPath(p string) string {
 | 
			
		||||
	if strings.HasPrefix(p, "/") {
 | 
			
		||||
		return path.Clean(p)
 | 
			
		||||
// PathJoinRel joins the path elements into a single path, each element is cleaned by path.Clean separately.
 | 
			
		||||
// It only returns the following values (like path.Join), any redundant part (empty, relative dots, slashes) is removed.
 | 
			
		||||
// It's caller's duty to make every element not bypass its own directly level, to avoid security issues.
 | 
			
		||||
//
 | 
			
		||||
//	empty => ``
 | 
			
		||||
//	`` => ``
 | 
			
		||||
//	`..` => `.`
 | 
			
		||||
//	`dir` => `dir`
 | 
			
		||||
//	`/dir/` => `dir`
 | 
			
		||||
//	`foo\..\bar` => `foo\..\bar`
 | 
			
		||||
//	{`foo`, ``, `bar`} => `foo/bar`
 | 
			
		||||
//	{`foo`, `..`, `bar`} => `foo/bar`
 | 
			
		||||
func PathJoinRel(elem ...string) string {
 | 
			
		||||
	elems := make([]string, len(elem))
 | 
			
		||||
	for i, e := range elem {
 | 
			
		||||
		if e == "" {
 | 
			
		||||
			continue
 | 
			
		||||
		}
 | 
			
		||||
		elems[i] = path.Clean("/" + e)
 | 
			
		||||
	}
 | 
			
		||||
	p := path.Join(elems...)
 | 
			
		||||
	if p == "" {
 | 
			
		||||
		return ""
 | 
			
		||||
	} else if p == "/" {
 | 
			
		||||
		return "."
 | 
			
		||||
	} else {
 | 
			
		||||
		return p[1:]
 | 
			
		||||
	}
 | 
			
		||||
	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 {
 | 
			
		||||
	if filepath.IsAbs(path) {
 | 
			
		||||
		return path
 | 
			
		||||
// PathJoinRelX joins the path elements into a single path like PathJoinRel,
 | 
			
		||||
// and covert all backslashes to slashes. (X means "extended", also means the combination of `\` and `/`).
 | 
			
		||||
// It's caller's duty to make every element not bypass its own directly level, to avoid security issues.
 | 
			
		||||
// It returns similar results as PathJoinRel except:
 | 
			
		||||
//
 | 
			
		||||
//	`foo\..\bar` => `bar`  (because it's processed as `foo/../bar`)
 | 
			
		||||
//
 | 
			
		||||
// All backslashes are handled as slashes, the result only contains slashes.
 | 
			
		||||
func PathJoinRelX(elem ...string) string {
 | 
			
		||||
	elems := make([]string, len(elem))
 | 
			
		||||
	for i, e := range elem {
 | 
			
		||||
		if e == "" {
 | 
			
		||||
			continue
 | 
			
		||||
		}
 | 
			
		||||
		elems[i] = path.Clean("/" + strings.ReplaceAll(e, "\\", "/"))
 | 
			
		||||
	}
 | 
			
		||||
	return filepath.Join(absoluteBase, path)
 | 
			
		||||
	return PathJoinRel(elems...)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
const pathSeparator = string(os.PathSeparator)
 | 
			
		||||
 | 
			
		||||
// FilePathJoinAbs joins the path elements into a single file path, each element is cleaned by filepath.Clean separately.
 | 
			
		||||
// All slashes/backslashes are converted to path separators before cleaning, the result only contains path separators.
 | 
			
		||||
// The first element must be an absolute path, caller should prepare the base path.
 | 
			
		||||
// It's caller's duty to make every element not bypass its own directly level, to avoid security issues.
 | 
			
		||||
// Like PathJoinRel, any redundant part (empty, relative dots, slashes) is removed.
 | 
			
		||||
//
 | 
			
		||||
//	{`/foo`, ``, `bar`} => `/foo/bar`
 | 
			
		||||
//	{`/foo`, `..`, `bar`} => `/foo/bar`
 | 
			
		||||
func FilePathJoinAbs(elem ...string) string {
 | 
			
		||||
	elems := make([]string, len(elem))
 | 
			
		||||
 | 
			
		||||
	// POISX filesystem can have `\` in file names. Windows: `\` and `/` are both used for path separators
 | 
			
		||||
	// to keep the behavior consistent, we do not allow `\` in file names, replace all `\` with `/`
 | 
			
		||||
	if isOSWindows() {
 | 
			
		||||
		elems[0] = filepath.Clean(elem[0])
 | 
			
		||||
	} else {
 | 
			
		||||
		elems[0] = filepath.Clean(strings.ReplaceAll(elem[0], "\\", pathSeparator))
 | 
			
		||||
	}
 | 
			
		||||
	if !filepath.IsAbs(elems[0]) {
 | 
			
		||||
		// This shouldn't happen. If there is really necessary to pass in relative path, return the full path with filepath.Abs() instead
 | 
			
		||||
		panic(fmt.Sprintf("FilePathJoinAbs: %q (for path %v) is not absolute, do not guess a relative path based on current working directory", elems[0], elems))
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	for i := 1; i < len(elem); i++ {
 | 
			
		||||
		if elem[i] == "" {
 | 
			
		||||
			continue
 | 
			
		||||
		}
 | 
			
		||||
		if isOSWindows() {
 | 
			
		||||
			elems[i] = filepath.Clean(pathSeparator + elem[i])
 | 
			
		||||
		} else {
 | 
			
		||||
			elems[i] = filepath.Clean(pathSeparator + strings.ReplaceAll(elem[i], "\\", pathSeparator))
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
	// the elems[0] must be an absolute path, just join them together
 | 
			
		||||
	return filepath.Join(elems...)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
// IsDir returns true if given path is a directory,
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -138,13 +138,75 @@ func TestMisc_IsReadmeFileName(t *testing.T) {
 | 
			
		|||
}
 | 
			
		||||
 | 
			
		||||
func TestCleanPath(t *testing.T) {
 | 
			
		||||
	cases := map[string]string{
 | 
			
		||||
		"../../test": "test",
 | 
			
		||||
		"/test":      "/test",
 | 
			
		||||
		"/../test":   "/test",
 | 
			
		||||
	cases := []struct {
 | 
			
		||||
		elems    []string
 | 
			
		||||
		expected string
 | 
			
		||||
	}{
 | 
			
		||||
		{[]string{}, ``},
 | 
			
		||||
		{[]string{``}, ``},
 | 
			
		||||
		{[]string{`..`}, `.`},
 | 
			
		||||
		{[]string{`a`}, `a`},
 | 
			
		||||
		{[]string{`/a/`}, `a`},
 | 
			
		||||
		{[]string{`../a/`, `../b`, `c/..`, `d`}, `a/b/d`},
 | 
			
		||||
		{[]string{`a\..\b`}, `a\..\b`},
 | 
			
		||||
		{[]string{`a`, ``, `b`}, `a/b`},
 | 
			
		||||
		{[]string{`a`, `..`, `b`}, `a/b`},
 | 
			
		||||
		{[]string{`lfs`, `repo/..`, `user/../path`}, `lfs/path`},
 | 
			
		||||
	}
 | 
			
		||||
	for _, c := range cases {
 | 
			
		||||
		assert.Equal(t, c.expected, PathJoinRel(c.elems...), "case: %v", c.elems)
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	for k, v := range cases {
 | 
			
		||||
		assert.Equal(t, v, CleanPath(k))
 | 
			
		||||
	cases = []struct {
 | 
			
		||||
		elems    []string
 | 
			
		||||
		expected string
 | 
			
		||||
	}{
 | 
			
		||||
		{[]string{}, ``},
 | 
			
		||||
		{[]string{``}, ``},
 | 
			
		||||
		{[]string{`..`}, `.`},
 | 
			
		||||
		{[]string{`a`}, `a`},
 | 
			
		||||
		{[]string{`/a/`}, `a`},
 | 
			
		||||
		{[]string{`../a/`, `../b`, `c/..`, `d`}, `a/b/d`},
 | 
			
		||||
		{[]string{`a\..\b`}, `b`},
 | 
			
		||||
		{[]string{`a`, ``, `b`}, `a/b`},
 | 
			
		||||
		{[]string{`a`, `..`, `b`}, `a/b`},
 | 
			
		||||
		{[]string{`lfs`, `repo/..`, `user/../path`}, `lfs/path`},
 | 
			
		||||
	}
 | 
			
		||||
	for _, c := range cases {
 | 
			
		||||
		assert.Equal(t, c.expected, PathJoinRelX(c.elems...), "case: %v", c.elems)
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	// for POSIX only, but the result is similar on Windows, because the first element must be an absolute path
 | 
			
		||||
	if isOSWindows() {
 | 
			
		||||
		cases = []struct {
 | 
			
		||||
			elems    []string
 | 
			
		||||
			expected string
 | 
			
		||||
		}{
 | 
			
		||||
			{[]string{`C:\..`}, `C:\`},
 | 
			
		||||
			{[]string{`C:\a`}, `C:\a`},
 | 
			
		||||
			{[]string{`C:\a/`}, `C:\a`},
 | 
			
		||||
			{[]string{`C:\..\a\`, `../b`, `c\..`, `d`}, `C:\a\b\d`},
 | 
			
		||||
			{[]string{`C:\a/..\b`}, `C:\b`},
 | 
			
		||||
			{[]string{`C:\a`, ``, `b`}, `C:\a\b`},
 | 
			
		||||
			{[]string{`C:\a`, `..`, `b`}, `C:\a\b`},
 | 
			
		||||
			{[]string{`C:\lfs`, `repo/..`, `user/../path`}, `C:\lfs\path`},
 | 
			
		||||
		}
 | 
			
		||||
	} else {
 | 
			
		||||
		cases = []struct {
 | 
			
		||||
			elems    []string
 | 
			
		||||
			expected string
 | 
			
		||||
		}{
 | 
			
		||||
			{[]string{`/..`}, `/`},
 | 
			
		||||
			{[]string{`/a`}, `/a`},
 | 
			
		||||
			{[]string{`/a/`}, `/a`},
 | 
			
		||||
			{[]string{`/../a/`, `../b`, `c/..`, `d`}, `/a/b/d`},
 | 
			
		||||
			{[]string{`/a\..\b`}, `/b`},
 | 
			
		||||
			{[]string{`/a`, ``, `b`}, `/a/b`},
 | 
			
		||||
			{[]string{`/a`, `..`, `b`}, `/a/b`},
 | 
			
		||||
			{[]string{`/lfs`, `repo/..`, `user/../path`}, `/lfs/path`},
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
	for _, c := range cases {
 | 
			
		||||
		assert.Equal(t, c.expected, FilePathJoinAbs(c.elems...), "case: %v", c.elems)
 | 
			
		||||
	}
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -45,7 +45,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 = util.PathJoinRelX(rPath)
 | 
			
		||||
 | 
			
		||||
				u, err := objStore.URL(rPath, path.Base(rPath))
 | 
			
		||||
				if err != nil {
 | 
			
		||||
| 
						 | 
				
			
			@ -81,8 +81,8 @@ 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, "\\", "/"))
 | 
			
		||||
			if rPath == "" {
 | 
			
		||||
			rPath = util.PathJoinRelX(rPath)
 | 
			
		||||
			if rPath == "" || rPath == "." {
 | 
			
		||||
				http.Error(w, "file not found", http.StatusNotFound)
 | 
			
		||||
				return
 | 
			
		||||
			}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -726,7 +726,7 @@ func UploadFilePost(ctx *context.Context) {
 | 
			
		|||
 | 
			
		||||
func cleanUploadFileName(name string) string {
 | 
			
		||||
	// Rebase the filename
 | 
			
		||||
	name = strings.Trim(util.CleanPath(name), "/")
 | 
			
		||||
	name = util.PathJoinRel(name)
 | 
			
		||||
	// Git disallows any filenames to have a .git directory in them.
 | 
			
		||||
	for _, part := range strings.Split(name, "/") {
 | 
			
		||||
		if strings.ToLower(part) == ".git" {
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -207,7 +207,7 @@ func LFSLockFile(ctx *context.Context) {
 | 
			
		|||
		ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks")
 | 
			
		||||
		return
 | 
			
		||||
	}
 | 
			
		||||
	lockPath = util.CleanPath(lockPath)
 | 
			
		||||
	lockPath = util.PathJoinRel(lockPath)
 | 
			
		||||
	if len(lockPath) == 0 {
 | 
			
		||||
		ctx.Flash.Error(ctx.Tr("repo.settings.lfs_invalid_locking_path", originalPath))
 | 
			
		||||
		ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks")
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -865,8 +865,8 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error {
 | 
			
		|||
				_, _, line, _ = git.ParseDiffHunkString(comment.DiffHunk)
 | 
			
		||||
			}
 | 
			
		||||
 | 
			
		||||
			// SECURITY: The TreePath must be cleaned!
 | 
			
		||||
			comment.TreePath = util.CleanPath(comment.TreePath)
 | 
			
		||||
			// SECURITY: The TreePath must be cleaned! use relative path
 | 
			
		||||
			comment.TreePath = util.PathJoinRel(comment.TreePath)
 | 
			
		||||
 | 
			
		||||
			var patch string
 | 
			
		||||
			reader, writer := io.Pipe()
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -8,8 +8,6 @@ import (
 | 
			
		|||
	"errors"
 | 
			
		||||
	"io"
 | 
			
		||||
	"os"
 | 
			
		||||
	"path/filepath"
 | 
			
		||||
	"strings"
 | 
			
		||||
 | 
			
		||||
	packages_model "code.gitea.io/gitea/models/packages"
 | 
			
		||||
	packages_module "code.gitea.io/gitea/modules/packages"
 | 
			
		||||
| 
						 | 
				
			
			@ -33,7 +31,7 @@ type BlobUploader struct {
 | 
			
		|||
}
 | 
			
		||||
 | 
			
		||||
func buildFilePath(id string) string {
 | 
			
		||||
	return filepath.Join(setting.Packages.ChunkedUploadPath, util.CleanPath(strings.ReplaceAll(id, "\\", "/")))
 | 
			
		||||
	return util.FilePathJoinAbs(setting.Packages.ChunkedUploadPath, id)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
// NewBlobUploader creates a new blob uploader for the given id
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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 = util.PathJoinRel(name)
 | 
			
		||||
	// Git disallows any filenames to have a .git directory in them.
 | 
			
		||||
	for _, part := range strings.Split(name, "/") {
 | 
			
		||||
		if strings.ToLower(part) == ".git" {
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue