development/git
development/git
1
0
Fork 0
mirror of https://github.com/git/git synced 2024-11-05 04:53:18 +00:00
Code Issues Projects Releases Packages Wiki Activity
git/t/t3438-rebase-broken-files.sh

71 lines
1.5 KiB
Bash
Raw Normal View History

sequencer: detect author name errors in read_author_script() As we parse the author-script file, we check for missing or duplicate lines for GIT_AUTHOR_NAME, etc. But after reading the whole file, our final error conditional checks "date_i" twice and "name_i" not at all. This not only leads to us failing to abort, but we may do an out-of-bounds read on the string_list array. The bug goes back to 442c36bd08 (am: improve author-script error reporting, 2018-10-31), though the code was soon after moved to this spot by bcd33ec25f (add read_author_script() to libgit, 2018-10-31). It was presumably just a typo in 442c36bd08. We'll add test coverage for all the error cases here, though only the GIT_AUTHOR_NAME ones fail (even in a vanilla build they segfault consistently, but certainly with SANITIZE=address). Reported-by: Michael V. Scovetta <michael.scovetta@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-03 17:35:02 +00:00
#!/bin/sh
test_description='rebase behavior when on-disk files are broken'
sequencer API users: fix get_replay_opts() leaks Make the replay_opts_release() function added in the preceding commit non-static, and use it for freeing the "struct replay_opts" constructed for "rebase" and "revert". To safely call our new replay_opts_release() we'll need to stop calling it in sequencer_remove_state(), and instead call it where we allocate the "struct replay_opts" itself. This is because in e.g. do_interactive_rebase() we construct a "struct replay_opts" with "get_replay_opts()", and then call "complete_action()". If we get far enough in that function without encountering errors we'll call "pick_commits()" which (indirectly) calls sequencer_remove_state() at the end. But if we encounter errors anywhere along the way we'd punt out early, and not free() the memory we allocated. Remembering whether we previously called sequencer_remove_state() would be a hassle. Using a FREE_AND_NULL() pattern would also work, as it would be safe to call replay_opts_release() repeatedly. But let's fix this properly instead, by having the owner of the data free() it. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 19:08:08 +00:00
TEST_PASSES_SANITIZE_LEAK=true
sequencer: detect author name errors in read_author_script() As we parse the author-script file, we check for missing or duplicate lines for GIT_AUTHOR_NAME, etc. But after reading the whole file, our final error conditional checks "date_i" twice and "name_i" not at all. This not only leads to us failing to abort, but we may do an out-of-bounds read on the string_list array. The bug goes back to 442c36bd08 (am: improve author-script error reporting, 2018-10-31), though the code was soon after moved to this spot by bcd33ec25f (add read_author_script() to libgit, 2018-10-31). It was presumably just a typo in 442c36bd08. We'll add test coverage for all the error cases here, though only the GIT_AUTHOR_NAME ones fail (even in a vanilla build they segfault consistently, but certainly with SANITIZE=address). Reported-by: Michael V. Scovetta <michael.scovetta@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-03 17:35:02 +00:00
. ./test-lib.sh
test_expect_success 'set up conflicting branches' '
test_commit base file &&
git checkout -b branch1 &&
test_commit one file &&
git checkout -b branch2 HEAD^ &&
test_commit two file
'
create_conflict () {
test_when_finished "git rebase --abort" &&
git checkout -B tmp branch2 &&
test_must_fail git rebase branch1
}
check_resolve_fails () {
echo resolved >file &&
git add file &&
test_must_fail git rebase --continue
}
for item in NAME EMAIL DATE
do
test_expect_success "detect missing GIT_AUTHOR_$item" '
create_conflict &&
grep -v $item .git/rebase-merge/author-script >tmp &&
mv tmp .git/rebase-merge/author-script &&
check_resolve_fails
'
done
for item in NAME EMAIL DATE
do
test_expect_success "detect duplicate GIT_AUTHOR_$item" '
create_conflict &&
grep -i $item .git/rebase-merge/author-script >tmp &&
cat tmp >>.git/rebase-merge/author-script &&
check_resolve_fails
'
done
test_expect_success 'unknown key in author-script' '
create_conflict &&
echo "GIT_AUTHOR_BOGUS=${SQ}whatever${SQ}" \
>>.git/rebase-merge/author-script &&
check_resolve_fails
'
rebase: use child_process_clear() to clean In the run_am() function, we set up a child_process struct to run "git-am", allocating memory for its args and env strvecs. These are normally cleaned up when we call run_command(). But if we encounter certain errors, we exit the function early and try to clean up ourselves by clearing the am.args field. This leaks the "env" strvec. We should use child_process_clear() instead, which covers both. And more importantly, it future proofs us against the struct ever growing more allocated fields. These are unlikely errors to happen in practice, so they don't actually trigger the leak sanitizer in the tests. But we can add a new test which does exercise one of the paths (and fails SANITIZE=leak without this patch). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-22 10:35:02 +00:00
test_expect_success POSIXPERM,SANITY 'unwritable rebased-patches does not leak' '
>.git/rebased-patches &&
chmod a-w .git/rebased-patches &&
git checkout -b side HEAD^ &&
test_commit unrelated &&
test_must_fail git rebase --apply --onto tmp HEAD^
'
sequencer: detect author name errors in read_author_script() As we parse the author-script file, we check for missing or duplicate lines for GIT_AUTHOR_NAME, etc. But after reading the whole file, our final error conditional checks "date_i" twice and "name_i" not at all. This not only leads to us failing to abort, but we may do an out-of-bounds read on the string_list array. The bug goes back to 442c36bd08 (am: improve author-script error reporting, 2018-10-31), though the code was soon after moved to this spot by bcd33ec25f (add read_author_script() to libgit, 2018-10-31). It was presumably just a typo in 442c36bd08. We'll add test coverage for all the error cases here, though only the GIT_AUTHOR_NAME ones fail (even in a vanilla build they segfault consistently, but certainly with SANITIZE=address). Reported-by: Michael V. Scovetta <michael.scovetta@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-03 17:35:02 +00:00
test_done
Reference in a new issue Copy permalink