Since 1.12.4 diff not working with blanks in filename or pathname

[fashberg]

• Gitea version (or commit ref): 1.12.4 or master e204398
• Git version: 2.25.1
• Operating system: ubuntu 20.04 LTS or docker-image 'gitea/gitea'
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL) https://try.gitea.io/fashberg/testrepo/commits/branch/master (check out both commits)
  • No
  • Not relevant
• Log gist:

Description

Gitea Diff shows either wrong filenames or produces HTTP 500 if there is a blank in the filename/pathname of a changed file.

This bug applies only if git, but this seems to be default, does not quote the filenames at diff output.

Bug is since issue #12554 (#12575) - which became live in 1.12.4

Detail

Originally posted by @fashberg in #12554 (comment)

This fix breaks filenames with blanks in path (filename or foldername).

If you have
"diff --git a/folder/filename withBlank b/folder/filename withBlank"

you get now this wrong variable assignments:

a="a/folder/filename"
b="withBlank"

And the this two lines at https://github.com/go-gitea/gitea/blob/master/services/gitdiff/gitdiff.go#L592

	a = a[2:]
	b = b[2:]

are deleting the first two characters resulting in

a="folder/filename"
b="thBlank"

In the diff you see a renaming filename → thBlank and if you click on the link to 'Show File' it links to 'thBlank' which results in 404.

The bug is getting worse if the filename has only a single character after the first blank, e.g. test - file

Resulting in

a="a/folder/test"
b="-"

Because strlen of b is only 2 the substring(b, 2)/ (b = b[2:]) will not work because string is too short, resulting in Error 500

2020/09/08 15:43:06 ...les/context/panic.go:35:1() [E] PANIC:: runtime error: slice bounds out of range [2:1]
	/usr/lib/go-1.13/src/runtime/panic.go:103 (0x436882)
		goPanicSliceB: panic(boundsError{x: int64(x), signed: true, y: y, code: boundsSliceB})
	/home/fashberg/gitea/services/gitdiff/gitdiff.go:593 (0x1d4191a)

So the middle = strings.Index(line, " b/") hack was not so bad.

Now filenames with blanks are completely broken in 1.12.4 and master!

Why ignoring the lines with --- a/xxxx and +++ b/xxxx?
There you can cat the real filename without any quoting problems. OK, new files are having --- /dev/null and maybe there are some more special cases.

Kind Regards
Folke

[zeripath]

Looks like there can be no way of actually detemining which file has been changed from the diff line:

diff --git a/a b/file b/a a/file b/a b/file b/a a/file
index d2186f1..f5c8ed2 100644
--- a/a b/file b/a a/file	
+++ b/a b/file b/a a/file	
@@ -1,3 +1,2 @@
 Create a weird file.
 
-and what does diff do here?
\ No newline at end of file

[mrsdizzie]

Could we somehow refactor and use output git diff --name-only or git diff --name-status for this instead of looking at the diff itself?

[zeripath]

Hopefully this won't reveal (yet) another bug