PR Error 500 on CreateCsvDiff when trying to merge some branches: invalid memory address

[99rgosse]

Description

Hello
We have a huge repo (800 branches, 6Gb lfs files, 12k commits, with branch protection) that we have migrated from Gitlab 2 weeks ago, on a Self hosted Gitea 1.16.5 with Pgsql

Since 2 days, some users encounters a 500 error when trying to create a pull request, there are some branches that generates this error
I tested this on the 1.16.5 & 1.16.6 instances I have. We have not many other repo to test otherwise

If we recreate the same branch with another name, the PR works as intended.

short version log :

2022/04/27 14:44:11 ...rs/web/repo/issue.go:751:setTemplateIfExists() [D] could not extract metadata fdev .gitea/pull_request_template.md [mainrepo/dev]: frontmatter must start with a separator line

2022/04/27 14:44:11 ...s/context/context.go:204:HTML() [D] Template: repo/diff/compare

2022/04/27 14:44:12 ...s/context/context.go:214: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: invalid memory address or nil pointer dereference

2022/04/27 14:44:12 ...s/context/context.go:204:HTML() [D] Template: status/500

2022/04/27 14:44:12 Completed GET /mainrepo/dev/compare/release/master...release/calibration 500 Internal Server Error in 1.331171119s

Gitea Version

1.16.5 / 1.16.6 / latest dev on master

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

https://try.gitea.io/99rgosse/issues/issues/1

Screenshots

Git Version

2.30.3

Operating System

Docker Gitea with Ubuntu

How are you running Gitea?

Docker official Gitea v.16.6
Caching enabled :

[cache]
  | ENABLED  = true
  | ADAPTER  = memory
  | INTERVAL = 120
  | ITEM_TTL = 24h
  |  
  | [cache.last_commit]
  | ENABLED       = true
  | ITEM_TTL      = 8760h
  | COMMITS_COUNT = 7000

I tried :

• Removing the caching
• Removing the template generating errors

Database

PostgreSQL

[wxiaoguang]

It's likely caused by a bug in CreateCsvDiff, do you have some CSV files to reproduce the bug?

---

If we recreate the same branch with another name, the PR works as intended., that's also weird ....

[wxiaoguang]

Is it possible that you could add a recover at the beginning of CreateCsvDiff to see the panic stack?

[99rgosse]

Yes, I will build from source and try this, and try to have more debug

[lunny]

I have tested a small csv in the pull request and it works. Maybe the csv file has a special format.

[99rgosse]

The CSV File format is not in cause

Okay, so here is the full history:
Consider branch Official and branches Secondary seemingly being two different products merged from time to time.

• New branches made from Official are working for a PR with Official - no problem
• New Branches made from Secondary will be failing to make a PR, trying to CSVDiff the deleted file
  But the file is deleted since the migration day. Precisely this is the latest commit made before I migrated

So the Error comes up as it cannot make a diff on files not existing anymore...

EDIT : adding back the file in a new commit avoid the Error 500

[99rgosse]

Quick and dirty fix here

gitea/routers/web/repo/compare.go

Line 114 in 8eb1cd9

if diffFile == nil || baseCommit == nil || headCommit == nil {

if diffFile == nil || baseCommit == nil || headCommit == nil || diffFile.IsDeleted == true {

I will do some tests, because it may fail if we try to compare a CSV then deleted...
It fails indeed...

The full error is "object does not exist" but somehow it is not displayed =>

gitea/routers/web/repo/compare.go

Line 132 in 8eb1cd9

return nil, nil, err

[wxiaoguang]

Yep, the logic is incomplete.

It doesn't check the err returned by csvReaderFromCommit correctly, then the nil readers are used below.

https://github.com/go-gitea/gitea/blob/8eb1cd9264af9493e0f57a4a4c0a1f764f7cefcf/routers/web/repo/compare.go#L139-L154

---

Update (2022-09-16): the comment above seems incorrect, as discussed in #21184, the CreateCsvDiff is intended to handle nil head or base commit. So, the 500 error is likely to be caused by other reasons.

---

Update (2023-02-17): according to #22946, the first guess seems reasonable, the CreateCsvDiff logic was incomplete, it should be able to process the situation when base and head are both nil.