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

Pull request overview page getting 404 (?repo=) #5892

Closed
2 of 7 tasks
ntimo opened this issue Jan 29, 2019 · 32 comments · Fixed by #5900
Closed
2 of 7 tasks

Pull request overview page getting 404 (?repo=) #5892

ntimo opened this issue Jan 29, 2019 · 32 comments · Fixed by #5900
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented

Comments

@ntimo
Copy link

ntimo commented Jan 29, 2019

  • Gitea version (or commit ref): 1.7.0 (3fa49f3)
  • Operating system: Docker (Ubuntu Host)
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant

Description

When I go to mygitea.com/pulls?type=all&repo=9&sort=&state=open I get a white page in Safari, Chrome simply shows the Chrome 404 page. When I change the ?repo= to be 0 it works and shows the correct repo. All other repos work fine. When I go to the Pull-Request page in the top menu and I select this one repo that is in my namespace I get this crazy error, but all other repos work fine.

No custom 404 Gitea page is shown.

Log

gitea-mailcow_1      | [Macaron] 2019-01-29 20:01:32: Completed GET /pulls?type=all&repo=9&sort=&state=open 404 Not Found in 37.341391ms
gitea-mailcow_1      | [Macaron] 2019-01-29 20:01:32: Started GET /serviceworker.js for 192.168.32.4
gitea-mailcow_1      | [Macaron] 2019-01-29 20:01:32: Completed GET /serviceworker.js 200 OK in 4.827606ms

Screenshots

ohne titel

@ntimo ntimo changed the title Pull request page getting 404 (?repo=) Pull request overview page getting 404 (?repo=) Jan 29, 2019
@zeripath
Copy link
Contributor

zeripath commented Jan 29, 2019

There's no route to /pulls defined in routers/routes/routes.go. There are /username/repo/pulls routes but not a /pulls. It should therefore be a 404. Where did you get directed to /pulls from? see below

@adelowo
Copy link
Member

adelowo commented Jan 29, 2019

@zeripath That route actually exists

m.Get("/^:type(issues|pulls)$", reqSignIn, user.Issues)

@adelowo
Copy link
Member

adelowo commented Jan 29, 2019

I was following this bug on Discord but it's past 12am here and I am exhausted, could have tested and sent a fix potentially

@zeripath
Copy link
Contributor

Ugh my bad. I just scanned that file and missed that ...

It's past 11 here too so won't be able to look at it myself further.

@techknowlogick
Copy link
Member

Go get some rest you two!

@zeripath
Copy link
Contributor

func Issues(ctx *context.Context) {
is probably the misbehaving function

@adelowo
Copy link
Member

adelowo commented Jan 30, 2019

Do you actually have a repository with an id of 9.

By the way, I think it is better we return a 404 in this instance rather than a 500

@ntimo
Copy link
Author

ntimo commented Jan 30, 2019

I just checked and the repo id of the repo is indeed 9. So trying to open mygitea.com/pulls?type=all&repo=9&sort=&state=open should not result in a 404.

@zeripath zeripath reopened this Jan 30, 2019
@zeripath
Copy link
Contributor

Sorry @ntimo . Just looking at your screenshots the likely place you're getting the 404 from is: https://github.com/go-gitea/gitea/blob/v1.7.0/routers/user/home.go#L296

That line sets the status as 404 without sending the gitea 404 page. So it seems that the ctxUser doesn't have permission to read that repository... This is odd.

@ntimo
Copy link
Author

ntimo commented Jan 31, 2019

@zeripath Thanks for reopening. But why would the ctxUser not have permissions to read the repo? I created it with my user and its also in my namespace ntimo/reponame. Also I can commits code changes, create pull requests, etc in this repo.

@zeripath
Copy link
Contributor

I'm not sure. That line is the only place that the 404 could be set. (It's also wrong - we should never just set a status of 404 without sending the 404 page!)

Looking at the code leading to that point I can't quite see what the problem could be - I don't think it's the clearest code but I'm not certain it's definitely wrong.

The only thing I can think of is that the internal cache on line is incorrect somehow and the repo returned on https://github.com/go-gitea/gitea/blob/master/routers/user/home.go#L300 is wrong.

Can you reproduce your problem on try or give me a script of exactly what to do to replicate it?

@ntimo
Copy link
Author

ntimo commented Jan 31, 2019

I can't replicate it on try. Well the steps I follow to reproduce the issue on my installation are quite simple:

  • Click on Pull-Requests in the header menu (opens mygita.com/pulls page)
  • Select the repo in the lift sidebar
  • Get a white page

Then it opens the url: mygitea.com/pulls?type=all&repo=9&sort=&state=open which shows the white page.
I can make a video later and send it to you via Discord if that would help?

@stale
Copy link

stale bot commented Apr 1, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Apr 1, 2019
@lunny lunny added issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented and removed issue/stale labels Apr 1, 2019
@lunny
Copy link
Member

lunny commented Apr 1, 2019

@ntimo can you still reproduce it on v1.7.5?

@ntimo
Copy link
Author

ntimo commented Apr 1, 2019

@lunny I am using 1.8.0-rc2 at the moment and I can still reproduce it :(

@yanOnGithub
Copy link

reproduced in 1.8.0, win7 64 bit, sqlite

20190430T102512+0800_YAN_chrome

@lunny
Copy link
Member

lunny commented Apr 30, 2019

I think this should be fixed by #6754

@Ahaus314
Copy link

We're experiencing the same issue with the version 1.9.4.

@guillep2k
Copy link
Member

@Ahaus314 This can happen if the repository is private and you somehow lost the session, e.g. you save a link for later, then open the browser and try to use the link without getting logged in first. If that's not your case, please:

  • Paste the URL with the problem here (you can change some names if you want).
  • Let us know whether the repository you want to access is private or public.
  • Let us know if your site is configured to require login to see any content, or some its content is public.
  • If possible, please paste a screen capture showing where did you click last before getting the error.

@Ahaus314
Copy link

Ahaus314 commented Nov 29, 2019

@guillep2k Thanks for the reply and sorry for the delay. This behavior still happen with v1.10.0.

@guillep2k
Copy link
Member

@Ahaus314 In my company I have 1.10.0 with a setting at first glance similar to yours but I can't reproduce, so there must be some subtle difference. I'm assuming that the user in the example you've detailed doesn't have an expired session (i.e. it's a recent login), so let's put that aside.

I'm suspecting there's some specific combination of permissions that leads to this problem, but I can't imagine what is it. In my case all repos are private by site policy, so there's that, but I've tested the following scenarios:

  • The user is admin.
  • The repo belongs to an org and the user is in a team with PR write permissions.
  • The repo belongs to an org and the user is in a team with PR read permissions.
  • The repo belongs to an org and the user is in no related team.
  • The repo belongs to a different user, but the executing user is a collaborator with write permissions.
  • The repo belongs to a different user, but the executing user is a collaborator with read permissions.
  • The repo belongs to a different user, and the executing user has no permissions whatsoever.

In all cases the user was either able to navigate normally or the repo wasn't listed at all (i.e. the user had not enough permissions).

Maybe the list above will give you some ideas to try yourself or some insight about the problem that could help us find it.

@jolheiser
Copy link
Member

As well, it seems some logging has been added to the spot @zeripath mentioned up above where a 404 status is set without a page.
Would you mind setting your logging to trace and sending some log lines around when you experience the blank page? (Apologies for having to sift through a logger set to TRACE)

For reference on the release/v1.9 branch:
https://github.com/go-gitea/gitea/blob/release/v1.9/routers/user/home.go#L311-L321

@Ahaus314
Copy link

@guillep2k I"m pretty sure I already did those tests (I did a lot and cannot remember), but to be sure, I'll do those. I highly think it's a permission related issue. As @jolheiser said, some logging was added and I remember seeing those "Permission denied" messages in the log.

@jolheiser I already setup a dev environment which is an exact replica from the prod environment. So I can do any tests without impacting the prod. I already changed the log level to trace on that instance. I'll join some log next week when I'll get back to office.

Thanks for your help!

@6543
Copy link
Member

6543 commented Nov 30, 2019

@Ahaus314 can you test #8741?
-> I'll have a similar problem and solved it at the PR

@lunny
Copy link
Member

lunny commented Nov 30, 2019

Maybe https://github.com/go-gitea/gitea/blob/v1.7.0/routers/user/home.go#L290 should be

-perm, err := models.GetUserRepoPermission(repo, ctxUser)
+perm, err := models.GetUserRepoPermission(repo, ctx.User)

Because ctxUser means
图片

but not the login user.

@davidsvantesson
Copy link
Contributor

With recent PRs this behavior has changed. However there are some small problems left:

Small nit in routers/user/home.go:313:

		if !perm.CanRead(models.UnitTypeIssues) {
			log.Error("User created Issues in Repository which they no longer have access to: [%d]", repoID)
		}

The check is always for unit type issues also for pull request page. Maybe the repository should also be filtered out and not be displayed in this case?

Currently you only see repositories where you are a team member, not repositories where you are collaborator of. Shouldn't the database query use the access table instead?

@Ahaus314
Copy link

Ahaus314 commented Dec 3, 2019

Here's the log when I try to access the Pull Request from the top menu.
2019/12/03 10:46:49 routers/user/home.go:315:Issues() [T] Permission Denied: User 824758234000:usrTest cannot read 824758234064:UnitTypeIssues of repo 824758234112:orgTest/repoTest User in repo has Permissions: AccessMode: 824758234672:write, 5 Units, 5 UnitsMode(s): [ Units[0]: ID: 824758234208 RepoID: 824758234240 Type: 824758234704:UnitTypeExternalTracker Config: {"ExternalTrackerURL":"https://<OUR_TICKET_MANAGER_ADDRESS>","ExternalTrackerFormat":"","ExternalTrackerStyle":""} Units[1]: ID: 824758234304 RepoID: 824758234336 Type: 824758234736:UnitTypePullRequests Config: {"IgnoreWhitespaceConflicts":true,"AllowMerge":true,"AllowRebase":true,"AllowRebaseMerge":true,"AllowSquash":true} Units[2]: ID: 824758234400 RepoID: 824758234432 Type: 824758234768:UnitTypeCode Config: {} Units[3]: ID: 824758234496 RepoID: 824758234528 Type: 824758234800:UnitTypeReleases Config: {} Units[4]: ID: 824758234592 RepoID: 824758234624 Type: 824758234832:UnitTypeExternalWiki Config: {"ExternalWikiURL":"https://<OUR_WIKI_ADDRESS>"} UnitMode[824758234864:UnitTypeCode]: 824758234896:write UnitMode[824758234928:UnitTypeReleases]: 824758234960:write UnitMode[824758234992:UnitTypeExternalWiki]: 824758235024:write UnitMode[824758235056:UnitTypeExternalTracker]: 824758235088:write UnitMode[824758235120:UnitTypePullRequests]: 824758235152:write ] /go/src/code.gitea.io/gitea/routers/user/home.go:315 (0x1357666) /usr/local/go/src/reflect/value.go:460 (0x49699c) /usr/local/go/src/reflect/value.go:321 (0x49615a) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:177 (0x9b87c0) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:137 (0x9b8160) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:121 (0x9e834f) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:112 (0x11582a8) /go/src/code.gitea.io/gitea/modules/context/panic.go:39 (0x1158294) /usr/local/go/src/reflect/value.go:460 (0x49699c) /usr/local/go/src/reflect/value.go:321 (0x49615a) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:177 (0x9b87c0) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:137 (0x9b8160) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:121 (0x9e834f) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:112 (0xab930c) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/session/session.go:192 (0xab92f7) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:79 (0x9e81c7) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:157 (0x9b84d0) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:135 (0x9b824f) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:121 (0x9e834f) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:112 (0x9f9d50) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/recovery.go:161 (0x9f9d3e) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/logger.go:40 (0x9ebfaa) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:157 (0x9b84d0) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:135 (0x9b824f) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:121 (0x9e834f) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:112 (0x167407d) /go/src/code.gitea.io/gitea/routers/routes/routes.go:68 (0x1674068) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:79 (0x9e81c7) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:157 (0x9b84d0) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:135 (0x9b824f) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:121 (0x9e834f) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:112 (0x16748aa) /go/src/code.gitea.io/gitea/routers/routes/routes.go:103 (0x1674895) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:79 (0x9e81c7) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:157 (0x9b84d0) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/inject/inject.go:135 (0x9b824f) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/context.go:121 (0x9e834f) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/router.go:187 (0x9faffd) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/router.go:303 (0x9f48ac) /go/src/code.gitea.io/gitea/vendor/gitea.com/macaron/macaron/macaron.go:220 (0x9ed381) /go/src/code.gitea.io/gitea/vendor/github.com/gorilla/context/context.go:141 (0xc5d204) /usr/local/go/src/net/http/server.go:2007 (0x75b75a) /usr/local/go/src/net/http/server.go:2802 (0x75ec4a) /usr/local/go/src/net/http/server.go:1890 (0x75a51b) /usr/local/go/src/runtime/asm_amd64.s:1357 (0x462c30)

Thanks!

@lunny
Copy link
Member

lunny commented Dec 3, 2019

This should be a different problem with the original.

@davidsvantesson
Copy link
Contributor

@Ahaus314 That is on 1.10?

@Ahaus314
Copy link

Ahaus314 commented Jan 7, 2020

@davidsvantesson

@Ahaus314 That is on 1.10?

I just checked, and yes, it's still on 1.10.0.

@davidsvantesson
Copy link
Contributor

#8741 is towards 1.11, so the 404 problem shall be solved there.

I think there are some other problems remaining with that page, not all PRs is included as expected.

@6543
Copy link
Member

6543 commented Sep 17, 2020

I'll close this since original issue is solved, and mentioned other issues should be too ...

... if not pleace open new issues for them

@6543 6543 closed this as completed Sep 17, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.