go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
44.59k stars 5.45k forks source link

500 Internal Server Error when creating new pull request #6225

Closed hotbain closed 4 years ago

hotbain commented 5 years ago

Description

[Macaron] 2019-03-02 09:59:20: Completed POST /hotbain/spring_cloud/compare/master...test_branch 404 Not Found in 175.311212ms [Macaron] 2019-03-02 09:59:21: Started GET /serviceworker.js for [::1]

Screenshots

image

ptman commented 5 years ago

We're also getting

Completed GET /org/proj/compare/master...fork:master 404 Not Found in 91.186937ms
lafriks commented 5 years ago

Are there are any error in gitea.log?

ptman commented 5 years ago

No errors. But if I go to /org/proj/pulls and press "New Pull Request" it goes to /org/proj/compare/master...fork:master which gives 404. We did not have this problem with 1.7.2.

ptman commented 5 years ago

Still an issue with 1.7.4

ptman commented 5 years ago

Are there schema changes between 1.7.2 and 1.7.4 or would downgrade be safe?

zeripath commented 5 years ago

@ptman I'm sorry to see this. I've not been following this so I don't completely understand the issue. Are you able to work out what the url should be?

Having seen two PRs touch this endpoint in quick succession because one completely broke I think this endpoint is fairly poorly described. I'm able get 404s on my rather pathological repos on try.

ptman commented 5 years ago

The URL should show the diff between the base and the branch to be merged, not 404. I could look closer at the SQL differences if I knew for sure that downgrading to 1.7.2 doesn't break anything.

zeripath commented 5 years ago

OK it looks like that compare urls are a bit of mess:

Try comparing any branch with % in it. https://try.gitea.io/arandomer/pathological/compare/master...foo%25foo

https://try.gitea.io/arandomer/pathological/compare/master...foo%2525foo

https://try.gitea.io/arandomer/pathological/compare/master...foo%252525foo

In particular try thinking about what happens with %2F:

https://try.gitea.io/arandomer/pathological/compare/path%252Fpath...path/path

It's not just %, preceding - is particularly broken:

https://try.gitea.io/arandomer/pathological/compare/--heads...foo%2525foo

https://try.gitea.io/arandomer/pathological/compare/foo%2525foo...--heads

Get the branches the wrong way round causes a 500:

https://try.gitea.io/arandomer/pathological/compare/notsubpath...master

vs:

https://try.gitea.io/arandomer/pathological/compare/master...notsubpath

There is a bug in the diff generation where escaping filenames with quotes in diff generation breaks:

https://try.gitea.io/arandomer/pathological/compare/master...quotebreak

Which is the same one that affects:

https://try.gitea.io/arandomer/test-perry123/commit/08c5f2e47bda90002aca34b46906d04a414c5a97

mrsdizzie commented 5 years ago

For reference here is some discussion of why it can 404 when there is a double quote in the filename: https://github.com/go-gitea/gitea/issues/6309. Basic testing says gitea will break on any situation where the diff includes the diff --git section inside double quotes like diff --git "a/file" "b/file". This includes having a backslash in the file name which is perhaps more common (Windows?).

I think this might be one of a few issues that lead to bad compare/pull/etc...

ptman commented 5 years ago

In my case the branch names don't contain anything that needs to be encoded. Is it safe to downgrade from 1.7.4 to 1.7.2 or have there been DB schema changes?

lunny commented 5 years ago

@ptman that's downgrade should be safe but of course backup before any change of your production environment.

ptman commented 5 years ago

Ok, here are some sanitized logs from both 1.7.2 and 1.7.4. It seems that after 1.7.2 (i.e. 1.7.3 & 1.7.4) there's a missing join. You can diff the files side by side to see the difference, but I've also added newlines to point it out: https://gist.github.com/ptman/f76b398d0503e5c50424c1ea8cbfafec

zeripath commented 5 years ago

Ok @ptman it looks like your issue is different from the initial issue reported by @hotbain. I'm not certain what's causing that issue as we need more information from @hotbain - If I had to guess, it's likely they're affected by the issue fixed by @mrsdizzie, it's unlikely that they're affected by the remaining branch escaping and preceding hyphen branch issues.

In any case your issue is related to:

https://github.com/go-gitea/gitea/commit/5c30817b5fa1fdeedc8d4fc02cf228dc0e6dc556

Which was reverted by:

https://github.com/go-gitea/gitea/commit/fe99c9901d302921eb1e4a67ba687ca3fe543425

Now, you appear to be in the subset for whom the original commit worked and it's reversion breaks.

This is #6302

Ok, so the revision did two things - one was change the url and the other was the permissions change.

So first of all, are the permissions correct - can the proposer read pulls on the base?

And I need to know where you click pull request and what you expect the URL to be. Can you hand change the URL to make it work?

ptman commented 5 years ago

so let's take this to #6302

nicovince commented 5 years ago

I am also experiencing this kinds of errors when creating pull requests and accepting pull requests.

I have not been able to pinpoint exactly how to produce an error, I tried using the PR url given on the console during the push or manually getting to the pull request page. Sometimes one way gives a 500, sometimes it is the other way around.

My setup is using gitea 1.7.3 built with go1.11.5 : tidb, sqlite3
git version : 2.18.1

Usually when the error 500 appears I have the following error in gitea.log :

/var/log/gitea/gitea.log:2019/03/26 08:55:12 [...user/notification.go:33 GetNotificationCount()] [E] GetNotificationCount: database table is locked: notification

Do you need more informations ?

lunny commented 5 years ago

@nicovince that's another issue. It caused by your sqlite database and which version are you using?

nicovince commented 5 years ago

@lunny I am using sqlite-3.25.3

edit: trying to update to 3.27, I'll let you know if that improves my situation.

zeripath commented 5 years ago

@nicovince please open another issue for this. I suspect we will need to move to a pure Gitea serv as client of Gitea web to prevent this.

nicovince commented 5 years ago

@nicovince please open another issue for this. I suspect we will need to move to a pure Gitea serv as client of Gitea web to prevent this.

opened #6435

lunny commented 5 years ago

@hotbain could you confirm that this has been resolved on v1.8.0

stale[bot] commented 5 years ago

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.

lunny commented 5 years ago

I think this has been resolved. Please feel free to reopen it.

jeansergegagnon commented 5 years ago

Still happens in Gitea Version: 1.9.0+dev-376-ge07ff2f89

logs show:

2019/06/25 11:01:14 .../xorm/session_get.go:99:nocacheGet() [I] [SQL] SELECT "id", "user_id", "issue_id", "is_watching", "created_unix", "updated_unix" FROM "issue_watch" WHERE (user_id = $1) AND (issue_id = $2) LIMIT 1 []interface {}{2, 6}
2019/06/25 11:01:14 .../xorm/session_get.go:99:nocacheGet() [I] [SQL] SELECT "id", "user_id", "repo_id" FROM "watch" WHERE "user_id"=$1 AND "repo_id"=$2 LIMIT 1 []interface {}{2, 2}
2019/06/25 11:01:14 routers/repo/pull.go:302:PrepareMergedViewPullInfo() [E] GetCompareInfo: exit status 128 - fatal: ambiguous argument 'b568d59d88af7ea7393d6ae2c9da09794a3c136d...refs/pull/4/head': unknown revision or path not in the working tree.
        Use '--' to separate paths from revisions, like this:
        'git <command> [<revision>...] -- [<file>...]'
zeripath commented 5 years ago

This is not the same bug as the original.

It has the same overall effect but it is not the same.

@jeansergegagnon how did you cause this?

rmbleeker commented 5 years ago

I'm experiencing the same issues as @jeansergegagnon on Gitea version 1.9.2, a simple installation with an sqlite database.

Steps to reproduce:

Log:

2019/08/28 15:30:20 routers/repo/pull.go:629:MergePullRequest() [E] Merge: getDiffTree: git diff-tree [/opt/gitea/data/tmp/local-repo/merge-911475386.git base:master head:head_repo/remove_readme]: fatal: ambiguous argument 'head_repo/remove_readme': unknown revision or path not in the working tree.
    Use '--' to separate paths from revisions, like this:
    'git <command> [<revision>...] -- [<file>...]'

If you need me to open a separate issue for this let me know.

guillep2k commented 5 years ago

I'm experiencing the same issues as @jeansergegagnon on Gitea version 1.9.2, a simple installation with an sqlite database.

Steps to reproduce:

* create a new branch

* push branch

* create a new pull request, it will be available to be merged automatically according to Gitea

* clicking the "Merge Pull Request" button results in a 500 error

Log:

2019/08/28 15:30:20 routers/repo/pull.go:629:MergePullRequest() [E] Merge: getDiffTree: git diff-tree [/opt/gitea/data/tmp/local-repo/merge-911475386.git base:master head:head_repo/remove_readme]: fatal: ambiguous argument 'head_repo/remove_readme': unknown revision or path not in the working tree.
  Use '--' to separate paths from revisions, like this:
  'git <command> [<revision>...] -- [<file>...]'

If you need me to open a separate issue for this let me know.

This looks like a different problem too. Please fill in another issue for it. Thank you.

lunny commented 4 years ago

OK. Could you reproduce this follow the steps:

1 fork A to B 2 push -f to B 3 click pull request on B 4 500

artfisica commented 4 years ago

OK. Could you reproduce this follow the steps:

1 fork A to B 2 push -f to B 3 click pull request on B 4 500

Dear All,

I found the same issue. After a good installation, simply stop to work.

Please, kindly ask any newer reference. Update the git version is not an option for me.

Thanks!

6543 commented 4 years ago

this should be fixed if there is a new issue feel free to open a new issue for it :)