Fix diff path unquoting

[zeripath]

services/gitdiff/gitdiff.go whereby there it assumed that the path would
always be quoted on both sides

This PR simplifies the code here and uses fmt.Fscanf to parse the
strings as necessary.

Fix #12546

Signed-off-by: Andrew Thornton [email protected]

[codecov-commenter]

Codecov Report

Merging #12554 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #12554      +/-   ##
==========================================
- Coverage   43.44%   43.44%   -0.01%     
==========================================
  Files         643      643              
  Lines       71159    71155       -4     
==========================================
- Hits        30914    30912       -2     
  Misses      35236    35236              
+ Partials     5009     5007       -2     

Impacted Files	Coverage Δ
services/gitdiff/gitdiff.go	74.54% <100.00%> (+0.66%)	⬆️
services/pull/update.go	51.92% <0.00%> (-5.77%)	⬇️
services/pull/temp_repo.go	26.59% <0.00%> (-3.20%)	⬇️
modules/process/manager.go	72.50% <0.00%> (-2.50%)	⬇️
modules/log/event.go	58.49% <0.00%> (-0.95%)	⬇️
services/pull/pull.go	42.03% <0.00%> (-0.24%)	⬇️
models/gpg_key.go	55.40% <0.00%> (+0.58%)	⬆️
modules/indexer/stats/db.go	52.17% <0.00%> (+8.69%)	⬆️
modules/indexer/stats/queue.go	76.47% <0.00%> (+23.52%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fcabbae...938d001. Read the comment docs.

[mrsdizzie]

Maybe add a test here for the case that caused linked issue:

gitea/services/gitdiff/gitdiff_test.go

Line 80 in a048489

func TestParsePatch(t *testing.T) {

[fashberg]

Hey @zeripath !

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

[lunny]

@fashberg Could you send a new issue?

[fashberg]

@lunny: yep, sure.