If remote.origin.push was set to ":", git segfaults during a push operation, due to bad parsing logic in query_matches_negative_refspec. Per bisect, the bug was introduced in: c0192df630 (refspec: add support for negative refspecs, 2020-09-30) We found this issue when rolling out git 2.29 at Dropbox - as several folks had "push = :" in their configuration. I based my diff off the master branch, but also confirmed that it patches cleanly onto maint - if the maintainers would like to also fix the segfault on 2.29 Update since Patch series V1: * Handled matching refspec explicitly * Added testing for "+:" case * Added comment explaining how the two loops work together Update since Patch series V2 * style suggestion in remote.c * Use test_config * Add test for a case with a matching refspec + negative refspec * Fix test_config to work with --add * Updated commit message to describe what git is told to do instead of segfaulting Update since Patch series V3 * Removed commit modifying test_config * Remove segfault-related comments in test * Consolidate the three tests to two tests (1st and 3rd test overlapped in functionality) * Base the patch series on the maint branch - since the bug affects 2.29.2 Appreciate the reviews from Junio and Eric! Happy Holidays! Nipunn Koorapati (2): negative-refspec: fix segfault on : refspec negative-refspec: improve comment on query_matches_negative_refspec remote.c | 16 +++++++++++++--- t/t5582-fetch-negative-refspec.sh | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) base-commit: 898f80736c75878acc02dc55672317fcc0e0a5a6 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-820%2Fnipunn1313%2Fnk%2Fpush-refspec-segfault-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-820/nipunn1313/nk/push-refspec-segfault-v4 Pull-Request: https://github.com/gitgitgadget/git/pull/820 Range-diff vs v3: 1: 733c674bd19 < -: ----------- test-lib-functions: handle --add in test_config 2: 20cff2f5c59 ! 1: e59ff29bdef negative-refspec: fix segfault on : refspec @@ Commit message (refspec: add support for negative refspecs, 2020-09-30) looks at refspec->src assuming it is never NULL, however when remote.origin.push is set to ":", then refspec->src is NULL, - causing a segfault within strcmp + causing a segfault within strcmp. Tell git to handle matching refspec by adding the needle to the set of positively matched refspecs, since matching ":" refspecs match anything as src. - Added testing for matching refspec pushes fetch-negative-refspec - both individually and in combination with a negative refspec + Add test for matching refspec pushes fetch-negative-refspec + both individually and in combination with a negative refspec. Signed-off-by: Nipunn Koorapati <nipunn@xxxxxxxxxxx> @@ t/t5582-fetch-negative-refspec.sh: test_expect_success "fetch --prune with negat ) ' -+test_expect_success "push with matching ':' refspec" ' ++test_expect_success "push with matching : and negative refspec" ' + test_config -C two remote.one.push : && -+ # Fails w/ tip behind counterpart - but should not segfault -+ test_must_fail git -C two push one -+' ++ # Fails to push master w/ tip behind counterpart ++ test_must_fail git -C two push one && + -+test_expect_success "push with matching '+:' refspec" ' -+ test_config -C two remote.one.push +: && -+ # Fails w/ tip behind counterpart - but should not segfault -+ test_must_fail git -C two push one ++ # If master is in negative refspec, then the command will not attempt ++ # to push and succeed. ++ # We do not need test_config here as we are updating remote.one.push ++ # again. The teardown of the first test_config will do --unset-all ++ git -C two config --add remote.one.push ^refs/heads/master && ++ git -C two push one +' + -+test_expect_success "push with matching and negative refspec" ' -+ test_config -C two --add remote.one.push : && ++test_expect_success "push with matching +: and negative refspec" ' ++ test_config -C two remote.one.push +: && + # Fails to push master w/ tip behind counterpart + test_must_fail git -C two push one && + -+ # If master is in negative refspec, then the command will succeed -+ test_config -C two --add remote.one.push ^refs/heads/master && ++ # If master is in negative refspec, then the command will not attempt ++ # to push and succeed ++ git -C two config --add remote.one.push ^refs/heads/master && + git -C two push one +' + 3: 0fd4e9f7459 = 2: 20575407cc0 negative-refspec: improve comment on query_matches_negative_refspec -- gitgitgadget
