Anchor patch hunks as git apply does, refuse patches touching non-100644 files by their real mode, survive bare-CR context lines (issue 1604 review)

modules/gitea/cmd/forge/applypatch_test.go

@@ -75,7 +75,8 @@ func TestAPatchThatCreatesDeletesAndRenamesIsApplied(t *testing.T) {
 func TestAHunkMovedByEarlierLinesIsFoundAndNoNewlineIsKept(t *testing.T) {
 	f := sectionFake(t)
 	f.trees["c0"]["x.txt"] = "pre\npre\nabc §\nd\ne"
-	patch := "--- a/x.txt\n+++ b/x.txt\n@@ -1,3 +1,3 @@\n abc §\n d\n-e\n\\ No newline at end of file\n+f\n\\ No newline at end of file\n"
+	// the hunk below starts at line 2, though its lines are at 3: the tool finds them one line on
+	patch := "--- a/x.txt\n+++ b/x.txt\n@@ -2,3 +2,3 @@\n abc §\n d\n-e\n\\ No newline at end of file\n+f\n\\ No newline at end of file\n"
 	got, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/n", "patch": patch}))
 	if err != nil {
 		t.Fatal(err)
@@ -130,3 +131,59 @@ func TestADefaultBranchIsStillRefusedForAPatch(t *testing.T) {
 		t.Fatalf("%v", err)
 	}
 }
+
+// Review round 1.
+
+func appliedTo(t *testing.T, content, patch string) (string, error) {
+	t.Helper()
+	f := sectionFake(t)
+	f.trees["c0"]["y.txt"] = content
+	got, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/y", "patch": patch}))
+	if err != nil {
+		return "", err
+	}
+	return f.trees[got.Commit]["y.txt"], nil
+}
+
+func TestAHunkAtLineOneMustMatchAtTheStartOfTheFile(t *testing.T) {
+	patch := "--- a/y.txt\n+++ b/y.txt\n@@ -1,3 +1,3 @@\n-a\n+A\n b\n c\n"
+	if got, err := appliedTo(t, "X\nb\nc\nzzz\na\nb\nc\n", patch); err == nil {
+		t.Fatalf("applied in the middle: %q", got)
+	}
+	if got, err := appliedTo(t, "a\nb\nc\nzzz\n", patch); err != nil || got != "A\nb\nc\nzzz\n" {
+		t.Fatalf("%q %v", got, err)
+	}
+}
+
+func TestAHunkWithNoTrailingContextMustMatchAtTheEndOfTheFile(t *testing.T) {
+	patch := "--- a/y.txt\n+++ b/y.txt\n@@ -1,3 +1,4 @@\n a\n b\n c\n+NEW\n"
+	if got, err := appliedTo(t, "a\nb\nc\nd\ne\n", patch); err == nil {
+		t.Fatalf("added in the middle: %q", got)
+	}
+	patch = "--- a/y.txt\n+++ b/y.txt\n@@ -3,3 +3,4 @@\n a\n b\n c\n+NEW\n"
+	if got, err := appliedTo(t, "x\ny\na\nb\nc\n", patch); err != nil || got != "x\ny\na\nb\nc\nNEW\n" {
+		t.Fatalf("%q %v", got, err)
+	}
+}
+
+func TestAnExistingExecutableFileIsNotSilentlyMadeNonExecutable(t *testing.T) {
+	for _, patch := range []string{
+		"diff --git a/run.sh b/run.sh2\nsimilarity index 100%\nrename from run.sh\nrename to run.sh2\n",
+		"--- a/run.sh\n+++ b/run.sh\n@@ -1,2 +1,2 @@\n #!/bin/sh\n-echo a\n+echo b\n",
+	} {
+		f := sectionFake(t)
+		f.modes = map[string]string{"run.sh": "100755"}
+		_, err := commit(context.Background(), f.serve(), ask(t, map[string]any{"branch": "fix/m", "patch": patch}))
+		if err == nil || !strings.Contains(err.Error(), "100755") || len(f.writes) != 0 {
+			t.Errorf("%v (%d writes)", err, len(f.writes))
+		}
+	}
+}
+
+func TestAPatchWithCRLFLinesAndABareCRContextLineDoesNotPanic(t *testing.T) {
+	patch := "--- a/y.txt\r\n+++ b/y.txt\r\n@@ -1,3 +1,3 @@\r\n a\r\n\r\n-c\r\n+C\r\n"
+	got, err := appliedTo(t, "a\r\n\r\nc\r\n", patch)
+	if err != nil || got != "a\r\n\r\nC\r\n" {
+		t.Fatalf("%q %v", got, err)
+	}
+}