From 774b37b9f8a0d08d2cbd279b4067479fb177fb39 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Wed, 22 Mar 2023 19:56:20 +0800
Subject: [PATCH] Introduce path Clean/Join helper functions, partially
backport&refactor (#23495) (#23607)
Backport #23495, partially backport&refactor
The `modules/options` files are just copied from 1.20 to 1.19
---
modules/options/base.go | 95 +++++++++++++++++++++++++++++++++
modules/options/dynamic.go | 104 ++-----------------------------------
modules/options/static.go | 75 ++++----------------------
modules/util/path.go | 92 +++++++++++++++++++++++++++++---
modules/util/path_test.go | 74 ++++++++++++++++++++++++++
5 files changed, 269 insertions(+), 171 deletions(-)
diff --git a/modules/options/base.go b/modules/options/base.go
index 039e934b3a..7882ed0081 100644
--- a/modules/options/base.go
+++ b/modules/options/base.go
@@ -9,9 +9,52 @@ import (
"os"
"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 fileFromOptionsDir("locale", name)
+}
+
+// Readme reads the content of a specific readme from static/bindata or custom path.
+func Readme(name string) ([]byte, error) {
+ 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 fileFromOptionsDir("gitignore", name)
+}
+
+// License reads the content of a specific license from static/bindata or custom path.
+func License(name string) ([]byte, error) {
+ 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 fileFromOptionsDir("label", name)
+}
+
+// WalkLocales reads the content of a specific locale
+func WalkLocales(callback func(path, name string, d fs.DirEntry, err error) error) error {
+ if IsDynamic() {
+ if err := walkAssetDir(filepath.Join(setting.StaticRootPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) {
+ return fmt.Errorf("failed to walk locales. Error: %w", err)
+ }
+ }
+
+ if err := walkAssetDir(filepath.Join(setting.CustomPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) {
+ return fmt.Errorf("failed to walk locales. Error: %w", err)
+ }
+ return nil
+}
+
func walkAssetDir(root string, callback func(path, name string, d fs.DirEntry, err error) error) error {
if err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
// name is the path relative to the root
@@ -37,3 +80,55 @@ func walkAssetDir(root string, callback func(path, name string, d fs.DirEntry, e
}
return nil
}
+
+// 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 {
+ // 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)
+ }
+ 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...))
+ }
+ 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
+}
diff --git a/modules/options/dynamic.go b/modules/options/dynamic.go
index a20253676e..3d6261983f 100644
--- a/modules/options/dynamic.go
+++ b/modules/options/dynamic.go
@@ -6,120 +6,26 @@
package options
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)
-
// 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
-
- customDir := path.Join(setting.CustomPath, "options", name)
-
- isDir, err := util.IsDir(customDir)
+ result, err := listLocalDirIfExist([]string{setting.CustomPath, setting.StaticRootPath}, "options", name)
if err != nil {
- return []string{}, fmt.Errorf("Unabe to check if custom directory %s is a directory. %w", customDir, err)
- }
- if isDir {
- files, err := util.StatDir(customDir, true)
- if err != nil {
- return []string{}, fmt.Errorf("Failed to read custom directory. %w", err)
- }
-
- result = append(result, files...)
- }
-
- staticDir := path.Join(setting.StaticRootPath, "options", name)
-
- isDir, err = util.IsDir(staticDir)
- if err != nil {
- return []string{}, fmt.Errorf("unable to check if static directory %s is a directory. %w", staticDir, err)
- }
- if isDir {
- files, err := util.StatDir(staticDir, true)
- if err != nil {
- return []string{}, fmt.Errorf("Failed to read static directory. %w", err)
- }
-
- result = append(result, files...)
+ return nil, err
}
return directories.AddAndGet(name, result), nil
}
-// Locale reads the content of a specific locale from static or custom path.
-func Locale(name string) ([]byte, error) {
- return fileFromDir(path.Join("locale", name))
-}
-
-// WalkLocales reads the content of a specific locale from static or custom path.
-func WalkLocales(callback func(path, name string, d fs.DirEntry, err error) error) error {
- if err := walkAssetDir(filepath.Join(setting.StaticRootPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) {
- return fmt.Errorf("failed to walk locales. Error: %w", err)
- }
-
- if err := walkAssetDir(filepath.Join(setting.CustomPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) {
- return fmt.Errorf("failed to walk locales. Error: %w", err)
- }
- return nil
-}
-
-// Readme reads the content of a specific readme from static or custom path.
-func Readme(name string) ([]byte, error) {
- return fileFromDir(path.Join("readme", name))
-}
-
-// Gitignore reads the content of a specific gitignore from static or custom path.
-func Gitignore(name string) ([]byte, error) {
- return fileFromDir(path.Join("gitignore", name))
-}
-
-// License reads the content of a specific license from static or custom path.
-func License(name string) ([]byte, error) {
- return fileFromDir(path.Join("license", name))
-}
-
-// Labels reads the content of a specific labels from static or custom path.
-func Labels(name string) ([]byte, error) {
- return fileFromDir(path.Join("label", name))
-}
-
-// 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)
diff --git a/modules/options/static.go b/modules/options/static.go
index ff3c86d3f8..0482dea681 100644
--- a/modules/options/static.go
+++ b/modules/options/static.go
@@ -8,38 +8,20 @@ package options
import (
"fmt"
"io"
- "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)
-
-// 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
-
- customDir := path.Join(setting.CustomPath, "options", name)
- isDir, err := util.IsDir(customDir)
+ result, err := listLocalDirIfExist([]string{setting.CustomPath}, "options", name)
if err != nil {
- return []string{}, fmt.Errorf("unable to check if custom directory %q is a directory. %w", customDir, err)
- }
- if isDir {
- files, err := util.StatDir(customDir, true)
- if err != nil {
- return []string{}, fmt.Errorf("unable to read custom directory %q. %w", customDir, err)
- }
-
- result = append(result, files...)
+ return nil, err
}
files, err := AssetDir(name)
@@ -69,57 +51,18 @@ func AssetDir(dirName string) ([]string, error) {
return results, nil
}
-// Locale reads the content of a specific locale from bindata or custom path.
-func Locale(name string) ([]byte, error) {
- return fileFromDir(path.Join("locale", name))
-}
-
-// WalkLocales reads the content of a specific locale from static or custom path.
-func WalkLocales(callback func(path, name string, d fs.DirEntry, err error) error) error {
- if err := walkAssetDir(filepath.Join(setting.CustomPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) {
- return fmt.Errorf("failed to walk locales. Error: %w", err)
- }
- return nil
-}
-
-// Readme reads the content of a specific readme from bindata or custom path.
-func Readme(name string) ([]byte, error) {
- return fileFromDir(path.Join("readme", name))
-}
-
-// Gitignore reads the content of a gitignore locale from bindata or custom path.
-func Gitignore(name string) ([]byte, error) {
- return fileFromDir(path.Join("gitignore", name))
-}
-
-// License reads the content of a specific license from bindata or custom path.
-func License(name string) ([]byte, error) {
- return fileFromDir(path.Join("license", name))
-}
-
-// Labels reads the content of a specific labels from static or custom path.
-func Labels(name string) ([]byte, error) {
- return fileFromDir(path.Join("label", name))
-}
-
-// 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)
}
diff --git a/modules/util/path.go b/modules/util/path.go
index 74acb7a85f..37d06e9813 100644
--- a/modules/util/path.go
+++ b/modules/util/path.go
@@ -5,6 +5,7 @@ package util
import (
"errors"
+ "fmt"
"net/url"
"os"
"path"
@@ -14,13 +15,92 @@ import (
"strings"
)
-// 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
+// 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)
}
- return filepath.Join(absoluteBase, path)
+ p := path.Join(elems...)
+ if p == "" {
+ return ""
+ } else if p == "/" {
+ return "."
+ } else {
+ return p[1:]
+ }
+}
+
+// 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 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,
diff --git a/modules/util/path_test.go b/modules/util/path_test.go
index 93f4f67cf6..1d27c9bf0c 100644
--- a/modules/util/path_test.go
+++ b/modules/util/path_test.go
@@ -136,3 +136,77 @@ func TestMisc_IsReadmeFileName(t *testing.T) {
assert.Equal(t, testCase.idx, idx)
}
}
+
+func TestCleanPath(t *testing.T) {
+ 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)
+ }
+
+ 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)
+ }
+}