Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gitea wiki repo uses urlencoded file name when creating new pages online #16122

Open
2 of 5 tasks
BLumia opened this issue Jun 9, 2021 · 5 comments
Open
2 of 5 tasks
Labels

Comments

@BLumia
Copy link
Member

BLumia commented Jun 9, 2021

Description

When creating a new wiki page online, it will generate a urlencoded file name for that page to the wiki repo, instead of using the page title as-is, which can make local management (i.e. clone the wiki repo locally and use it as a regular git repo to manage documentation or notes) vary hard.

Not sure if that's intended, but for reference, GitHub, instead, will (at least try to) use the wiki page title as-is as the name of the new file.


List of related issues that need to be discussed/fixed

  • wiki page with non-urlencoded filename cannot be edited
  • wiki page with non-urlencoded filename cannot be deleted
  • wiki page contains space cannot be viewed from the Gitea wiki webpage (e.g. Test Test.md)
  • when can we drop the backward compatibility so we can remove the urlencoded behavior?
  • once we no longer use urlencoded page, how to deal with pages that contain / character?
@lunny lunny added the type/enhancement An improvement of existing functionality label Jun 10, 2021
@lunny
Copy link
Member

lunny commented Jun 10, 2021

This is indeed a problem.

@BLumia
Copy link
Member Author

BLumia commented Jun 11, 2021

While I try to write a patch for this issue, I found there is a bug related to this issue.

If you created a non-urlencoded markdown file and push it to the wiki repo, the file name can be displayed correctly, but if you edit that page online and save it with the save button, it will create a new page with urlencoded file name, and the old non-urlencoded file will still exist.

My patch is here but I haven't tested it due to some other issue I've got, anyway here is the patch. be aware it's NOT TESTED

diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go
index 75b9d1d1f..dbf2771b3 100644
--- a/services/wiki/wiki.go
+++ b/services/wiki/wiki.go
@@ -44,6 +44,11 @@ func NormalizeWikiName(name string) string {
 }
 
 // NameToFilename converts a wiki name to its corresponding filename.
+func NameToUnescapedFilename(name string) string {
+	name = strings.ReplaceAll(name, " ", "-")
+	return name + ".md"
+}
+
 func NameToFilename(name string) string {
 	name = strings.ReplaceAll(name, " ", "-")
 	return url.QueryEscape(name) + ".md"
@@ -81,6 +86,19 @@ func InitWiki(repo *models.Repository) error {
 	return nil
 }
 
+func CheckFileExistence(gitRepo *git.Repository, filePath string) (bool, error) {
+	filesInIndex, err := gitRepo.LsFiles(filePath)
+	if err != nil {
+		log.Error("%v", err)
+		return false, err
+	}
+	if util.IsStringInSlice(filePath, filesInIndex) {
+		return true, nil
+	}
+
+	return false, nil
+}
+
 // updateWikiPage adds a new page to the repository wiki.
 func updateWikiPage(doer *models.User, repo *models.Repository, oldWikiName, newWikiName, content, message string, isNew bool) (err error) {
 	if err = nameAllowed(newWikiName); err != nil {
@@ -133,27 +151,40 @@ func updateWikiPage(doer *models.User, repo *models.Repository, oldWikiName, new
 		}
 	}
 
-	newWikiPath := NameToFilename(newWikiName)
+	newWikiPath := NameToUnescapedFilename(newWikiName)
+	isWikiExist, err := CheckFileExistence(gitRepo, newWikiName)
+	if err != nil {
+		return err
+	}
+	if !isWikiExist {
+		newWikiPath = NameToFilename(newWikiName)
+	}
+
 	if isNew {
-		filesInIndex, err := gitRepo.LsFiles(newWikiPath)
+		isNewWikiExist, err := CheckFileExistence(gitRepo, newWikiPath)
 		if err != nil {
-			log.Error("%v", err)
 			return err
 		}
-		if util.IsStringInSlice(newWikiPath, filesInIndex) {
+		if isNewWikiExist {
 			return models.ErrWikiAlreadyExist{
 				Title: newWikiPath,
 			}
 		}
 	} else {
-		oldWikiPath := NameToFilename(oldWikiName)
-		filesInIndex, err := gitRepo.LsFiles(oldWikiPath)
+		oldWikiPath := NameToUnescapedFilename(oldWikiName)
+		isOldWikiExist, err := CheckFileExistence(gitRepo, newWikiName)
 		if err != nil {
-			log.Error("%v", err)
 			return err
 		}
+		if !isOldWikiExist {
+			oldWikiPath = NameToFilename(newWikiName)
+			isOldWikiExist, err = CheckFileExistence(gitRepo, newWikiName)
+			if err != nil {
+				return err
+			}
+		}
 
-		if util.IsStringInSlice(oldWikiPath, filesInIndex) {
+		if isOldWikiExist {
 			err := gitRepo.RemoveFilesFromIndex(oldWikiPath)
 			if err != nil {
 				log.Error("%v", err)

This patch will try to find if there is a non-urlencoded file in the repo and try to update that file if there is, which may fixes the bug mention in this comment. This will of course not fix the main issue since it will not try to use the wiki page name as-is as the newly created file name. Using the page name as-is by default may probably break already existed wikis so I'm not sure about the right way to do it.

Edit: the patch in this comment is not correct, a patch tested on my laptop is sent via PR, see #16139

@BLumia
Copy link
Member Author

BLumia commented Jul 26, 2021

I updated the description to match the main point of this issue.

While working on the third issue (wiki page contains space cannot be viewed from the Gitea wiki webpage), I found it seems harder to fix. I cannot find a way to fix this issue and also keep the old behavior not break. Any suggestions?

To help others reproduce that issue, you can:

  1. Clone the wiki repo locally
  2. Create a file names test test.md, commit it and push it to the wiki repo
  3. Browse the file online.

@BLumia
Copy link
Member Author

BLumia commented Jul 28, 2021

I tested GitHub Wiki's behavior for reference:

When creating wiki page online (with Markdown chose as its format):

Wiki page name entered online Created file name in wiki repo Page URL (last part only) Page title displayed online
Wiki Page Wiki-Page.md Wiki-Page Wiki Page
学习 C++ 学习-C--.md 学习-C-- 学习 C
Chrome / Chromium Chrome---Chromium.md Chrome---Chromium Chrome Chromium
安装指南 安装指南.md 安装指南 安装指南
Page-contains-dash&% Page-contains-dash&%.md Page-contains-dash&%25 Page contains dash&%
. ..md ..md ..md

Space( ) got converted to dash(-), and it seems characters unsupported by GitHub Wiki (+ and /, and maybe others listed in their doc) will also get converted to dash(-) and will no longer exist on the page title. Percent(%) will become %25 in URL but it will still be % in the generated file name.

When creating wiki page locally, then commit and push it to wiki repo:

Wiki file name created locally Page URL (last part only) Page title displayed online
KDE Connect.md KDE-Connect KDE Connect
掌握 C++.md 掌握-C-- 掌握 C++
Test%20test+test test.md Test%2520test-test-test Test%20test+test test

Page URL seems weird but the page title can still be displayed correctly.

When editing a non-regular naming page, let's assume there is a file names 学习 C++.md in the wiki repo, once we edit that page, the file will get renamed to 学习 C--.md since the plus(+) got converted to dash(-), and as the result, the page display name will become 学习 C after that.

@BLumia
Copy link
Member Author

BLumia commented Jul 30, 2021

Some additional tests:

Create 学习 C.md学习-C.md学习 C--.md in the repo, according to the previous comment, their URL would be 学习-C学习-C and 学习-C--(first two URL are the same, and both of these pages display with the name 学习 C).

  • If we try to visit the URL ends with 学习-C: 学习 C.md content will be shown instead of 学习-C.md
  • If we try to edit the page with URL 学习-C-- and leave the title field unchanged (学习 C ): An error will be shown says Cannot rename, target already exists.

BTW, I don't think GitHub Wiki dees are good since users won't be able to use some characters in a wiki title which can be confused. A page with title names 学习 C++ will be not possible, for example. I think these need to be discussed before working on future bug fixes related to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants