Re-work MAX_DIFF_LINES: supress diff per file, not the whole diff (#3174)

conf/app.ini

@@ -328,7 +328,12 @@ RUN_AT_START = true
 SCHEDULE = @every 24h
 
 [git]
-MAX_GIT_DIFF_LINES = 10000
+; Max number of lines allowed of a single file in diff view.
+MAX_GIT_DIFF_LINES = 500
+; Max number of characters of a line allowed in diff view.
+MAX_GIT_DIFF_LINE_CHARACTERS = 500
+; Max number of files shown in diff view.
+MAX_GIT_DIFF_FILES = 100
 ; Arguments for command 'git gc', e.g. "--aggressive --auto"
 ; see more on http://git-scm.com/docs/git-gc/1.7.5
 GC_ARGS =

conf/locale/locale_en-US.ini

@@ -690,6 +690,8 @@ diff.show_unified_view = Unified View
 diff.stats_desc = <strong> %d changed files</strong> with <strong>%d additions</strong> and <strong>%d deletions</strong>
 diff.bin = BIN
 diff.view_file = View File
+diff.file_supressed = File diff supressed because it is too large
+diff.too_many_files = Some files were not shown because too many files changed in this diff
 
 release.releases = Releases
 release.new_release = New Release

models/git_diff.go

@@ -48,10 +48,10 @@ const (
 )
 
 type DiffLine struct {
-	LeftIdx       int
-	RightIdx      int
-	Type          DiffLineType
-	Content       string
+	LeftIdx  int
+	RightIdx int
+	Type     DiffLineType
+	Content  string
 }
 
 func (d *DiffLine) GetType() int {
@@ -161,6 +161,7 @@ type DiffFile struct {
 	IsBin              bool
 	IsRenamed          bool
 	Sections           []*DiffSection
+	IsIncomplete       bool
 }
 
 func (diffFile *DiffFile) GetType() int {
@@ -174,6 +175,7 @@ func (diffFile *DiffFile) GetHighlightClass() string {
 type Diff struct {
 	TotalAddition, TotalDeletion int
 	Files                        []*DiffFile
+	IsIncomplete                 bool
 }
 
 func (diff *Diff) NumFiles() int {
@@ -182,7 +184,7 @@ func (diff *Diff) NumFiles() int {
 
 const DIFF_HEAD = "diff --git "
 
-func ParsePatch(maxlines int, reader io.Reader) (*Diff, error) {
+func ParsePatch(maxLines, maxLineCharacteres, maxFiles int, reader io.Reader) (*Diff, error) {
 	var (
 		diff = &Diff{Files: make([]*DiffFile, 0)}
 
@@ -193,15 +195,12 @@ func ParsePatch(maxlines int, reader io.Reader) (*Diff, error) {
 
 		leftLine, rightLine int
 		lineCount           int
+		curFileLinesCount   int
 	)
 
 	input := bufio.NewReader(reader)
 	isEOF := false
-	for {
-		if isEOF {
-			break
-		}
-
+	for !isEOF {
 		line, err := input.ReadString('\n')
 		if err != nil {
 			if err == io.EOF {
@@ -216,20 +215,16 @@ func ParsePatch(maxlines int, reader io.Reader) (*Diff, error) {
 			line = line[:len(line)-1]
 		}
 
-		if strings.HasPrefix(line, "+++ ") || strings.HasPrefix(line, "--- ") {
-			continue
-		} else if len(line) == 0 {
+		if strings.HasPrefix(line, "+++ ") || strings.HasPrefix(line, "--- ") || len(line) == 0 {
 			continue
 		}
 
+		curFileLinesCount++
 		lineCount++
 
 		// Diff data too large, we only show the first about maxlines lines
-		if lineCount >= maxlines {
-			log.Warn("Diff data too large")
-			io.Copy(ioutil.Discard, reader)
-			diff.Files = nil
-			return diff, nil
+		if curFileLinesCount >= maxLines || len(line) >= maxLineCharacteres {
+			curFile.IsIncomplete = true
 		}
 
 		switch {
@@ -304,6 +299,12 @@ func ParsePatch(maxlines int, reader io.Reader) (*Diff, error) {
 				Sections: make([]*DiffSection, 0, 10),
 			}
 			diff.Files = append(diff.Files, curFile)
+			if len(diff.Files) >= maxFiles {
+				diff.IsIncomplete = true
+				io.Copy(ioutil.Discard, reader)
+				break
+			}
+			curFileLinesCount = 0
 
 			// Check file diff type.
 			for {
@@ -366,7 +367,7 @@ func ParsePatch(maxlines int, reader io.Reader) (*Diff, error) {
 	return diff, nil
 }
 
-func GetDiffRange(repoPath, beforeCommitID string, afterCommitID string, maxlines int) (*Diff, error) {
+func GetDiffRange(repoPath, beforeCommitID string, afterCommitID string, maxLines, maxLineCharacteres, maxFiles int) (*Diff, error) {
 	repo, err := git.OpenRepository(repoPath)
 	if err != nil {
 		return nil, err
@@ -405,7 +406,7 @@ func GetDiffRange(repoPath, beforeCommitID string, afterCommitID string, maxline
 	pid := process.Add(fmt.Sprintf("GetDiffRange (%s)", repoPath), cmd)
 	defer process.Remove(pid)
 
-	diff, err := ParsePatch(maxlines, stdout)
+	diff, err := ParsePatch(maxLines, maxLineCharacteres, maxFiles, stdout)
 	if err != nil {
 		return nil, fmt.Errorf("ParsePatch: %v", err)
 	}
@@ -417,6 +418,6 @@ func GetDiffRange(repoPath, beforeCommitID string, afterCommitID string, maxline
 	return diff, nil
 }
 
-func GetDiffCommit(repoPath, commitId string, maxlines int) (*Diff, error) {
-	return GetDiffRange(repoPath, "", commitId, maxlines)
+func GetDiffCommit(repoPath, commitId string, maxLines, maxLineCharacteres, maxFiles int) (*Diff, error) {
+	return GetDiffRange(repoPath, "", commitId, maxLines, maxLineCharacteres, maxFiles)
 }

modules/setting/setting.go

@@ -161,9 +161,11 @@ var (
 
 	// Git settings
 	Git struct {
-		MaxGitDiffLines int
-		GcArgs          []string `delim:" "`
-		Timeout         struct {
+		MaxGitDiffLines          int
+		MaxGitDiffLineCharacters int
+		MaxGitDiffFiles          int
+		GcArgs                   []string `delim:" "`
+		Timeout                  struct {
 			Migrate int
 			Mirror  int
 			Clone   int

routers/repo/commit.go

@@ -157,7 +157,8 @@ func Diff(ctx *context.Context) {
 	}
 
 	diff, err := models.GetDiffCommit(models.RepoPath(userName, repoName),
-		commitID, setting.Git.MaxGitDiffLines)
+		commitID, setting.Git.MaxGitDiffLines,
+		setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles)
 	if err != nil {
 		ctx.Handle(404, "GetDiffCommit", err)
 		return
@@ -212,7 +213,8 @@ func CompareDiff(ctx *context.Context) {
 	}
 
 	diff, err := models.GetDiffRange(models.RepoPath(userName, repoName), beforeCommitID,
-		afterCommitID, setting.Git.MaxGitDiffLines)
+		afterCommitID, setting.Git.MaxGitDiffLines,
+		setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles)
 	if err != nil {
 		ctx.Handle(404, "GetDiffRange", err)
 		return

routers/repo/pull.go

@@ -348,7 +348,8 @@ func ViewPullFiles(ctx *context.Context) {
 	}
 
 	diff, err := models.GetDiffRange(diffRepoPath,
-		startCommitID, endCommitID, setting.Git.MaxGitDiffLines)
+		startCommitID, endCommitID, setting.Git.MaxGitDiffLines,
+		setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles)
 	if err != nil {
 		ctx.Handle(500, "GetDiffRange", err)
 		return
@@ -545,7 +546,8 @@ func PrepareCompareDiff(
 	}
 
 	diff, err := models.GetDiffRange(models.RepoPath(headUser.Name, headRepo.Name),
-		prInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines)
+		prInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines,
+		setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles)
 	if err != nil {
 		ctx.Handle(500, "GetDiffRange", err)
 		return false

templates/repo/diff_box.tmpl

@@ -34,92 +34,120 @@
 	</div>
 
 	{{range $i, $file := .Diff.Files}}
-		{{$highlightClass := $file.GetHighlightClass}}
-		<div class="diff-file-box diff-box file-content" id="diff-{{.Index}}">
-			<h4 class="ui top attached normal header">
-				<div class="diff-counter count ui left">
-					{{if $file.IsBin}}
-						{{$.i18n.Tr "repo.diff.bin"}}
-					{{else if not $file.IsRenamed}}
-						<span class="add" data-line="{{.Addition}}">+ {{.Addition}}</span>
-						<span class="bar">
-							<span class="pull-left add"></span>
-							<span class="pull-left del"></span>
-						</span>
-						<span class="del" data-line="{{.Deletion}}">- {{.Deletion}}</span>
-					{{end}}
-				</div>
-				<span class="file">{{if $file.IsRenamed}}{{$file.OldName}} &rarr; {{end}}{{$file.Name}}</span>
-				<div class="ui right">
-					{{if $file.IsDeleted}}
-						<a class="ui basic tiny button" rel="nofollow" href="{{EscapePound $.BeforeSourcePath}}/{{EscapePound .Name}}">{{$.i18n.Tr "repo.diff.view_file"}}</a>
-					{{else}}
-						<a class="ui basic tiny button" rel="nofollow" href="{{EscapePound $.SourcePath}}/{{EscapePound .Name}}">{{$.i18n.Tr "repo.diff.view_file"}}</a>
-					{{end}}
-				</div>
-			</h4>
-			<div class="ui attached table segment">
-				{{if not $file.IsRenamed}}
-					{{$isImage := (call $.IsImageFile $file.Name)}}
-					{{if and $isImage}}
-						<div class="center">
-							<img src="{{$.RawPath}}/{{EscapePound .Name}}">
-						</div>
-					{{else}}
-						<div class="file-body file-code code-view code-diff">
-							<table>
-								<tbody>
-									{{if $.IsSplitStyle}}
-										{{range $j, $section := .Sections}}
-											{{range $k, $line := .Lines}}
-												<tr class="{{DiffLineTypeToStr .GetType}}-code nl-{{$k}} ol-{{$k}}">
-													<td class="lines-num lines-num-old">
-														<span rel="{{if $line.LeftIdx}}diff-{{Sha1 $file.Name}}L{{$line.LeftIdx}}{{end}}">{{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}}</span>
-													</td>
-													<td class="lines-code halfwidth">
-														<pre><code class="wrap {{if $highlightClass}}language-{{$highlightClass}}{{else}}nohighlight{{end}}">{{if $line.LeftIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}</code></pre>
-													</td>
-													<td class="lines-num lines-num-new">
-														<span rel="{{if $line.RightIdx}}diff-{{Sha1 $file.Name}}R{{$line.RightIdx}}{{end}}">{{if $line.RightIdx}}{{$line.RightIdx}}{{end}}</span>
-													</td>
-													<td class="lines-code halfwidth">
-														<pre><code class="wrap {{if $highlightClass}}language-{{$highlightClass}}{{else}}nohighlight{{end}}">{{if $line.RightIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}</code></pre>
-													</td>
-												</tr>
-											{{end}}
-										{{end}}
-									{{else}}
-										{{range $j, $section := .Sections}}
-											{{range $k, $line := .Lines}}
-												<tr class="{{DiffLineTypeToStr .GetType}}-code nl-{{$k}} ol-{{$k}}">
-													{{if eq .GetType 4}}
-													<td colspan="2" class="lines-num">
-														{{/* {{if gt $j 0}}<span class="fold octicon octicon-fold"></span>{{end}} */}}
-													</td>
-													{{else}}
-													<td class="lines-num lines-num-old">
-														<span rel="{{if $line.LeftIdx}}diff-{{Sha1 $file.Name}}L{{$line.LeftIdx}}{{end}}">{{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}}</span>
-													</td>
-													<td class="lines-num lines-num-new">
-														<span rel="{{if $line.RightIdx}}diff-{{Sha1 $file.Name}}R{{$line.RightIdx}}{{end}}">{{if $line.RightIdx}}{{$line.RightIdx}}{{end}}</span>
-													</td>
-													{{end}}
-													<td class="lines-code">
-														<pre><code class="{{if $highlightClass}}language-{{$highlightClass}}{{else}}nohighlight{{end}}">{{$section.GetComputedInlineDiffFor $line}}</code></pre>
-													</td>
-												</tr>
-											{{end}}
-										{{end}}
-									{{end}}
-								</tbody>
-							</table>
-						</div>
-					{{end}}
-				{{end}}
+		{{if $file.IsIncomplete}}
+			<div class="diff-file-box diff-box file-content">
+				<h4 class="ui top attached normal header">
+					{{$.i18n.Tr "repo.diff.file_supressed"}}
+					<div class="diff-counter count ui left">
+						{{if not $file.IsRenamed}}
+							<span class="add" data-line="{{.Addition}}">+ {{.Addition}}</span>
+							<span class="bar">
+								<span class="pull-left add"></span>
+								<span class="pull-left del"></span>
+							</span>
+							<span class="del" data-line="{{.Deletion}}">- {{.Deletion}}</span>
+						{{end}}
+					</div>
+					<span class="file">{{$file.Name}}</span>
+				</h4>
 			</div>
-		</div>
+		{{else}}
+			{{$highlightClass := $file.GetHighlightClass}}
+			<div class="diff-file-box diff-box file-content" id="diff-{{.Index}}">
+				<h4 class="ui top attached normal header">
+					<div class="diff-counter count ui left">
+						{{if $file.IsBin}}
+							{{$.i18n.Tr "repo.diff.bin"}}
+						{{else if not $file.IsRenamed}}
+							<span class="add" data-line="{{.Addition}}">+ {{.Addition}}</span>
+							<span class="bar">
+								<span class="pull-left add"></span>
+								<span class="pull-left del"></span>
+							</span>
+							<span class="del" data-line="{{.Deletion}}">- {{.Deletion}}</span>
+						{{end}}
+					</div>
+					<span class="file">{{if $file.IsRenamed}}{{$file.OldName}} &rarr; {{end}}{{$file.Name}}</span>
+					<div class="ui right">
+						{{if $file.IsDeleted}}
+							<a class="ui basic tiny button" rel="nofollow" href="{{EscapePound $.BeforeSourcePath}}/{{EscapePound .Name}}">{{$.i18n.Tr "repo.diff.view_file"}}</a>
+						{{else}}
+							<a class="ui basic tiny button" rel="nofollow" href="{{EscapePound $.SourcePath}}/{{EscapePound .Name}}">{{$.i18n.Tr "repo.diff.view_file"}}</a>
+						{{end}}
+					</div>
+				</h4>
+				<div class="ui attached table segment">
+					{{if not $file.IsRenamed}}
+						{{$isImage := (call $.IsImageFile $file.Name)}}
+						{{if and $isImage}}
+							<div class="center">
+								<img src="{{$.RawPath}}/{{EscapePound .Name}}">
+							</div>
+						{{else}}
+							<div class="file-body file-code code-view code-diff">
+								<table>
+									<tbody>
+										{{if $.IsSplitStyle}}
+											{{range $j, $section := .Sections}}
+												{{range $k, $line := .Lines}}
+													<tr class="{{DiffLineTypeToStr .GetType}}-code nl-{{$k}} ol-{{$k}}">
+														<td class="lines-num lines-num-old">
+															<span rel="{{if $line.LeftIdx}}diff-{{Sha1 $file.Name}}L{{$line.LeftIdx}}{{end}}">{{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}}</span>
+														</td>
+														<td class="lines-code halfwidth">
+															<pre><code class="wrap {{if $highlightClass}}language-{{$highlightClass}}{{else}}nohighlight{{end}}">{{if $line.LeftIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}</code></pre>
+														</td>
+														<td class="lines-num lines-num-new">
+															<span rel="{{if $line.RightIdx}}diff-{{Sha1 $file.Name}}R{{$line.RightIdx}}{{end}}">{{if $line.RightIdx}}{{$line.RightIdx}}{{end}}</span>
+														</td>
+														<td class="lines-code halfwidth">
+															<pre><code class="wrap {{if $highlightClass}}language-{{$highlightClass}}{{else}}nohighlight{{end}}">{{if $line.RightIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}</code></pre>
+														</td>
+													</tr>
+												{{end}}
+											{{end}}
+										{{else}}
+											{{range $j, $section := .Sections}}
+												{{range $k, $line := .Lines}}
+													<tr class="{{DiffLineTypeToStr .GetType}}-code nl-{{$k}} ol-{{$k}}">
+														{{if eq .GetType 4}}
+														<td colspan="2" class="lines-num">
+															{{/* {{if gt $j 0}}<span class="fold octicon octicon-fold"></span>{{end}} */}}
+														</td>
+														{{else}}
+														<td class="lines-num lines-num-old">
+															<span rel="{{if $line.LeftIdx}}diff-{{Sha1 $file.Name}}L{{$line.LeftIdx}}{{end}}">{{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}}</span>
+														</td>
+														<td class="lines-num lines-num-new">
+															<span rel="{{if $line.RightIdx}}diff-{{Sha1 $file.Name}}R{{$line.RightIdx}}{{end}}">{{if $line.RightIdx}}{{$line.RightIdx}}{{end}}</span>
+														</td>
+														{{end}}
+														<td class="lines-code">
+															<pre><code class="{{if $highlightClass}}language-{{$highlightClass}}{{else}}nohighlight{{end}}">{{$section.GetComputedInlineDiffFor $line}}</code></pre>
+														</td>
+													</tr>
+												{{end}}
+											{{end}}
+										{{end}}
+									</tbody>
+								</table>
+							</div>
+						{{end}}
+					{{end}}
+				</div>
+			</div>
+		{{end}}
 	<br>
 	{{end}}
+
+	{{if .Diff.IsIncomplete}}
+		<div class="diff-file-box diff-box file-content">
+			<h4 class="ui top attached normal header">
+				{{$.i18n.Tr "repo.diff.too_many_files"}}
+			</h4>
+		</div>
+	{{end}}
+
 	{{if .IsSplitStyle}}
 		<script>
 		(function() {