Error creating pull requests or diffs with very long diffs containing very long lines

[davidsvantesson]

• Gitea version (or commit ref): 1.12.6
• Git version:
• Operating system: Ubuntu
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL)
  • No
• Log gist:
  2020/11/17 10:12:48 ...ters/repo/compare.go:449:PrepareCompareDiff() [E] GetDiffRange: ParsePatch: Unexpected line in hunk: <line containing /// characters>

Description

When creating a PR with a file containing a "//" this will cause a 500 error page. Issue was introduced with #13157 (see services/gitdiff/gitdiff.go line 671).
Suggested solution: Even if this is considered not valid, it should only affect the specific file (For example with a text "Gitea can't generate a diff for this file"). It should not cause a 500 error page making it impossible to create PR.

This is similar to #13597 but seem to be related to LFS, so I decided to create a new issue.

[lunny]

So we should add some new unit tests for git diff.

[johanvdw]

Also affects 1.13.rc2

[mrsdizzie]

Whats a working example of this? I don't seem to be able to recreate it with the description"create a file containing a "//"

If anybody could post one or link to a current example of a 500 on PR at try.gitea.io

[johanvdw]

Here is an example:
https://try.gitea.io/johanvdw/problem500/compare/master...problem

does not contain // though, but shows the same behaviour (it is reworked from a file that showed this error).

seems files have to be large and have to contain long lines for this to happen

[johanvdw]

The file contains random chars, so feel free to reuse for tests if needed.

[mrsdizzie]

Thanks for example that helps:

This happens in this case because:

gitea/services/gitdiff/gitdiff.go

Lines 743 to 749 in 24b3b21

	case '+':
	curFileLinesCount++
	curFile.Addition++
	if curFileLinesCount >= maxLines {
	curFile.IsIncomplete = true
	continue
	}

Once the current file number is above the maxLines value, it does a continue for some reason. this means it never hits this check below the switch:

gitea/services/gitdiff/gitdiff.go

Lines 782 to 792 in 24b3b21

	if isFragment {
	curFile.IsIncomplete = true
	for isFragment {
	lineBytes, isFragment, err = input.ReadLine()
	if err != nil {
	// Now by the definition of ReadLine this cannot be io.EOF
	err = fmt.Errorf("Unable to ReadLine: %v", err)
	return
	}
	}
	}

So esentially anytime you have a line which is longer than 4096 char (which will make isFragment true) and curFileLinesCount >= maxLines you will hit this error

Its going to read the first part of the line that starts with +, hit that if statement in side the +case, continue back to the top of the loop, continue reading from the middle of the previous line where it left off by calling input.ReadLine() again and then fail here:

gitea/services/gitdiff/gitdiff.go

Lines 775 to 778 in 24b3b21

	default:
	// This is unexpected
	err = fmt.Errorf("Unexpected line in hunk: %s", string(lineBytes))
	return

Because lineBytes[0] is just going to be whatever value was next in the large line. I'm not sure why it is like this, @zeripath should know hopefully

[zeripath]

It's simply a bug - I've failed to munch the end of the fragment when there is a maxLinesCount because I've just forgotten to do it.