[PATCH v3 0/3] negative-refspec: fix segfault on : refspec
- From
Nipunn Koorapati via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Dec 21, 2020, 02:05 UTC
- Message-ID
- <pull.820.v3.git.1608516320.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.820.v2.git.1608415117.gitgitgadget@gmail.com>
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@dropbox.com>
@@ 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