ParsePatch returns 404 if diff --git string is double quoted

[mrsdizzie]

• Gitea version (or commit ref): master
• Git version: 2.21.0
• Operating system: macOS
• Database (use [x]):
  • [ ] PostgreSQL
  • [ ] MySQL
  • [ ] MSSQL
  • [x ] SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • [ x] Yes https://try.gitea.io/mrsdizzie/parsing-errors/commit/c338b88002fef425763d7268628471ef81c87ce7
  • [ ] No
  • [ ] Not relevant
• Log gist:

Description

When a filename has a double quote, backslash, or other potentially escapable character gitea will return a 404 when trying to view a particular commit to that file. This is because the git diff will double quote the a/b/ file names .

This can be recreated easily by creating a new file with a double quote character in the name and the trying to view the commit (see example above).

This happens because of the following code:

https://github.com/go-gitea/gitea/blob/50631b5ac3ea06e167eca42c564ff4d89568ae99/models/git_diff.go#L541-L563

In the case of creating a new file with the literal name of test"file, What happens here is you end up with a 'line' like:

line: diff --git "a/test\"file" "b/test\"file"

Gitea notices the first character after diff --git and correctly alters the middle bettween the a/b sections, but then does this:

https://github.com/go-gitea/gitea/blob/50631b5ac3ea06e167eca42c564ff4d89568ae99/models/git_diff.go#L550-L552

Which gives you:

a: /test\"file"
b: /test\"file"

Then it tries to pass those to strconv.Unquote which triggers a Unquote: invalid syntaxerror. This bit of logic seems flawed for a few reasons:

First, the

beg := len(cmdDiffHead) 
 a := line[beg+2 : middle] 
 b := line[middle+3:] 

Seems confusing because you'd probably not want to get rid of the first quote and keep the last one. With the exampled listed in the code comment of diff --git "a/xxx" "b/xxx" you'd end up with things like /xxx"

Then, I'm not sure why you would want strconv.Unquote at all if you've previously tried to parse what is between the quotes. This appears to work properly if you don't try and strip off the "/a" and "/b first and just use:

beg := len(cmdDiffHead) 
 a := line[beg: middle] 
 b := line[middle+1:] 

That leaves you with

a: "a/test\"file"
b: "b/test\"file"

Which work fine in strconv.Unquote and show the diff as expected. Also related, this error should probably be returning a 500 rather than a 404 here:

https://github.com/go-gitea/gitea/blob/b2e9894988a8cb486f8838f4bf532401124802c4/routers/repo/commit.go#L215

Opening an issue before a pull request for any discussion if this would be an acceptable change to fix this.

[mrsdizzie]

I suspect this has some role to play in https://github.com/go-gitea/gitea/issues/6225 or other cases where doing any sort of compare results in an error