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

Fix redirect when using lowercase repo name #18775

Merged
merged 1 commit into from
Feb 17, 2022

Conversation

AbdulrhmnGhanem
Copy link
Contributor

Fixes ##18774.

@silverwind
Copy link
Member

It feels like the wrong fix, especially setting.AppSubURL may contain significant case and that is eliminated here, which can be bad. Not sure I fully understand the issue but it would be better to accept both case variants in the router instead.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 15, 2022
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 15, 2022
@AbdulrhmnGhanem
Copy link
Contributor Author

AbdulrhmnGhanem commented Feb 16, 2022

It feels like the wrong fix, especially setting.AppSubURL

You are right about this one. Changed it to

prefix := strings.TrimPrefix(setting.AppSubURL+strings.ToLower(strings.TrimSuffix(ctx.Req.URL.Path, ctx.Params("*"))), strings.ToLower(ctx.Repo.RepoLink))

it would be better to accept both case variants in the router instead.

The bug in the RepoRefLegacy handling:

if refType == RepoRefLegacy {
// redirect from old URL scheme to new URL scheme
prefix := strings.TrimPrefix(setting.AppSubURL+strings.TrimSuffix(ctx.Req.URL.Path, ctx.Params("*")), ctx.Repo.RepoLink)

other RepoRefTypes are handled right and not in the router:

gitea/routers/web/web.go

Lines 1017 to 1024 in 616146f

m.Group("/raw", func() {
m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.SingleDownload)
m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.SingleDownload)
m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownload)
m.Get("/blob/{sha}", context.RepoRefByType(context.RepoRefBlob), repo.DownloadByID)
// "/*" route is deprecated, and kept for backward compatibility
m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.SingleDownload)
}, repo.MustBeNotEmpty, reqRepoCodeReader)

for consistency, I don't think this should be handled in the router.

@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 Feb 16, 2022
@lunny lunny added backport/v1.16 type/bug skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Feb 16, 2022
@lunny lunny added this to the 1.17.0 milestone Feb 16, 2022
@lunny lunny added type/enhancement An improvement of existing functionality and removed type/bug labels Feb 16, 2022
@lunny
Copy link
Member

lunny commented Feb 17, 2022

@AbdulrhmnGhanem Could you rebase and then we can merge this PR.

* Previously,  `GET {username}/{reponame}/raw///file-path` (the middle two slashes are blank to get the default branch) when the repo name has uppercase letters, e.g., https://try.gitea.io/AbdulrhmnGhanem/CH330_Hardware, using a lowercase version of the name redirected to the correct URL
* In other words both
   * `GET https://try.gitea.io/AbdulrhmnGhanem/CH330_Hardware/raw///images/back.png`
   * `GET https://try.gitea.io/AbdulrhmnGhanem/ch330_hardware/raw///images/back.png`
were redirecting to ` GET https://try.gitea.io/AbdulrhmnGhanem/CH330_Hardware/raw/branch/master/images/back.png`
This isn't the case after  go-gitea#17551. Specifically because of this [line](https://github.com/zeripath/gitea/blob/cbd5eecd148dfca5fcb1a3da469e491a84f6b32b/modules/context/repo.go#L860).
@lunny
Copy link
Member

lunny commented Feb 17, 2022

make L-G-T-M work.

@lunny lunny merged commit 1856467 into go-gitea:main Feb 17, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 18, 2022
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Fix redirect when using lowercase reponame (go-gitea#18775)
@fnetX
Copy link
Contributor

fnetX commented Feb 25, 2022

We just hit this bug at Codeberg immediately after deployment. Should I send a backport? Was this forgotten? (I think I'll do)

@Gusted
Copy link
Contributor

Gusted commented Feb 25, 2022

Was this forgotten?

This was marked as enhancement(not sure why), we only backport PR's marked as bug fixes. Feel free to backport.

@fnetX
Copy link
Contributor

fnetX commented Feb 25, 2022

Well it got the backport label, so it appeared to me as intended for backporting.

@Gusted Gusted added type/bug and removed type/enhancement An improvement of existing functionality labels Feb 25, 2022
@Gusted Gusted added the backport/done All backports for this PR have been created label Feb 25, 2022
lunny pushed a commit that referenced this pull request Feb 26, 2022
* Previously,  `GET {username}/{reponame}/raw///file-path` (the middle two slashes are blank to get the default branch) when the repo name has uppercase letters, e.g., https://try.gitea.io/AbdulrhmnGhanem/CH330_Hardware, using a lowercase version of the name redirected to the correct URL
* In other words both
   * `GET https://try.gitea.io/AbdulrhmnGhanem/CH330_Hardware/raw///images/back.png`
   * `GET https://try.gitea.io/AbdulrhmnGhanem/ch330_hardware/raw///images/back.png`
were redirecting to ` GET https://try.gitea.io/AbdulrhmnGhanem/CH330_Hardware/raw/branch/master/images/back.png`
This isn't the case after  #17551. Specifically because of this [line](https://github.com/zeripath/gitea/blob/cbd5eecd148dfca5fcb1a3da469e491a84f6b32b/modules/context/repo.go#L860).

Co-authored-by: Ghanem <[email protected]>
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
* Previously,  `GET {username}/{reponame}/raw///file-path` (the middle two slashes are blank to get the default branch) when the repo name has uppercase letters, e.g., https://try.gitea.io/AbdulrhmnGhanem/CH330_Hardware, using a lowercase version of the name redirected to the correct URL
* In other words both
   * `GET https://try.gitea.io/AbdulrhmnGhanem/CH330_Hardware/raw///images/back.png`
   * `GET https://try.gitea.io/AbdulrhmnGhanem/ch330_hardware/raw///images/back.png`
were redirecting to ` GET https://try.gitea.io/AbdulrhmnGhanem/CH330_Hardware/raw/branch/master/images/back.png`
This isn't the case after  go-gitea#17551. Specifically because of this [line](https://github.com/zeripath/gitea/blob/cbd5eecd148dfca5fcb1a3da469e491a84f6b32b/modules/context/repo.go#L860).
@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. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants