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

redirect .wiki/* ui link to /wiki #18831

Merged
merged 12 commits into from
Mar 23, 2022
Merged

Conversation

a1012112796
Copy link
Member

fix #18590

fix go-gitea#18590

Signed-off-by: a1012112796 <1012112796@qq.com>
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Feb 23, 2022
@@ -440,6 +440,12 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) {
ctx.Repo.Owner = owner
ctx.Data["Username"] = ctx.Repo.Owner.Name

// redirect link to wiki
if strings.HasSuffix(repoName, ".wiki") {
ctx.Redirect(strings.Replace(ctx.Req.RequestURI, ".wiki", "/wiki", 1))
Copy link
Member

@silverwind silverwind Feb 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would break on https://foo.wiki/user/repo.wiki?query=foo.wiki. Would at least include the repo name in the replacement, or properly parse the URL apart and join it again after replacement on the path.

Signed-off-by: a1012112796 <1012112796@qq.com>
* main:
  ignore missing comment for user notifications (go-gitea#18954)
  allow overwrite artifacts for github releases (go-gitea#18987)
  fix & refactor (go-gitea#18973)
  Don't clean up hardcoded `tmp` (go-gitea#18983)
  git backend ignore replace objects (go-gitea#18979)
  Improve the deletion of issue (go-gitea#18945)
  Add note to GPG key response if user has no keys (go-gitea#18961)
  adds restore docs for docker based instances (go-gitea#18844)
  Refactor admin user filter query parameters (go-gitea#18965)
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 4, 2022
modules/context/repo.go Outdated Show resolved Hide resolved
@zeripath
Copy link
Contributor

@wxiaoguang @a1012112796 I've changed the redirect code to more closely match

// RedirectToUser redirect to a differently-named user
func RedirectToUser(ctx *Context, userName string, redirectUserID int64) {
user, err := user_model.GetUserByID(redirectUserID)
if err != nil {
ctx.ServerError("GetUserByID", err)
return
}
redirectPath := strings.Replace(
ctx.Req.URL.EscapedPath(),
url.PathEscape(userName),
url.PathEscape(user.Name),
1,
)
if ctx.Req.URL.RawQuery != "" {
redirectPath += "?" + ctx.Req.URL.RawQuery
}
ctx.Redirect(path.Join(setting.AppSubURL, redirectPath), http.StatusTemporaryRedirect)
}

This now works on suburl mounted giteas too.

modules/context/repo.go Outdated Show resolved Hide resolved
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems correct.

I have a small question that could we just use redirectPath := url.PathEscape(userName)+"/"+url.PathEscape(mainRepoName)+"/wiki" + what_ever? Then we do not need the replace.

Maybe like this?

	// redirect link to from "/org/repo.wiki" to "/org/repo/wiki"
	if strings.HasSuffix(repoName, ".wiki") {
		// ctx.Req.URL.Path does not have the preceding appSubURL - any redirect must have this added
		// Now we happen to know that all of our paths are: /:username/:reponame/whatever_else
		pathFields := strings.SplitN(ctx.Req.URL.EscapedPath(), "/", 4)
		redirectUri := "/" + pathFields[1] + "/" + strings.TrimSuffix(pathFields[2], ".wiki") + "/wiki"
		if len(pathFields) == 4 {
			redirectUri += "/" + pathFields[3]
		}
		if ctx.Req.URL.RawQuery != "" {
			redirectUri += "?" + ctx.Req.URL.RawQuery
		}
		ctx.Redirect(path.Join(setting.AppSubURL, redirectUri))
		return
	}

@zeripath
Copy link
Contributor

So I initially considered that but decided that it would be better to keep the same code style as the RedirectToUser.

@wxiaoguang
Copy link
Contributor

Either is fine to me, already approved. (while personally I would choose a straight way if it's better than the old method)

@zeripath
Copy link
Contributor

make lgtm work

@zeripath zeripath merged commit d8f5784 into go-gitea:main Mar 23, 2022
zeripath added a commit to zeripath/gitea that referenced this pull request Mar 23, 2022
Backport go-gitea#18831

Redirect .wiki/* ui link to /wiki

fix go-gitea#18590

Signed-off-by: a1012112796 <1012112796@qq.com>
Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added the backport/done All backports for this PR have been created label Mar 23, 2022
@zeripath
Copy link
Contributor

Either is fine to me, already approved. (while personally I would choose a straight way if it's better than the old method)

TBH I didn't feel that either was much better. They're both making pretty bad assumptions about the structure of the urls. Ideally we need some way of getting chi to do the replacement instead - chi's router is the thing that knows which thing is matching the :reponame parameter here.

@wxiaoguang
Copy link
Contributor

Maybe it's worthy to introduce a function RedirectToOrgRepo(org, repo, extra, query) to replace RedirectToUser and this code.

zeripath added a commit that referenced this pull request Mar 23, 2022
Backport #18831

Redirect .wiki/* ui link to /wiki

fix #18590

Signed-off-by: a1012112796 <1012112796@qq.com>
Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: a1012112796 <1012112796@qq.com>
zeripath added a commit to zeripath/gitea that referenced this pull request Mar 23, 2022
 ## [1.16.5](https://github.com/go-gitea/gitea/releases/tag/1.16.5) - 2022-03-23

* BREAKING
  * Bump to build with go1.18 (go-gitea#19120 et al) (go-gitea#19127)
* SECURITY
  * Prevent redirect to Host (2) (go-gitea#19175) (go-gitea#19186)
  * Try to prevent autolinking of displaynames by email readers (go-gitea#19169) (go-gitea#19183)
  * Clean paths when looking in Storage (go-gitea#19124) (go-gitea#19179)
  * Do not send notification emails to inactive users (go-gitea#19131) (go-gitea#19139)
  * Do not send activation email if manual confirm is set (go-gitea#19119) (go-gitea#19122)
* ENHANCEMENTS
  * Use the new/choose link for New Issue on project page (go-gitea#19172) (go-gitea#19176)
* BUGFIXES
  * Fix compare link in active feeds for new branch (go-gitea#19149) (go-gitea#19185)
  * Redirect .wiki/* ui link to /wiki (go-gitea#18831) (go-gitea#19184)
  * Ensure deploy keys with write access can push (go-gitea#19010) (go-gitea#19182)
  * Ensure that setting.LocalURL always has a trailing slash (go-gitea#19171) (go-gitea#19177)
  * Cleanup protected branches when deleting users & teams (go-gitea#19158) (go-gitea#19174)
  * Use IterateBufferSize whilst querying repositories during adoption check (go-gitea#19140) (go-gitea#19160)
  * Fix NPE /repos/issues/search when not signed in (go-gitea#19154) (go-gitea#19155)
  * Use custom favicon when viewing static files if it exists (go-gitea#19130) (go-gitea#19152)
  * Fix the editor height in review box (go-gitea#19003) (go-gitea#19147)
  * Ensure isSSH is set whenever DISABLE_HTTP_GIT is set (go-gitea#19028) (go-gitea#19146)
  * Fix wrong scopes caused by empty scope input (go-gitea#19029) (go-gitea#19145)
  * Make migrations SKIP_TLS_VERIFY apply to git too (go-gitea#19132) (go-gitea#19141)
  * Handle email address not exist (go-gitea#19089) (go-gitea#19121)
* MISC
  * Update json-iterator to allow compilation with go1.18 (go-gitea#18644) (go-gitea#19100)
  * Update golang.org/x/crypto (go-gitea#19097) (go-gitea#19098)

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Mar 23, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 24, 2022
* giteaofficial/main:
  Bump minimist from 1.2.5 to 1.2.6 (go-gitea#19194)
  Changelog for 1.16.5 (go-gitea#19189) (go-gitea#19192)
  Fix showing issues in your repositories (go-gitea#18916)
  Update issue_no_dependencies description (go-gitea#19112)
  Prevent redirect to Host (2) (go-gitea#19175)
  Prevent start panic due to missing DotEscape function
  Fix compare link in active feeds for new branch (go-gitea#19149)
  Redirect .wiki/* ui link to /wiki (go-gitea#18831)
  Try to prevent autolinking of displaynames by email readers (go-gitea#19169)
  Update HTTP status codes to modern codes (go-gitea#18063)
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
Redirect .wiki/* ui link to /wiki

fix go-gitea#18590

Signed-off-by: a1012112796 <1012112796@qq.com>
Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] Wiki as submodule
6 participants