500 on CSV diff: runtime error: index out of range

[silverwind]

• Gitea version (or commit ref): latest 1.15, 0c7927f
• Git version: 2.33.0
• Operating system: Ubuntu 20.04

When viewing a certain CSV diff, I see a 500 and this error in the log:

2021/08/27 13:14:14 ...s/context/context.go:195:HTML() [E] Render failed: template: repo/diff/csv_diff:3:15: executing "repo/diff/csv_diff" at <call .root.CreateCsvDiff .file .root.BaseCommit .root.HeadCommit>: error calling call: runtime error: index out of range [-1]

I can't share the particular CSV content but will try to fabricate a file that reproduces that error.

[zeripath]

It would be helpful if you could upload an example CSV to try.gitea.io.

I think:

diff --git a/templates/repo/diff/csv_diff.tmpl b/templates/repo/diff/csv_diff.tmpl
index b86e9d2a7..b037e29b8 100644
--- a/templates/repo/diff/csv_diff.tmpl
+++ b/templates/repo/diff/csv_diff.tmpl
@@ -12,7 +12,9 @@
 						{{if and (eq $i 0) (eq $j 0)}}
 							<th class="line-num">{{.RowIdx}}</th>
 							{{range $j, $cell := $row.Cells}}
-								{{if eq $cell.Type 2}}
+								{{if not $cell}}
+									<th></th>
+								{{else if eq $cell.Type 2}}
 									<th class="modified"><span class="removed-code">{{.LeftCell}}</span> <span class="added-code">{{.RightCell}}</span></th>
 								{{else if eq $cell.Type 3}}
 									<th class="added"><span class="added-code">{{.LeftCell}}</span></th>
@@ -25,7 +27,9 @@
 						{{else}}
 							<td class="line-num">{{if .RowIdx}}{{.RowIdx}}{{end}}</td>
 							{{range $j, $cell := $row.Cells}}
-								{{if eq $cell.Type 2}}
+								{{if not $cell}}
+									<td></td>
+								{{else if eq $cell.Type 2}}
 									<td class="modified"><span class="removed-code">{{.LeftCell}}</span> <span class="added-code">{{.RightCell}}</span></td>
 								{{else if eq $cell.Type 3}}
 									<td class="added"><span class="added-code">{{.LeftCell}}</span></td>

Would stop the panic but I'm not sure if there is a deeper bug here.

[silverwind]

Here you go:

https://try.gitea.io/silverwind/symlink-test/commit/46c2ca254f58499afea56cbd16912c3523cc8f00
silverwind/symlink-test@46c2ca2

Essentially, it is a CSV where column names and content have changed:

diff --git a/test.csv b/test.csv
index bfde6bf..ae961ff 100644
--- a/test.csv
+++ b/test.csv
@@ -1,2 +1,2 @@
-a,b,c
-1,2,3
+d,e,f
+4,5,6

Just changing column names alone also triggers it:

https://try.gitea.io/silverwind/symlink-test/commit/653094402e4af6fa3814dc83f9708bc299acae94

diff --git a/test.csv b/test.csv
index ae961ff..f68e6c8 100644
--- a/test.csv
+++ b/test.csv
@@ -1,2 +1,2 @@
-d,e,f
+g,h,i
 4,5,6

[richmahn]

Is anyone working on this one? Having got quite familiar with the CSV module I'll look.

[richmahn]

Maybe what @zeripath wrote above will make it work, but the error one gets from switching the column name is:

An error has occurred:

template: repo/diff/csv_diff:15:21: executing "repo/diff/csv_diff" at <$cell.Type>: nil pointer evaluating *gitdiff.TableDiffCell.Type

[richmahn]

Yes, what @zeripath wrote above removes the error, but then you don't see what it was changed FROM....I assume we need to show two header rows or something to see the change.

[zeripath]

I suspect the diff may need a close looking at.

[silverwind]

Just did a test with the patch above, did not help, still failing with the same error.

[zeripath]

@silverwind can you chat to me on discord - it might be easiest to just give me a copy of the problematic CSVs in question privately so we can just get this sorted.

@KN4CK3R do you have any ideas?

[silverwind]

@zeripath did you see #16837 (comment)?

File is also on try: https://try.gitea.io/silverwind/symlink-test/src/branch/master/test.csv.

Essentially I think the csv diff crashes when any column names change.

[richmahn]

Easy to reproduce. Just make a test.csv file like:

a, b, c
1, 2, 3

Then edit again and change it to

d, b, c
1, 2, 3

and then view the commit. It will be 500 error:

Using the #16837 (comment) patch works, but does NOT give you the right diff info o know what is going on.... a PR would just tell you a column is being added, but not what was deleted. You are just shown this:

Where it is just showing a column has been added and a blank column that means nothing, but what about what the text used to be? It's fine to show a column being added, but we should also show the column that was deleted, such as (I edited the HTML to mock up this):

So I'm looking into how to best convey this in the row diff data that the template gets.

[richmahn]

So yeah, I'm stepping through the diff and how the Row/Cell array is populated. It should't be giving a nil entry, but an entry that shows a deleted column and its text, just as if you changed the original CSV above to:

a, a1, b
1, 1a, 2

[richmahn]

Actually it crashes with that too....yet I did get it to work with the real code where you delete one column and add another, but I think there had to be maybe a few columns in-between.

[richmahn]

Found the bug:

gitea/services/gitdiff/csv.go

Line 184 in 0c61376

cells[i] = &TableDiffCell{LeftCell: row[i], Type: celltype}

gitea/services/gitdiff/csv.go

Line 219 in 0c61376

cells[i] = &TableDiffCell{LeftCell: bcell, Type: TableDiffCellAdd}

Going through the unmapped b row (the new content) clobbers the old content's "cells" entries. The cells should be shifted right and then this unmapped b cell can be inserted. Making fix.

[richmahn]

@zeripath @silverwind and anyone else: for review ^^^ #17018

[richmahn]

@silverwind @zeripath I finally got back to work on this issue, with my total rework of the diffing of CSV files and even showing when a column is moved. I have pushed my latest changes including test fixes. #17018