From 7c55efcfa1700e90ea9fb027b7e890e30662261d Mon Sep 17 00:00:00 2001
From: Billy Keyes <bluekeyes@gmail.com>
Date: Sun, 12 Sep 2021 15:32:31 -0700
Subject: [PATCH] Return errors for empty text fragments

This fixes two issues:

  1. Fragments with only context lines were accepted
  2. Fragments with only a "no newline" marker caused a panic

The panic was found by go-fuzz. While fixing that issue, I
discovered the first problem with other types of empty fragments while
comparing behavior against 'git apply'.

Also extract some helper functions and modify the loop conditions to
clean things up while making changes in this area.
---
 gitdiff/text.go      | 48 +++++++++++++++++++++++++++-----------------
 gitdiff/text_test.go | 18 +++++++++++++++++
 2 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/gitdiff/text.go b/gitdiff/text.go
index 04de8f4..ee30792 100644
--- a/gitdiff/text.go
+++ b/gitdiff/text.go
@@ -79,14 +79,8 @@ func (p *parser) ParseTextChunk(frag *TextFragment) error {
 		return p.Errorf(0, "no content following fragment header")
 	}
 
-	isNoNewlineLine := func(s string) bool {
-		// test for "\ No newline at end of file" by prefix because the text
-		// changes by locale (git claims all versions are at least 12 chars)
-		return len(s) >= 12 && s[:2] == "\\ "
-	}
-
 	oldLines, newLines := frag.OldLines, frag.NewLines
-	for {
+	for oldLines > 0 || newLines > 0 {
 		line := p.Line(0)
 		op, data := line[0], line[1:]
 
@@ -113,13 +107,14 @@ func (p *parser) ParseTextChunk(frag *TextFragment) error {
 			frag.LinesAdded++
 			frag.TrailingContext = 0
 			frag.Lines = append(frag.Lines, Line{OpAdd, data})
-		default:
+		case '\\':
 			// this may appear in middle of fragment if it's for a deleted line
-			if isNoNewlineLine(line) {
-				last := &frag.Lines[len(frag.Lines)-1]
-				last.Line = strings.TrimSuffix(last.Line, "\n")
+			if isNoNewlineMarker(line) {
+				removeLastNewline(frag)
 				break
 			}
+			fallthrough
+		default:
 			// TODO(bkeyes): if this is because we hit the next header, it
 			// would be helpful to return the miscounts line error. We could
 			// either test for the common headers ("@@ -", "diff --git") or
@@ -128,11 +123,6 @@ func (p *parser) ParseTextChunk(frag *TextFragment) error {
 			return p.Errorf(0, "invalid line operation: %q", op)
 		}
 
-		next := p.Line(1)
-		if oldLines <= 0 && newLines <= 0 && !isNoNewlineLine(next) {
-			break
-		}
-
 		if err := p.Next(); err != nil {
 			if err == io.EOF {
 				break
@@ -145,13 +135,35 @@ func (p *parser) ParseTextChunk(frag *TextFragment) error {
 		hdr := max(frag.OldLines-oldLines, frag.NewLines-newLines) + 1
 		return p.Errorf(-hdr, "fragment header miscounts lines: %+d old, %+d new", -oldLines, -newLines)
 	}
+	if frag.LinesAdded == 0 && frag.LinesDeleted == 0 {
+		return p.Errorf(0, "fragment contains no changes")
+	}
 
-	if err := p.Next(); err != nil && err != io.EOF {
-		return err
+	// check for a final "no newline" marker since it is not included in the
+	// counters used to stop the loop above
+	if isNoNewlineMarker(p.Line(0)) {
+		removeLastNewline(frag)
+		if err := p.Next(); err != nil && err != io.EOF {
+			return err
+		}
 	}
+
 	return nil
 }
 
+func isNoNewlineMarker(s string) bool {
+	// test for "\ No newline at end of file" by prefix because the text
+	// changes by locale (git claims all versions are at least 12 chars)
+	return len(s) >= 12 && s[:2] == "\\ "
+}
+
+func removeLastNewline(frag *TextFragment) {
+	if len(frag.Lines) > 0 {
+		last := &frag.Lines[len(frag.Lines)-1]
+		last.Line = strings.TrimSuffix(last.Line, "\n")
+	}
+}
+
 func parseRange(s string) (start int64, end int64, err error) {
 	parts := strings.SplitN(s, ",", 2)
 
diff --git a/gitdiff/text_test.go b/gitdiff/text_test.go
index d1caed1..990b3bc 100644
--- a/gitdiff/text_test.go
+++ b/gitdiff/text_test.go
@@ -317,6 +317,24 @@ func TestParseTextChunk(t *testing.T) {
 			},
 			Err: true,
 		},
+		"onlyContext": {
+			Input: ` context line
+ context line
+`,
+			Fragment: TextFragment{
+				OldLines: 2,
+				NewLines: 2,
+			},
+			Err: true,
+		},
+		"unexpectedNoNewlineMarker": {
+			Input: `\ No newline at end of file`,
+			Fragment: TextFragment{
+				OldLines: 1,
+				NewLines: 1,
+			},
+			Err: true,
+		},
 	}
 
 	for name, test := range tests {