Various wiki bug fixes (#2996)

* Update macaron

* Various wiki bug fixes

4 files changed, 208 insertions, 58 deletions

diff --git a/models/error.go b/models/error.go
index 9df419aee8..7ea4e9e2f2 100644
--- a/models/error.go
+++ b/models/error.go
@@ -191,7 +191,7 @@ type ErrWikiAlreadyExist struct {
 	Title string
 }
 
-// IsErrWikiAlreadyExist checks if an error is a ErrWikiAlreadyExist.
+// IsErrWikiAlreadyExist checks if an error is an ErrWikiAlreadyExist.
 func IsErrWikiAlreadyExist(err error) bool {
 	_, ok := err.(ErrWikiAlreadyExist)
 	return ok
@@ -201,6 +201,21 @@ func (err ErrWikiAlreadyExist) Error() string {
 	return fmt.Sprintf("wiki page already exists [title: %s]", err.Title)
 }
 
+// ErrWikiReservedName represents a reserved name error.
+type ErrWikiReservedName struct {
+	Title string
+}
+
+// IsErrWikiReservedName checks if an error is an ErrWikiReservedName.
+func IsErrWikiReservedName(err error) bool {
+	_, ok := err.(ErrWikiReservedName)
+	return ok
+}
+
+func (err ErrWikiReservedName) Error() string {
+	return fmt.Sprintf("wiki title is reserved: %s", err.Title)
+}
+
 // __________     ___.   .__  .__          ____  __.
 // \______   \__ _\_ |__ |  | |__| ____   |    |/ _|____ ___.__.
 //  |     ___/  |  \ __ \|  | |  |/ ___\  |      <_/ __ <   |  |
diff --git a/models/twofactor.go b/models/twofactor.go
index f54c8d1a22..86718b4cda 100644
--- a/models/twofactor.go
+++ b/models/twofactor.go
@@ -61,7 +61,7 @@ func (t *TwoFactor) getEncryptionKey() []byte {
 
 // SetSecret sets the 2FA secret.
 func (t *TwoFactor) SetSecret(secret string) error {
-	secretBytes, err := com.AESEncrypt(t.getEncryptionKey(), []byte(secret))
+	secretBytes, err := com.AESGCMEncrypt(t.getEncryptionKey(), []byte(secret))
 	if err != nil {
 		return err
 	}
@@ -75,7 +75,7 @@ func (t *TwoFactor) ValidateTOTP(passcode string) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	secret, err := com.AESDecrypt(t.getEncryptionKey(), decodedStoredSecret)
+	secret, err := com.AESGCMDecrypt(t.getEncryptionKey(), decodedStoredSecret)
 	if err != nil {
 		return false, err
 	}
diff --git a/models/wiki.go b/models/wiki.go
index a4a9cb9dcb..3819eb95f0 100644
--- a/models/wiki.go
+++ b/models/wiki.go
@@ -22,22 +22,37 @@ import (
 )
 
 var (
-	reservedWikiPaths = []string{"_pages", "_new", "_edit"}
+	reservedWikiNames = []string{"_pages", "_new", "_edit"}
 	wikiWorkingPool   = sync.NewExclusivePool()
 )
 
-// ToWikiPageURL formats a string to corresponding wiki URL name.
-func ToWikiPageURL(name string) string {
+// NormalizeWikiName normalizes a wiki name
+func NormalizeWikiName(name string) string {
+	return strings.Replace(name, "-", " ", -1)
+}
+
+// WikiNameToSubURL converts a wiki name to its corresponding sub-URL.
+func WikiNameToSubURL(name string) string {
 	return url.QueryEscape(strings.Replace(name, " ", "-", -1))
 }
 
-// ToWikiPageName formats a URL back to corresponding wiki page name,
-// and removes leading characters './' to prevent changing files
-// that are not belong to wiki repository.
-func ToWikiPageName(urlString string) string {
-	name, _ := url.QueryUnescape(strings.Replace(urlString, "-", " ", -1))
-	name = strings.Replace(name, "\t", " ", -1)
-	return strings.Replace(strings.TrimLeft(name, "./"), "/", " ", -1)
+// WikiNameToFilename converts a wiki name to its corresponding filename.
+func WikiNameToFilename(name string) string {
+	name = strings.Replace(name, " ", "-", -1)
+	return url.QueryEscape(name) + ".md"
+}
+
+// WikiFilenameToName converts a wiki filename to its corresponding page name.
+func WikiFilenameToName(filename string) (string, error) {
+	if !strings.HasSuffix(filename, ".md") {
+		return "", fmt.Errorf("Invalid wiki filename: %s", filename)
+	}
+	basename := filename[:len(filename)-3]
+	unescaped, err := url.QueryUnescape(basename)
+	if err != nil {
+		return "", err
+	}
+	return NormalizeWikiName(unescaped), nil
 }
 
 // WikiCloneLink returns clone URLs of repository wiki.
@@ -81,7 +96,7 @@ func (repo *Repository) LocalWikiPath() string {
 }
 
 // UpdateLocalWiki makes sure the local copy of repository wiki is up-to-date.
-func (repo *Repository) UpdateLocalWiki() error {
+func (repo *Repository) updateLocalWiki() error {
 	// Don't pass branch name here because it fails to clone and
 	// checkout to a specific branch when wiki is an empty repository.
 	var branch = ""
@@ -95,19 +110,19 @@ func discardLocalWikiChanges(localPath string) error {
 	return discardLocalRepoBranchChanges(localPath, "master")
 }
 
-// pathAllowed checks if a wiki path is allowed
-func pathAllowed(path string) error {
-	for i := range reservedWikiPaths {
-		if path == reservedWikiPaths[i] {
-			return ErrWikiAlreadyExist{path}
+// nameAllowed checks if a wiki name is allowed
+func nameAllowed(name string) error {
+	for _, reservedName := range reservedWikiNames {
+		if name == reservedName {
+			return ErrWikiReservedName{name}
 		}
 	}
 	return nil
 }
 
-// updateWikiPage adds new page to repository wiki.
-func (repo *Repository) updateWikiPage(doer *User, oldWikiPath, wikiPath, content, message string, isNew bool) (err error) {
-	if err = pathAllowed(wikiPath); err != nil {
+// updateWikiPage adds a new page to the repository wiki.
+func (repo *Repository) updateWikiPage(doer *User, oldWikiName, newWikiName, content, message string, isNew bool) (err error) {
+	if err = nameAllowed(newWikiName); err != nil {
 		return err
 	}
 
@@ -121,23 +136,21 @@ func (repo *Repository) updateWikiPage(doer *User, oldWikiPath, wikiPath, conten
 	localPath := repo.LocalWikiPath()
 	if err = discardLocalWikiChanges(localPath); err != nil {
 		return fmt.Errorf("discardLocalWikiChanges: %v", err)
-	} else if err = repo.UpdateLocalWiki(); err != nil {
+	} else if err = repo.updateLocalWiki(); err != nil {
 		return fmt.Errorf("UpdateLocalWiki: %v", err)
 	}
 
-	title := ToWikiPageName(wikiPath)
-	filename := path.Join(localPath, wikiPath+".md")
+	newWikiPath := path.Join(localPath, WikiNameToFilename(newWikiName))
 
 	// If not a new file, show perform update not create.
 	if isNew {
-		if com.IsExist(filename) {
-			return ErrWikiAlreadyExist{filename}
+		if com.IsExist(newWikiPath) {
+			return ErrWikiAlreadyExist{newWikiPath}
 		}
 	} else {
-		file := path.Join(localPath, oldWikiPath+".md")
-
-		if err := os.Remove(file); err != nil {
-			return fmt.Errorf("Failed to remove %s: %v", file, err)
+		oldWikiPath := path.Join(localPath, WikiNameToFilename(oldWikiName))
+		if err := os.Remove(oldWikiPath); err != nil {
+			return fmt.Errorf("Failed to remove %s: %v", oldWikiPath, err)
 		}
 	}
 
@@ -146,15 +159,16 @@ func (repo *Repository) updateWikiPage(doer *User, oldWikiPath, wikiPath, conten
 	// as a new page operation.
 	// So we want to make sure the symlink is removed before write anything.
 	// The new file we created will be in normal text format.
+	if err = os.RemoveAll(newWikiPath); err != nil {
+		return err
+	}
 
-	_ = os.Remove(filename)
-
-	if err = ioutil.WriteFile(filename, []byte(content), 0666); err != nil {
+	if err = ioutil.WriteFile(newWikiPath, []byte(content), 0666); err != nil {
 		return fmt.Errorf("WriteFile: %v", err)
 	}
 
 	if len(message) == 0 {
-		message = "Update page '" + title + "'"
+		message = "Update page '" + newWikiName + "'"
 	}
 	if err = git.AddChanges(localPath, true); err != nil {
 		return fmt.Errorf("AddChanges: %v", err)
@@ -174,36 +188,35 @@ func (repo *Repository) updateWikiPage(doer *User, oldWikiPath, wikiPath, conten
 }
 
 // AddWikiPage adds a new wiki page with a given wikiPath.
-func (repo *Repository) AddWikiPage(doer *User, wikiPath, content, message string) error {
-	return repo.updateWikiPage(doer, "", wikiPath, content, message, true)
+func (repo *Repository) AddWikiPage(doer *User, wikiName, content, message string) error {
+	return repo.updateWikiPage(doer, "", wikiName, content, message, true)
 }
 
 // EditWikiPage updates a wiki page identified by its wikiPath,
 // optionally also changing wikiPath.
-func (repo *Repository) EditWikiPage(doer *User, oldWikiPath, wikiPath, content, message string) error {
-	return repo.updateWikiPage(doer, oldWikiPath, wikiPath, content, message, false)
+func (repo *Repository) EditWikiPage(doer *User, oldWikiName, newWikiName, content, message string) error {
+	return repo.updateWikiPage(doer, oldWikiName, newWikiName, content, message, false)
 }
 
-// DeleteWikiPage deletes a wiki page identified by its wikiPath.
-func (repo *Repository) DeleteWikiPage(doer *User, wikiPath string) (err error) {
+// DeleteWikiPage deletes a wiki page identified by its path.
+func (repo *Repository) DeleteWikiPage(doer *User, wikiName string) (err error) {
 	wikiWorkingPool.CheckIn(com.ToStr(repo.ID))
 	defer wikiWorkingPool.CheckOut(com.ToStr(repo.ID))
 
 	localPath := repo.LocalWikiPath()
 	if err = discardLocalWikiChanges(localPath); err != nil {
 		return fmt.Errorf("discardLocalWikiChanges: %v", err)
-	} else if err = repo.UpdateLocalWiki(); err != nil {
+	} else if err = repo.updateLocalWiki(); err != nil {
 		return fmt.Errorf("UpdateLocalWiki: %v", err)
 	}
 
-	filename := path.Join(localPath, wikiPath+".md")
+	filename := path.Join(localPath, WikiNameToFilename(wikiName))
 
 	if err := os.Remove(filename); err != nil {
 		return fmt.Errorf("Failed to remove %s: %v", filename, err)
 	}
 
-	title := ToWikiPageName(wikiPath)
-	message := "Delete page '" + title + "'"
+	message := "Delete page '" + wikiName + "'"
 
 	if err = git.AddChanges(localPath, true); err != nil {
 		return fmt.Errorf("AddChanges: %v", err)
diff --git a/models/wiki_test.go b/models/wiki_test.go
index ed914d1e55..e80c6cbb8d 100644
--- a/models/wiki_test.go
+++ b/models/wiki_test.go
@@ -5,24 +5,91 @@
 package models
 
 import (
+	"path"
 	"path/filepath"
 	"testing"
 
 	"code.gitea.io/gitea/modules/setting"
 
+	"github.com/Unknwon/com"
 	"github.com/stretchr/testify/assert"
 )
 
-func TestToWikiPageURL(t *testing.T) {
-	assert.Equal(t, "wiki-name", ToWikiPageURL("wiki-name"))
-	assert.Equal(t, "wiki-name-with-many-spaces", ToWikiPageURL("wiki name with many spaces"))
+func TestNormalizeWikiName(t *testing.T) {
+	type test struct {
+		Expected string
+		WikiName string
+	}
+	for _, test := range []test{
+		{"wiki name", "wiki name"},
+		{"wiki name", "wiki-name"},
+		{"name with/slash", "name with/slash"},
+		{"name with%percent", "name-with%percent"},
+		{"%2F", "%2F"},
+	} {
+		assert.Equal(t, test.Expected, NormalizeWikiName(test.WikiName))
+	}
 }
 
-func TestToWikiPageName(t *testing.T) {
-	assert.Equal(t, "wiki name", ToWikiPageName("wiki name"))
-	assert.Equal(t, "wiki name", ToWikiPageName("wiki-name"))
-	assert.Equal(t, "wiki name", ToWikiPageName("wiki\tname"))
-	assert.Equal(t, "wiki name", ToWikiPageName("./.././wiki/name"))
+func TestWikiNameToFilename(t *testing.T) {
+	type test struct {
+		Expected string
+		WikiName string
+	}
+	for _, test := range []test{
+		{"wiki-name.md", "wiki name"},
+		{"wiki-name.md", "wiki-name"},
+		{"name-with%2Fslash.md", "name with/slash"},
+		{"name-with%25percent.md", "name with%percent"},
+	} {
+		assert.Equal(t, test.Expected, WikiNameToFilename(test.WikiName))
+	}
+}
+
+func TestWikiNameToSubURL(t *testing.T) {
+	type test struct {
+		Expected string
+		WikiName string
+	}
+	for _, test := range []test{
+		{"wiki-name", "wiki name"},
+		{"wiki-name", "wiki-name"},
+		{"name-with%2Fslash", "name with/slash"},
+		{"name-with%25percent", "name with%percent"},
+	} {
+		assert.Equal(t, test.Expected, WikiNameToSubURL(test.WikiName))
+	}
+}
+
+func TestWikiFilenameToName(t *testing.T) {
+	type test struct {
+		Expected string
+		Filename string
+	}
+	for _, test := range []test{
+		{"hello world", "hello-world.md"},
+		{"symbols/?*", "symbols%2F%3F%2A.md"},
+	} {
+		name, err := WikiFilenameToName(test.Filename)
+		assert.NoError(t, err)
+		assert.Equal(t, test.Expected, name)
+	}
+}
+
+func TestWikiNameToFilenameToName(t *testing.T) {
+	// converting from wiki name to filename, then back to wiki name should
+	// return the original (normalized) name
+	for _, name := range []string{
+		"wiki-name",
+		"wiki name",
+		"wiki name with/slash",
+		"$$$%%%^^&&!@#$(),.<>",
+	} {
+		filename := WikiNameToFilename(name)
+		resultName, err := WikiFilenameToName(filename)
+		assert.NoError(t, err)
+		assert.Equal(t, NormalizeWikiName(name), resultName)
+	}
 }
 
 func TestRepository_WikiCloneLink(t *testing.T) {
@@ -47,17 +114,72 @@ func TestRepository_WikiPath(t *testing.T) {
 	assert.Equal(t, expected, repo.WikiPath())
 }
 
-// TODO TestRepository_HasWiki
+func TestRepository_HasWiki(t *testing.T) {
+	prepareTestEnv(t)
+	repo1 := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
+	assert.True(t, repo1.HasWiki())
+	repo2 := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository)
+	assert.False(t, repo2.HasWiki())
+}
+
+func TestRepository_InitWiki(t *testing.T) {
+	prepareTestEnv(t)
+	// repo1 already has a wiki
+	repo1 := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
+	assert.NoError(t, repo1.InitWiki())
 
-// TODO TestRepository_InitWiki
+	// repo2 does not already have a wiki
+	repo2 := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository)
+	assert.NoError(t, repo2.InitWiki())
+	assert.True(t, repo2.HasWiki())
+}
 
 func TestRepository_LocalWikiPath(t *testing.T) {
-	assert.NoError(t, PrepareTestDatabase())
+	prepareTestEnv(t)
 	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
 	expected := filepath.Join(setting.AppDataPath, "tmp/local-wiki/1")
 	assert.Equal(t, expected, repo.LocalWikiPath())
 }
 
-// TODO TestRepository_UpdateLocalWiki
+func TestRepository_AddWikiPage(t *testing.T) {
+	const wikiContent = "This is the wiki content"
+	const commitMsg = "Commit message"
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
+	doer := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+	for _, wikiName := range []string{
+		"Another page",
+		"Here's a <tag> and a/slash",
+	} {
+		prepareTestEnv(t)
+		assert.NoError(t, repo.AddWikiPage(doer, wikiName, wikiContent, commitMsg))
+		expectedPath := path.Join(repo.LocalWikiPath(), WikiNameToFilename(wikiName))
+		assert.True(t, com.IsExist(expectedPath))
+	}
+}
+
+func TestRepository_EditWikiPage(t *testing.T) {
+	const newWikiContent = "This is the new content"
+	const commitMsg = "Commit message"
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
+	doer := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+	for _, newWikiName := range []string{
+		"New home",
+		"New/name/with/slashes",
+	} {
+		prepareTestEnv(t)
+		assert.NoError(t, repo.EditWikiPage(doer, "Home", newWikiName, newWikiContent, commitMsg))
+		newPath := path.Join(repo.LocalWikiPath(), WikiNameToFilename(newWikiName))
+		assert.True(t, com.IsExist(newPath))
+		oldPath := path.Join(repo.LocalWikiPath(), "Home.md")
+		assert.False(t, com.IsExist(oldPath))
+	}
+}
 
-// TODO ... (all remaining untested functions)
+func TestRepository_DeleteWikiPage(t *testing.T) {
+	prepareTestEnv(t)
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
+	doer := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+	assert.NoError(t, repo.DeleteWikiPage(doer, "Home"))
+	wikiPath := path.Join(repo.LocalWikiPath(), "Home.md")
+	assert.False(t, com.IsExist(wikiPath))
+}