modules/options/base.go

@@ -9,52 +9,9 @@ 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
@@ -80,55 +37,3 @@ 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
-}

modules/options/dynamic.go

@@ -6,26 +6,120 @@
 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
 	}
 
-	result, err := listLocalDirIfExist([]string{setting.CustomPath, setting.StaticRootPath}, "options", name)
+	var result []string
+
+	customDir := path.Join(setting.CustomPath, "options", name)
+
+	isDir, err := util.IsDir(customDir)
 	if err != nil {
-		return nil, err
+		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 directories.AddAndGet(name, result), nil
 }
 
-// fileFromOptionsDir is a helper to read files from custom or static path.
+// Locale reads the content of a specific locale from static or custom path.
-func fileFromOptionsDir(elems ...string) ([]byte, error) {
+func Locale(name string) ([]byte, error) {
-	return readLocalFile([]string{setting.CustomPath, setting.StaticRootPath}, "options", elems...)
+	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)
 }
 
 // IsDynamic will return false when using embedded data (-tags bindata)

modules/options/static.go

@@ -8,20 +8,38 @@ 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"
 )
 
-// Dir returns all files from custom directory or bindata.
+var directories = make(directorySet)
+
+// Dir returns all files from bindata or custom directory.
 func Dir(name string) ([]string, error) {
 	if directories.Filled(name) {
 		return directories.Get(name), nil
 	}
 
-	result, err := listLocalDirIfExist([]string{setting.CustomPath}, "options", name)
+	var result []string
+
+	customDir := path.Join(setting.CustomPath, "options", name)
+	isDir, err := util.IsDir(customDir)
 	if err != nil {
-		return nil, err
+		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...)
 	}
 
 	files, err := AssetDir(name)
@@ -51,18 +69,57 @@ func AssetDir(dirName string) ([]string, error) {
 	return results, nil
 }
 
-// fileFromOptionsDir is a helper to read files from custom path or bindata.
+// Locale reads the content of a specific locale from bindata or custom path.
-func fileFromOptionsDir(elems ...string) ([]byte, error) {
+func Locale(name string) ([]byte, error) {
-	// only try custom dir, no static dir
+	return fileFromDir(path.Join("locale", name))
-	if data, err := readLocalFile([]string{setting.CustomPath}, "options", elems...); err == nil {
-		return data, nil
 }
 
-	f, err := Assets.Open(util.PathJoinRelX(elems...))
+// 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)
+	}
+
+	f, err := Assets.Open(name)
 	if err != nil {
 		return nil, err
 	}
 	defer f.Close()
+
 	return io.ReadAll(f)
 }
 

modules/util/path.go

@@ -5,7 +5,6 @@ package util
 
 import (
 	"errors"
-	"fmt"
 	"net/url"
 	"os"
 	"path"
@@ -15,92 +14,13 @@ import (
 	"strings"
 )
 
-// PathJoinRel joins the path elements into a single path, each element is cleaned by path.Clean separately.
+// EnsureAbsolutePath ensure that a path is absolute, making it
-// It only returns the following values (like path.Join), any redundant part (empty, relative dots, slashes) is removed.
+// relative to absoluteBase if necessary
-// It's caller's duty to make every element not bypass its own directly level, to avoid security issues.
+func EnsureAbsolutePath(path, absoluteBase string) string {
-//
+	if filepath.IsAbs(path) {
-//	empty => ``
+		return path
-//	`` => ``
-//	`..` => `.`
-//	`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,

modules/util/path_test.go

@@ -136,77 +136,3 @@ 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)
-	}
-}

web_src/css/repository.css

@@ -1611,12 +1611,14 @@
   padding: 7px 0;
   background: var(--color-body);
   line-height: 30px;
+  height: 47px; /* match .ui.attached.header.diff-file-header.sticky-2nd-row */
 }
 
 @media (max-width: 991px) {
   .repository .diff-detail-box {
     flex-direction: column;
     align-items: flex-start;
+    height: 77px; /* match .ui.attached.header.diff-file-header.sticky-2nd-row */
   }
 }
 
@@ -1672,13 +1674,6 @@
   }
 }
 
-.diff-detail-actions {
-  /* prevent font-size from increasing element height so that .diff-detail-box comes
-     out with height of 47px (one line) and 77px (two lines), which is important for
-     position: sticky */
-  height: 33px;
-}
-
 .repository .diff-detail-box .diff-detail-actions > * {
   margin-right: 0;
 }
@@ -1853,24 +1848,10 @@
   padding-bottom: 5px;
 }
 
-.diff-file-box {
-  border: 1px solid transparent;
-  border-radius: var(--border-radius);
-}
-
-/* TODO: this can potentially be made "global" by removing the class prefix */
-.diff-file-box .ui.attached.header,
-.diff-file-box .ui.attached.table {
-  margin: 0; /* remove fomantic negative margins */;
-  width: initial; /* remove fomantic over 100% width */;
-  max-width: initial; /* remove fomantic over 100% width */;
-}
-
 .repository .diff-stats {
   clear: both;
   margin-bottom: 5px;
-  max-height: 200px;
+  max-height: 400px;
-  height: fit-content;
   overflow: auto;
   padding-left: 0;
 }
@@ -2666,8 +2647,7 @@
   filter: drop-shadow(-3px 0 0 var(--color-primary-alpha-30)) !important;
 }
 
-.code-comment:target,
+.code-comment:target {
-.diff-file-box:target {
   border-color: var(--color-primary) !important;
   border-radius: var(--border-radius) !important;
   box-shadow: 0 0 0 3px var(--color-primary-alpha-30) !important;
@@ -3258,13 +3238,13 @@ td.blob-excerpt {
 }
 
 #diff-file-tree {
-  flex: 0 0 20%;
+  width: 20%;
   max-width: 380px;
   line-height: inherit;
   position: sticky;
   padding-top: 0;
   top: 47px;
-  max-height: calc(100vh - 47px);
+  max-height: calc(100vh - 50px);
   height: 100%;
   overflow-y: auto;
 }

web_src/css/review.css

@@ -308,3 +308,11 @@ a.blob-excerpt:hover {
   width: 72px;
   height: 10px;
 }
+
+.diff-file-box {
+  border-radius: 0.285rem; /* Just like ui.top.attached.header */
+}
+
+.diff-file-box:target {
+  box-shadow: 0 0 0 3px var(--color-accent);
+}