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 Nipunn Koorapati (3): test-lib-functions: handle --add in test_config negative-refspec: fix segfault on : refspec negative-refspec: improve comment on query_matches_negative_refspec remote.c | 16 +++++++++++++--- t/t5582-fetch-negative-refspec.sh | 22 ++++++++++++++++++++++ t/test-lib-functions.sh | 9 ++++++++- 3 files changed, 43 insertions(+), 4 deletions(-) base-commit: 6d3ef5b467eccd2769f1aa1c555d317d3c8dc707 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-820%2Fnipunn1313%2Fnk%2Fpush-refspec-segfault-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-820/nipunn1313/nk/push-refspec-segfault-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/820 Range-diff vs v2: -: ----------- > 1: 733c674bd19 test-lib-functions: handle --add in test_config 1: e42200b644a ! 2: 20cff2f5c59 negative-refspec: fix segfault on : refspec @@ Commit message remote.origin.push is set to ":", then refspec->src is NULL, causing a segfault within strcmp - Added testing for this case in fetch-negative-refspec + 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 Signed-off-by: Nipunn Koorapati <nipunn@xxxxxxxxxxx> @@ remote.c: static int query_matches_negative_refspec(struct refspec *rs, struct r + } else if (refspec->matching) { + /* For the special matching refspec, any query should match */ + string_list_append(&reversed, needle); -+ } else if (refspec->src == NULL) { ++ } else if (!refspec->src) { + BUG("refspec->src should not be null here"); + } else if (!strcmp(needle, refspec->src)) { + string_list_append(&reversed, refspec->src); @@ t/t5582-fetch-negative-refspec.sh: test_expect_success "fetch --prune with negat ' +test_expect_success "push with matching ':' refspec" ' -+ ( -+ cd two && -+ git config remote.one.push : && -+ # Fails w/ tip behind counterpart - but should not segfault -+ test_must_fail git push one master && ++ test_config -C two remote.one.push : && ++ # Fails w/ tip behind counterpart - but should not segfault ++ 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 ++' + -+ git config remote.one.push +: && -+ # Fails w/ tip behind counterpart - but should not segfault -+ test_must_fail git push one master && ++test_expect_success "push with matching and negative refspec" ' ++ test_config -C two --add remote.one.push : && ++ # Fails to push master w/ tip behind counterpart ++ test_must_fail git -C two push one && + -+ git config --unset remote.one.push -+ ) ++ # If master is in negative refspec, then the command will succeed ++ test_config -C two --add remote.one.push ^refs/heads/master && ++ git -C two push one +' + test_done 2: 8da8d9cd1c5 = 3: 0fd4e9f7459 negative-refspec: improve comment on query_matches_negative_refspec -- gitgitgadget
