Fix unittest and repo create bug (#33061)

1. `StatDir` was not right, fix the FIXME
2. Clarify the test cases for `IsUsableRepoName`
3. Fix regression bug in `repo-new.ts`

Fix #33060

models/db/name.go

@@ -5,20 +5,13 @@ package db
 
 import (
 	"fmt"
-	"regexp"
 	"strings"
 	"unicode/utf8"
 
 	"code.gitea.io/gitea/modules/util"
 )
 
-var (
+var ErrNameEmpty = util.SilentWrap{Message: "name is empty", Err: util.ErrInvalidArgument}
-	// ErrNameEmpty name is empty error
-	ErrNameEmpty = util.SilentWrap{Message: "name is empty", Err: util.ErrInvalidArgument}
-
-	// AlphaDashDotPattern characters prohibited in a username (anything except A-Za-z0-9_.-)
-	AlphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`)
-)
 
 // ErrNameReserved represents a "reserved name" error.
 type ErrNameReserved struct {
@@ -82,20 +75,20 @@ func (err ErrNameCharsNotAllowed) Unwrap() error {
 
 // IsUsableName checks if name is reserved or pattern of name is not allowed
 // based on given reserved names and patterns.
-// Names are exact match, patterns can be prefix or suffix match with placeholder '*'.
+// Names are exact match, patterns can be a prefix or suffix match with placeholder '*'.
-func IsUsableName(names, patterns []string, name string) error {
+func IsUsableName(reservedNames, reservedPatterns []string, name string) error {
 	name = strings.TrimSpace(strings.ToLower(name))
 	if utf8.RuneCountInString(name) == 0 {
 		return ErrNameEmpty
 	}
 
-	for i := range names {
+	for i := range reservedNames {
-		if name == names[i] {
+		if name == reservedNames[i] {
 			return ErrNameReserved{name}
 		}
 	}
 
-	for _, pat := range patterns {
+	for _, pat := range reservedPatterns {
 		if pat[0] == '*' && strings.HasSuffix(name, pat[1:]) ||
 			(pat[len(pat)-1] == '*' && strings.HasPrefix(name, pat[:len(pat)-1])) {
 			return ErrNamePatternNotAllowed{pat}

models/repo/repo.go

@@ -11,6 +11,7 @@ import (
 	"net"
 	"net/url"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -60,13 +61,15 @@ func (err ErrRepoIsArchived) Error() string {
 }
 
 var (
-	reservedRepoNames    = []string{".", "..", "-"}
+	validRepoNamePattern   = regexp.MustCompile(`[-.\w]+`)
-	reservedRepoPatterns = []string{"*.git", "*.wiki", "*.rss", "*.atom"}
+	invalidRepoNamePattern = regexp.MustCompile(`[.]{2,}`)
+	reservedRepoNames      = []string{".", "..", "-"}
+	reservedRepoPatterns   = []string{"*.git", "*.wiki", "*.rss", "*.atom"}
 )
 
-// IsUsableRepoName returns true when repository is usable
+// IsUsableRepoName returns true when name is usable
 func IsUsableRepoName(name string) error {
-	if db.AlphaDashDotPattern.MatchString(name) {
+	if !validRepoNamePattern.MatchString(name) || invalidRepoNamePattern.MatchString(name) {
 		// Note: usually this error is normally caught up earlier in the UI
 		return db.ErrNameCharsNotAllowed{Name: name}
 	}

models/repo/repo_test.go

@@ -217,3 +217,15 @@ func TestComposeSSHCloneURL(t *testing.T) {
 	setting.SSH.Port = 123
 	assert.Equal(t, "ssh://git@[::1]:123/user/repo.git", ComposeSSHCloneURL("user", "repo"))
 }
+
+func TestIsUsableRepoName(t *testing.T) {
+	assert.NoError(t, IsUsableRepoName("a"))
+	assert.NoError(t, IsUsableRepoName("-1_."))
+	assert.NoError(t, IsUsableRepoName(".profile"))
+
+	assert.Error(t, IsUsableRepoName("-"))
+	assert.Error(t, IsUsableRepoName("🌞"))
+	assert.Error(t, IsUsableRepoName("the..repo"))
+	assert.Error(t, IsUsableRepoName("foo.wiki"))
+	assert.Error(t, IsUsableRepoName("foo.git"))
+}

models/unittest/fscopy.go

@@ -67,7 +67,7 @@ func SyncDirs(srcPath, destPath string) error {
 	}
 
 	// find and delete all untracked files
-	destFiles, err := util.StatDir(destPath, true)
+	destFiles, err := util.ListDirRecursively(destPath, &util.ListDirOptions{IncludeDir: true})
 	if err != nil {
 		return err
 	}
@@ -86,13 +86,13 @@ func SyncDirs(srcPath, destPath string) error {
 	}
 
 	// sync src files to dest
-	srcFiles, err := util.StatDir(srcPath, true)
+	srcFiles, err := util.ListDirRecursively(srcPath, &util.ListDirOptions{IncludeDir: true})
 	if err != nil {
 		return err
 	}
 	for _, srcFile := range srcFiles {
 		destFilePath := filepath.Join(destPath, srcFile)
-		// util.StatDir appends a slash to the directory name
+		// util.ListDirRecursively appends a slash to the directory name
 		if strings.HasSuffix(srcFile, "/") {
 			err = os.MkdirAll(destFilePath, os.ModePerm)
 		} else {

modules/assetfs/layered.go

@@ -103,7 +103,7 @@ func (l *LayeredFS) ReadLayeredFile(elems ...string) ([]byte, string, error) {
 }
 
 func shouldInclude(info fs.FileInfo, fileMode ...bool) bool {
-	if util.CommonSkip(info.Name()) {
+	if util.IsCommonHiddenFileName(info.Name()) {
 		return false
 	}
 	if len(fileMode) == 0 {

modules/repository/init.go

@@ -81,7 +81,7 @@ func LoadRepoConfig() error {
 		if isDir, err := util.IsDir(customPath); err != nil {
 			return fmt.Errorf("failed to check custom %s dir: %w", t, err)
 		} else if isDir {
-			if typeFiles[i].custom, err = util.StatDir(customPath); err != nil {
+			if typeFiles[i].custom, err = util.ListDirRecursively(customPath, &util.ListDirOptions{SkipCommonHiddenNames: true}); err != nil {
 				return fmt.Errorf("failed to list custom %s files: %w", t, err)
 			}
 		}

modules/util/path.go

@@ -140,82 +140,51 @@ func IsExist(path string) (bool, error) {
 	return false, err
 }
 
-func statDir(dirPath, recPath string, includeDir, isDirOnly, followSymlinks bool) ([]string, error) {
+func listDirRecursively(result *[]string, fsDir, recordParentPath string, opts *ListDirOptions) error {
-	dir, err := os.Open(dirPath)
+	dir, err := os.Open(fsDir)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	defer dir.Close()
 
 	fis, err := dir.Readdir(0)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	statList := make([]string, 0)
 	for _, fi := range fis {
-		if CommonSkip(fi.Name()) {
+		if opts.SkipCommonHiddenNames && IsCommonHiddenFileName(fi.Name()) {
 			continue
 		}
-
+		relPath := path.Join(recordParentPath, fi.Name())
-		relPath := path.Join(recPath, fi.Name())
+		curPath := filepath.Join(fsDir, fi.Name())
-		curPath := path.Join(dirPath, fi.Name())
 		if fi.IsDir() {
-			if includeDir {
+			if opts.IncludeDir {
-				statList = append(statList, relPath+"/")
+				*result = append(*result, relPath+"/")
 			}
-			s, err := statDir(curPath, relPath, includeDir, isDirOnly, followSymlinks)
+			if err = listDirRecursively(result, curPath, relPath, opts); err != nil {
-			if err != nil {
+				return err
-				return nil, err
-			}
-			statList = append(statList, s...)
-		} else if !isDirOnly {
-			statList = append(statList, relPath)
-		} else if followSymlinks && fi.Mode()&os.ModeSymlink != 0 {
-			link, err := os.Readlink(curPath)
-			if err != nil {
-				return nil, err
-			}
-
-			isDir, err := IsDir(link)
-			if err != nil {
-				return nil, err
-			}
-			if isDir {
-				if includeDir {
-					statList = append(statList, relPath+"/")
-				}
-				s, err := statDir(curPath, relPath, includeDir, isDirOnly, followSymlinks)
-				if err != nil {
-					return nil, err
-				}
-				statList = append(statList, s...)
 			}
+		} else {
+			*result = append(*result, relPath)
 		}
 	}
-	return statList, nil
+	return nil
 }
 
-// StatDir gathers information of given directory by depth-first.
+type ListDirOptions struct {
-// It returns slice of file list and includes subdirectories if enabled;
+	IncludeDir            bool // subdirectories are also included with suffix slash
-// it returns error and nil slice when error occurs in underlying functions,
+	SkipCommonHiddenNames bool
-// or given path is not a directory or does not exist.
+}
-//
-// Slice does not include given path itself.
-// If subdirectories is enabled, they will have suffix '/'.
-// FIXME: it doesn't like dot-files, for example: "owner/.profile.git"
-func StatDir(rootPath string, includeDir ...bool) ([]string, error) {
-	if isDir, err := IsDir(rootPath); err != nil {
-		return nil, err
-	} else if !isDir {
-		return nil, errors.New("not a directory or does not exist: " + rootPath)
-	}
 
-	isIncludeDir := false
+// ListDirRecursively gathers information of given directory by depth-first.
-	if len(includeDir) != 0 {
+// The paths are always in "dir/slash/file" format (not "\\" even in Windows)
-		isIncludeDir = includeDir[0]
+// Slice does not include given path itself.
+func ListDirRecursively(rootDir string, opts *ListDirOptions) (res []string, err error) {
+	if err = listDirRecursively(&res, rootDir, "", opts); err != nil {
+		return nil, err
 	}
-	return statDir(rootPath, "", isIncludeDir, false, false)
+	return res, nil
 }
 
 func isOSWindows() bool {
@@ -266,8 +235,8 @@ func HomeDir() (home string, err error) {
 	return home, nil
 }
 
-// CommonSkip will check a provided name to see if it represents file or directory that should not be watched
+// IsCommonHiddenFileName will check a provided name to see if it represents file or directory that should not be watched
-func CommonSkip(name string) bool {
+func IsCommonHiddenFileName(name string) bool {
 	if name == "" {
 		return true
 	}
@@ -276,9 +245,9 @@ func CommonSkip(name string) bool {
 	case '.':
 		return true
 	case 't', 'T':
-		return name[1:] == "humbs.db"
+		return name[1:] == "humbs.db" // macOS
 	case 'd', 'D':
-		return name[1:] == "esktop.ini"
+		return name[1:] == "esktop.ini" // Windows
 	}
 
 	return false

modules/util/path_test.go

@@ -5,10 +5,12 @@ package util
 
 import (
 	"net/url"
+	"os"
 	"runtime"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestFileURLToPath(t *testing.T) {
@@ -210,3 +212,21 @@ func TestCleanPath(t *testing.T) {
 		assert.Equal(t, c.expected, FilePathJoinAbs(c.elems[0], c.elems[1:]...), "case: %v", c.elems)
 	}
 }
+
+func TestListDirRecursively(t *testing.T) {
+	tmpDir := t.TempDir()
+	_ = os.WriteFile(tmpDir+"/.config", nil, 0o644)
+	_ = os.Mkdir(tmpDir+"/d1", 0o755)
+	_ = os.WriteFile(tmpDir+"/d1/f-d1", nil, 0o644)
+	_ = os.Mkdir(tmpDir+"/d1/s1", 0o755)
+	_ = os.WriteFile(tmpDir+"/d1/s1/f-d1s1", nil, 0o644)
+	_ = os.Mkdir(tmpDir+"/d2", 0o755)
+
+	res, err := ListDirRecursively(tmpDir, &ListDirOptions{IncludeDir: true})
+	require.NoError(t, err)
+	assert.ElementsMatch(t, []string{".config", "d1/", "d1/f-d1", "d1/s1/", "d1/s1/f-d1s1", "d2/"}, res)
+
+	res, err = ListDirRecursively(tmpDir, &ListDirOptions{SkipCommonHiddenNames: true})
+	require.NoError(t, err)
+	assert.ElementsMatch(t, []string{"d1/f-d1", "d1/s1/f-d1s1"}, res)
+}

web_src/js/features/repo-new.ts

@@ -11,7 +11,8 @@ export function initRepoNew() {
   const updateUiAutoInit = () => {
     inputAutoInit.checked = Boolean(inputGitIgnores.value || inputLicense.value);
   };
-  form.addEventListener('change', updateUiAutoInit);
+  inputGitIgnores.addEventListener('change', updateUiAutoInit);
+  inputLicense.addEventListener('change', updateUiAutoInit);
   updateUiAutoInit();
 
   const inputRepoName = form.querySelector<HTMLInputElement>('input[name="repo_name"]');