500 Internal Server Error when creating new pull request

[hotbain]

• Gitea version (or commit ref): 1.7.3
• Git version: 2.20.1
• Operating system: mac
• Database : mysql
• Can you reproduce the bug at https://try.gitea.io: NO
• Log gist:

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

[ptman]

We're also getting

Completed GET /org/proj/compare/master...fork:master 404 Not Found in 91.186937ms

[lafriks]

Are there are any error in gitea.log?

[ptman]

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]

Still an issue with 1.7.4

[ptman]

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

[zeripath]

@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]

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]

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]

For reference here is some discussion of why it can 404 when there is a double quote in the filename: #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]

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?