apply --reject: overwrite existing `.rej` symlink if it exists

The `git apply --reject` is expected to write out `.rej` files in case
one or more hunks fail to apply cleanly. Historically, the command
overwrites any existing `.rej` files. The idea being that
apply/reject/edit cycles are relatively common, and the generated `.rej`
files are not considered precious.

But the command does not overwrite existing `.rej` symbolic links, and
instead follows them. This is unsafe because the same patch could
potentially create such a symbolic link and point at arbitrary paths
outside the current worktree, and `git apply` would write the contents
of the `.rej` file into that location.

Therefore, let's make sure that any existing `.rej` file or symbolic
link is removed before writing it.

Reported-by: RyotaK <ryotak.mail@gmail.com>
Helped-by: Taylor Blau <me@ttaylorr.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Linus Torvalds <torvalds@linuxfoundation.org>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

diff --git a/apply.c b/apply.c
index d80382c940619b60cf23a9d535b56f92122a2a37..6634e9c5108723d71faa7021c6523293bd91003e 100644 (file)
--- a/apply.c
+++ b/apply.c
@@ -4558,7 +4558,7 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
        FILE *rej;
        char namebuf[PATH_MAX];
        struct fragment *frag;
-       int cnt = 0;
+       int fd, cnt = 0;
        struct strbuf sb = STRBUF_INIT;
 
        for (cnt = 0, frag = patch->fragments; frag; frag = frag->next) {
@@ -4598,7 +4598,17 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
        memcpy(namebuf, patch->new_name, cnt);
        memcpy(namebuf + cnt, ".rej", 5);
 
-       rej = fopen(namebuf, "w");
+       fd = open(namebuf, O_CREAT | O_EXCL | O_WRONLY, 0666);
+       if (fd < 0) {
+               if (errno != EEXIST)
+                       return error_errno(_("cannot open %s"), namebuf);
+               if (unlink(namebuf))
+                       return error_errno(_("cannot unlink '%s'"), namebuf);
+               fd = open(namebuf, O_CREAT | O_EXCL | O_WRONLY, 0666);
+               if (fd < 0)
+                       return error_errno(_("cannot open %s"), namebuf);
+       }
+       rej = fdopen(fd, "w");
        if (!rej)
                return error_errno(_("cannot open %s"), namebuf);
 

diff --git a/t/t4115-apply-symlink.sh b/t/t4115-apply-symlink.sh
index 14e0f4d70590d9a0221a1b1c51171c71fa2bf648..2d03c4e4d19abac87da0b0543ad423790081d7b2 100755 (executable)
--- a/t/t4115-apply-symlink.sh
+++ b/t/t4115-apply-symlink.sh
@@ -125,4 +125,19 @@ test_expect_success SYMLINKS 'symlink escape when deleting file' '
        test_path_is_file .git/delete-me
 '
 
+test_expect_success SYMLINKS '--reject removes .rej symlink if it exists' '
+       test_when_finished "git reset --hard && git clean -dfx" &&
+
+       test_commit file &&
+       echo modified >file.t &&
+       git diff -- file.t >patch &&
+       echo modified-again >file.t &&
+
+       ln -s foo file.t.rej &&
+       test_must_fail git apply patch --reject 2>err &&
+       test_i18ngrep "Rejected hunk" err &&
+       test_path_is_missing foo &&
+       test_path_is_file file.t.rej
+'
+
 test_done