[PATCH 0/3] sparse-checkout: fix segfault on malformed patterns

This series fixes some issues with parsing sparse-checkout patterns when
core.sparseCheckoutCone is enabled but the sparse-checkout file itself
contains patterns that don't match the cone mode format.

The first patch fixes a segfault first reported in [1]. The other two
patches are from an earlier submission [2] that never got picked up and I
lost track of. There was another patch involving 'git sparse-checkout init
--cone' that isn't necessary, especially with Elijah doing some work in that
space right now.

[1] https://github.com/git-for-windows/git/issues/3498 [2]
https://lore.kernel.org/git/pull.1043.git.1632160658.gitgitgadget@gmail.com

Thanks, -Stolee

Derrick Stolee (3):
  sparse-checkout: fix segfault on malformed patterns
  sparse-checkout: fix OOM error with mixed patterns
  sparse-checkout: refuse to add to bad patterns

 builtin/sparse-checkout.c          |  5 ++++-
 dir.c                              |  5 +----
 t/t1091-sparse-checkout-builtin.sh | 31 +++++++++++++++++++++++++++++-
 3 files changed, 35 insertions(+), 6 deletions(-)


base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1069%2Fderrickstolee%2Fsparse-checkout%2Finput-bug-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1069/derrickstolee/sparse-checkout/input-bug-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1069
-- 
gitgitgadget

From: Derrick Stolee <redacted>

Then core.sparseCheckoutCone is enabled, the sparse-checkout patterns are
used to populate two hashsets that accelerate pattern matching. If the user
modifies the sparse-checkout file outside of the 'sparse-checkout' builtin,
then strange patterns can happen, triggering some error checks.

One of these error checks is possible to hit when some special characters
exist in a line. A warning message is correctly written to stderr, but then
there is additional logic that attempts to remove the line from the hashset
and free the data. This leads to a segfault in the 'git sparse-checkout
list' command because it iterates over the contents of the hashset, which is
no invalid.

The fix here is to stop trying to remove from the hashset. Better to leave
bad data in the sparse-checkout matching logic (with a warning) than to
segfault. If we are in this state, then we are already traversing into
undefined behavior, so this change to keep the entry in the hashset is no
worse than removing it.

Add a test that triggers the segfault without the code change.

Reported-by: John Burnett <redacted>
Signed-off-by: Derrick Stolee <redacted>
---
 dir.c                              |  3 ---
 t/t1091-sparse-checkout-builtin.sh | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/dir.c b/dir.c
index 5aa6fbad0b7..0693c7cb3ee 100644
--- a/dir.c
+++ b/dir.c

@@ -819,9 +819,6 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
 		/* we already included this at the parent level */
 		warning(_("your sparse-checkout file may have issues: pattern '%s' is repeated"),
 			given->pattern);
-		hashmap_remove(&pl->parent_hashmap, &translated->ent, &data);
-		free(data);
-		free(translated);
 	}
 
 	return;

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 272ba1b566b..c72b8ee2e7b 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -708,4 +708,19 @@ test_expect_success 'cone mode clears ignored subdirectories' '
 	test_cmp expect out
 '
 
+test_expect_success 'malformed cone-mode patterns' '
+	git -C repo sparse-checkout init --cone &&
+	mkdir -p repo/foo/bar &&
+	touch repo/foo/bar/x repo/foo/y &&
+	cat >repo/.git/info/sparse-checkout <<-\EOF &&
+	/*
+	!/*/
+	/foo/
+	!/foo/*/
+	/foo/\*/
+	EOF
+	cat repo/.git/info/sparse-checkout &&
+	git -C repo sparse-checkout list
+'
+
 test_done

-- 
gitgitgadget

From: Derrick Stolee <redacted>

Add a test to t1091-sparse-checkout-builtin.sh that would result in an
infinite loop and out-of-memory error before this change. The issue
relies on having non-cone-mode patterns while trying to modify the
patterns in cone-mode.

The fix is simple, allowing us to break from the loop when the input
path does not contain a slash, as the "dir" pattern we added does not.

This is only a fix to the critical out-of-memory error. A better
response to such a strange state will follow in a later change.

Reported-by: Calbabreaker <redacted>
Helped-by: Taylor Blau [off-list ref]
Signed-off-by: Derrick Stolee <redacted>
---
 builtin/sparse-checkout.c          |  2 +-
 t/t1091-sparse-checkout-builtin.sh | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index d0f5c4702be..9ccdcde9832 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c

@@ -483,7 +483,7 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat
 		char *oldpattern = e->pattern;
 		size_t newlen;
 
-		if (slash == e->pattern)
+		if (!slash || slash == e->pattern)
 			break;
 
 		newlen = slash - e->pattern;

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index c72b8ee2e7b..67ce24c9c83 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -103,6 +103,17 @@ test_expect_success 'clone --sparse' '
 	check_files clone a
 '
 
+test_expect_success 'switching to cone mode with non-cone mode patterns' '
+	git init bad-patterns &&
+	(
+		cd bad-patterns &&
+		git sparse-checkout init &&
+		git sparse-checkout add dir &&
+		git config core.sparseCheckoutCone true &&
+		git sparse-checkout add dir
+	)
+'
+
 test_expect_success 'interaction with clone --no-checkout (unborn index)' '
 	git clone --no-checkout "file://$(pwd)/repo" clone_no_checkout &&
 	git -C clone_no_checkout sparse-checkout init --cone &&

-- 
gitgitgadget

From: Derrick Stolee <redacted>

When in cone mode sparse-checkout, it is unclear how 'git
sparse-checkout add <dir1> ...' should behave if the existing
sparse-checkout file does not match the cone mode patterns. Change the
behavior to fail with an error message about the existing patterns.

Also, all cone mode patterns start with a '/' character, so add that
restriction. This is necessary for our example test 'cone mode: warn on
bad pattern', but also requires modifying the example sparse-checkout
file we use to test the warnings related to recognizing cone mode
patterns.

This error checking would cause a failure further down the test script
because of a test that adds non-cone mode patterns without cleaning them
up. Perform that cleanup as part of the test now.

Signed-off-by: Derrick Stolee <redacted>
---
 builtin/sparse-checkout.c          | 3 +++
 dir.c                              | 2 +-
 t/t1091-sparse-checkout-builtin.sh | 7 +++++--
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 9ccdcde9832..6580075a631 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c

@@ -598,6 +598,9 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 		die(_("unable to load existing sparse-checkout patterns"));
 	free(sparse_filename);
 
+	if (!existing.use_cone_patterns)
+		die(_("existing sparse-checkout patterns do not use cone mode"));
+
 	hashmap_for_each_entry(&existing.recursive_hashmap, &iter, pe, ent) {
 		if (!hashmap_contains_parent(&pl->recursive_hashmap,
 					pe->pattern, &buffer) ||

diff --git a/dir.c b/dir.c
index 0693c7cb3ee..a5dddafa16d 100644
--- a/dir.c
+++ b/dir.c

@@ -727,7 +727,7 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
 	}
 
 	if (given->patternlen < 2 ||
-	    *given->pattern == '*' ||
+	    *given->pattern != '/' ||
 	    strstr(given->pattern, "**")) {
 		/* Not a cone pattern. */
 		warning(_("unrecognized pattern: '%s'"), given->pattern);

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 67ce24c9c83..2ed247d75a5 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -110,7 +110,8 @@ test_expect_success 'switching to cone mode with non-cone mode patterns' '
 		git sparse-checkout init &&
 		git sparse-checkout add dir &&
 		git config core.sparseCheckoutCone true &&
-		git sparse-checkout add dir
+		test_must_fail git sparse-checkout add dir 2>err &&
+		grep "existing sparse-checkout patterns do not use cone mode" err
 	)
 '
 

@@ -176,12 +177,14 @@ test_expect_success 'set sparse-checkout using --stdin' '
 '
 
 test_expect_success 'add to sparse-checkout' '
-	cat repo/.git/info/sparse-checkout >expect &&
+	cat repo/.git/info/sparse-checkout >old &&
+	test_when_finished cp old repo/.git/info/sparse-checkout &&
 	cat >add <<-\EOF &&
 	pattern1
 	/folder1/
 	pattern2
 	EOF
+	cat old >expect &&
 	cat add >>expect &&
 	git -C repo sparse-checkout add --stdin <add &&
 	git -C repo sparse-checkout list >actual &&

-- 
gitgitgadget

On Tue, Dec 7, 2021 at 12:02 PM Derrick Stolee via GitGitGadget
[off-list ref] wrote:

From: Derrick Stolee <redacted>

Then core.sparseCheckoutCone is enabled, the sparse-checkout patterns are
used to populate two hashsets that accelerate pattern matching. If the user
modifies the sparse-checkout file outside of the 'sparse-checkout' builtin,
then strange patterns can happen, triggering some error checks.

One of these error checks is possible to hit when some special characters
exist in a line. A warning message is correctly written to stderr, but then
there is additional logic that attempts to remove the line from the hashset
and free the data. This leads to a segfault in the 'git sparse-checkout
list' command because it iterates over the contents of the hashset, which is
no invalid.

s/no invalid/now invalid/ ?

On Tue, Dec 7, 2021 at 12:02 PM Derrick Stolee via GitGitGadget
[off-list ref] wrote:

This series fixes some issues with parsing sparse-checkout patterns when
core.sparseCheckoutCone is enabled but the sparse-checkout file itself
contains patterns that don't match the cone mode format.

I was only able to find what I think is a small typo in one of the
commits.  Everything else looks good to me.

The first patch fixes a segfault first reported in [1]. The other two
patches are from an earlier submission [2] that never got picked up and I
lost track of. There was another patch involving 'git sparse-checkout init

Sorry for missing that series earlier.  Glad we've got some of them now.

--cone' that isn't necessary, especially with Elijah doing some work in that
space right now.

[1] https://github.com/git-for-windows/git/issues/3498 [2]
https://lore.kernel.org/git/pull.1043.git.1632160658.gitgitgadget@gmail.com

Thanks, -Stolee

Derrick Stolee (3):
  sparse-checkout: fix segfault on malformed patterns
  sparse-checkout: fix OOM error with mixed patterns
  sparse-checkout: refuse to add to bad patterns

 builtin/sparse-checkout.c          |  5 ++++-
 dir.c                              |  5 +----
 t/t1091-sparse-checkout-builtin.sh | 31 +++++++++++++++++++++++++++++-
 3 files changed, 35 insertions(+), 6 deletions(-)


base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1069%2Fderrickstolee%2Fsparse-checkout%2Finput-bug-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1069/derrickstolee/sparse-checkout/input-bug-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1069
--
gitgitgadget

On 12/7/2021 4:51 PM, Elijah Newren wrote:

On Tue, Dec 7, 2021 at 12:02 PM Derrick Stolee via GitGitGadget
[off-list ref] wrote:

quoted

This series fixes some issues with parsing sparse-checkout patterns when
core.sparseCheckoutCone is enabled but the sparse-checkout file itself
contains patterns that don't match the cone mode format.

I was only able to find what I think is a small typo in one of the
commits.  Everything else looks good to me.

Thanks. I'll give this a few more days for more feedback before sending
a v2 with that fix.
 

quoted

The first patch fixes a segfault first reported in [1]. The other two
patches are from an earlier submission [2] that never got picked up and I
lost track of. There was another patch involving 'git sparse-checkout init

Sorry for missing that series earlier.  Glad we've got some of them now.

The fault was my own for dropping this during a particularly busy time.

Thanks,
-Stolee

This series fixes some issues with parsing sparse-checkout patterns when
core.sparseCheckoutCone is enabled but the sparse-checkout file itself
contains patterns that don't match the cone mode format.

The first patch fixes a segfault first reported in [1]. The other two
patches are from an earlier submission [2] that never got picked up and I
lost track of. There was another patch involving 'git sparse-checkout init
--cone' that isn't necessary, especially with Elijah doing some work in that
space right now.

[1] https://github.com/git-for-windows/git/issues/3498 [2]
https://lore.kernel.org/git/pull.1043.git.1632160658.gitgitgadget@gmail.com

Thanks, -Stolee

Derrick Stolee (4):
  sparse-checkout: fix segfault on malformed patterns
  sparse-checkout: fix OOM error with mixed patterns
  sparse-checkout: refuse to add to bad patterns
  amend! sparse-checkout: fix segfault on malformed patterns

 builtin/sparse-checkout.c          |  5 ++++-
 dir.c                              |  5 +----
 t/t1091-sparse-checkout-builtin.sh | 31 +++++++++++++++++++++++++++++-
 3 files changed, 35 insertions(+), 6 deletions(-)


base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1069%2Fderrickstolee%2Fsparse-checkout%2Finput-bug-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1069/derrickstolee/sparse-checkout/input-bug-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1069

Range-diff vs v1:

 1:  becbee16d2e ! 1:  a0e3dd335c9 sparse-checkout: fix segfault on malformed patterns
     @@ Commit message
          Add a test that triggers the segfault without the code change.
      
          Reported-by: John Burnett [off-list ref]
     +    Reviewed-by: Elijah Newren [off-list ref]
          Signed-off-by: Derrick Stolee [off-list ref]
      
       ## dir.c ##
 2:  239bf23eacb ! 2:  86fbf130c03 sparse-checkout: fix OOM error with mixed patterns
     @@ Commit message
      
          Reported-by: Calbabreaker [off-list ref]
          Helped-by: Taylor Blau [off-list ref]
     +    Reviewed-by: Elijah Newren [off-list ref]
          Signed-off-by: Derrick Stolee [off-list ref]
      
       ## builtin/sparse-checkout.c ##
 3:  cc52fb2b0b7 ! 3:  5d096e380a4 sparse-checkout: refuse to add to bad patterns
     @@ Commit message
          because of a test that adds non-cone mode patterns without cleaning them
          up. Perform that cleanup as part of the test now.
      
     +    Reviewed-by: Elijah Newren [off-list ref]
          Signed-off-by: Derrick Stolee [off-list ref]
      
       ## builtin/sparse-checkout.c ##
 -:  ----------- > 4:  7bacb3760f3 amend! sparse-checkout: fix segfault on malformed patterns

-- 
gitgitgadget

From: Derrick Stolee <redacted>

Add a test to t1091-sparse-checkout-builtin.sh that would result in an
infinite loop and out-of-memory error before this change. The issue
relies on having non-cone-mode patterns while trying to modify the
patterns in cone-mode.

The fix is simple, allowing us to break from the loop when the input
path does not contain a slash, as the "dir" pattern we added does not.

This is only a fix to the critical out-of-memory error. A better
response to such a strange state will follow in a later change.

Reported-by: Calbabreaker <redacted>
Helped-by: Taylor Blau [off-list ref]
Reviewed-by: Elijah Newren <redacted>
Signed-off-by: Derrick Stolee <redacted>
---
 builtin/sparse-checkout.c          |  2 +-
 t/t1091-sparse-checkout-builtin.sh | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index d0f5c4702be..9ccdcde9832 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c

@@ -483,7 +483,7 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat
 		char *oldpattern = e->pattern;
 		size_t newlen;
 
-		if (slash == e->pattern)
+		if (!slash || slash == e->pattern)
 			break;
 
 		newlen = slash - e->pattern;

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index c72b8ee2e7b..67ce24c9c83 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -103,6 +103,17 @@ test_expect_success 'clone --sparse' '
 	check_files clone a
 '
 
+test_expect_success 'switching to cone mode with non-cone mode patterns' '
+	git init bad-patterns &&
+	(
+		cd bad-patterns &&
+		git sparse-checkout init &&
+		git sparse-checkout add dir &&
+		git config core.sparseCheckoutCone true &&
+		git sparse-checkout add dir
+	)
+'
+
 test_expect_success 'interaction with clone --no-checkout (unborn index)' '
 	git clone --no-checkout "file://$(pwd)/repo" clone_no_checkout &&
 	git -C clone_no_checkout sparse-checkout init --cone &&

-- 
gitgitgadget

From: Derrick Stolee <redacted>

Then core.sparseCheckoutCone is enabled, the sparse-checkout patterns are
used to populate two hashsets that accelerate pattern matching. If the user
modifies the sparse-checkout file outside of the 'sparse-checkout' builtin,
then strange patterns can happen, triggering some error checks.

One of these error checks is possible to hit when some special characters
exist in a line. A warning message is correctly written to stderr, but then
there is additional logic that attempts to remove the line from the hashset
and free the data. This leads to a segfault in the 'git sparse-checkout
list' command because it iterates over the contents of the hashset, which is
no invalid.

The fix here is to stop trying to remove from the hashset. Better to leave
bad data in the sparse-checkout matching logic (with a warning) than to
segfault. If we are in this state, then we are already traversing into
undefined behavior, so this change to keep the entry in the hashset is no
worse than removing it.

Add a test that triggers the segfault without the code change.

Reported-by: John Burnett <redacted>
Reviewed-by: Elijah Newren <redacted>
Signed-off-by: Derrick Stolee <redacted>
---
 dir.c                              |  3 ---
 t/t1091-sparse-checkout-builtin.sh | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/dir.c b/dir.c
index 5aa6fbad0b7..0693c7cb3ee 100644
--- a/dir.c
+++ b/dir.c

@@ -819,9 +819,6 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
 		/* we already included this at the parent level */
 		warning(_("your sparse-checkout file may have issues: pattern '%s' is repeated"),
 			given->pattern);
-		hashmap_remove(&pl->parent_hashmap, &translated->ent, &data);
-		free(data);
-		free(translated);
 	}
 
 	return;

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 272ba1b566b..c72b8ee2e7b 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -708,4 +708,19 @@ test_expect_success 'cone mode clears ignored subdirectories' '
 	test_cmp expect out
 '
 
+test_expect_success 'malformed cone-mode patterns' '
+	git -C repo sparse-checkout init --cone &&
+	mkdir -p repo/foo/bar &&
+	touch repo/foo/bar/x repo/foo/y &&
+	cat >repo/.git/info/sparse-checkout <<-\EOF &&
+	/*
+	!/*/
+	/foo/
+	!/foo/*/
+	/foo/\*/
+	EOF
+	cat repo/.git/info/sparse-checkout &&
+	git -C repo sparse-checkout list
+'
+
 test_done

-- 
gitgitgadget

From: Derrick Stolee <redacted>

When in cone mode sparse-checkout, it is unclear how 'git
sparse-checkout add <dir1> ...' should behave if the existing
sparse-checkout file does not match the cone mode patterns. Change the
behavior to fail with an error message about the existing patterns.

Also, all cone mode patterns start with a '/' character, so add that
restriction. This is necessary for our example test 'cone mode: warn on
bad pattern', but also requires modifying the example sparse-checkout
file we use to test the warnings related to recognizing cone mode
patterns.

This error checking would cause a failure further down the test script
because of a test that adds non-cone mode patterns without cleaning them
up. Perform that cleanup as part of the test now.

Reviewed-by: Elijah Newren <redacted>
Signed-off-by: Derrick Stolee <redacted>
---
 builtin/sparse-checkout.c          | 3 +++
 dir.c                              | 2 +-
 t/t1091-sparse-checkout-builtin.sh | 7 +++++--
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 9ccdcde9832..6580075a631 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c

@@ -598,6 +598,9 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 		die(_("unable to load existing sparse-checkout patterns"));
 	free(sparse_filename);
 
+	if (!existing.use_cone_patterns)
+		die(_("existing sparse-checkout patterns do not use cone mode"));
+
 	hashmap_for_each_entry(&existing.recursive_hashmap, &iter, pe, ent) {
 		if (!hashmap_contains_parent(&pl->recursive_hashmap,
 					pe->pattern, &buffer) ||

diff --git a/dir.c b/dir.c
index 0693c7cb3ee..a5dddafa16d 100644
--- a/dir.c
+++ b/dir.c

@@ -727,7 +727,7 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
 	}
 
 	if (given->patternlen < 2 ||
-	    *given->pattern == '*' ||
+	    *given->pattern != '/' ||
 	    strstr(given->pattern, "**")) {
 		/* Not a cone pattern. */
 		warning(_("unrecognized pattern: '%s'"), given->pattern);

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 67ce24c9c83..2ed247d75a5 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -110,7 +110,8 @@ test_expect_success 'switching to cone mode with non-cone mode patterns' '
 		git sparse-checkout init &&
 		git sparse-checkout add dir &&
 		git config core.sparseCheckoutCone true &&
-		git sparse-checkout add dir
+		test_must_fail git sparse-checkout add dir 2>err &&
+		grep "existing sparse-checkout patterns do not use cone mode" err
 	)
 '
 

@@ -176,12 +177,14 @@ test_expect_success 'set sparse-checkout using --stdin' '
 '
 
 test_expect_success 'add to sparse-checkout' '
-	cat repo/.git/info/sparse-checkout >expect &&
+	cat repo/.git/info/sparse-checkout >old &&
+	test_when_finished cp old repo/.git/info/sparse-checkout &&
 	cat >add <<-\EOF &&
 	pattern1
 	/folder1/
 	pattern2
 	EOF
+	cat old >expect &&
 	cat add >>expect &&
 	git -C repo sparse-checkout add --stdin <add &&
 	git -C repo sparse-checkout list >actual &&

-- 
gitgitgadget

This series fixes some issues with parsing sparse-checkout patterns when
core.sparseCheckoutCone is enabled but the sparse-checkout file itself
contains patterns that don't match the cone mode format.

The first patch fixes a segfault first reported in [1]. The other two
patches are from an earlier submission [2] that never got picked up and I
lost track of. There was another patch involving 'git sparse-checkout init
--cone' that isn't necessary, especially with Elijah doing some work in that
space right now.

[1] https://github.com/git-for-windows/git/issues/3498 [2]
https://lore.kernel.org/git/pull.1043.git.1632160658.gitgitgadget@gmail.com

Thanks, -Stolee


Updates in v2 and v3
====================

 * I intended to fix a typo in a patch, but accidentally sent the amend!
   commit in v2
 * v3 has the typo fix properly squashed in.
 * Added Elijah's review.

Derrick Stolee (3):
  sparse-checkout: fix segfault on malformed patterns
  sparse-checkout: fix OOM error with mixed patterns
  sparse-checkout: refuse to add to bad patterns

 builtin/sparse-checkout.c          |  5 ++++-
 dir.c                              |  5 +----
 t/t1091-sparse-checkout-builtin.sh | 31 +++++++++++++++++++++++++++++-
 3 files changed, 35 insertions(+), 6 deletions(-)


base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1069%2Fderrickstolee%2Fsparse-checkout%2Finput-bug-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1069/derrickstolee/sparse-checkout/input-bug-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1069

Range-diff vs v2:

 1:  a0e3dd335c9 ! 1:  1744a26845f sparse-checkout: fix segfault on malformed patterns
     @@ Commit message
          there is additional logic that attempts to remove the line from the hashset
          and free the data. This leads to a segfault in the 'git sparse-checkout
          list' command because it iterates over the contents of the hashset, which is
     -    no invalid.
     +    now invalid.
      
          The fix here is to stop trying to remove from the hashset. Better to leave
          bad data in the sparse-checkout matching logic (with a warning) than to
 2:  86fbf130c03 = 2:  a2fe867222e sparse-checkout: fix OOM error with mixed patterns
 3:  5d096e380a4 = 3:  a0e5a942ae0 sparse-checkout: refuse to add to bad patterns
 4:  7bacb3760f3 < -:  ----------- amend! sparse-checkout: fix segfault on malformed patterns

-- 
gitgitgadget

From: Derrick Stolee <redacted>

Then core.sparseCheckoutCone is enabled, the sparse-checkout patterns are
used to populate two hashsets that accelerate pattern matching. If the user
modifies the sparse-checkout file outside of the 'sparse-checkout' builtin,
then strange patterns can happen, triggering some error checks.

One of these error checks is possible to hit when some special characters
exist in a line. A warning message is correctly written to stderr, but then
there is additional logic that attempts to remove the line from the hashset
and free the data. This leads to a segfault in the 'git sparse-checkout
list' command because it iterates over the contents of the hashset, which is
now invalid.

The fix here is to stop trying to remove from the hashset. Better to leave
bad data in the sparse-checkout matching logic (with a warning) than to
segfault. If we are in this state, then we are already traversing into
undefined behavior, so this change to keep the entry in the hashset is no
worse than removing it.

Add a test that triggers the segfault without the code change.

Reported-by: John Burnett <redacted>
Reviewed-by: Elijah Newren <redacted>
Signed-off-by: Derrick Stolee <redacted>
---
 dir.c                              |  3 ---
 t/t1091-sparse-checkout-builtin.sh | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/dir.c b/dir.c
index 5aa6fbad0b7..0693c7cb3ee 100644
--- a/dir.c
+++ b/dir.c

@@ -819,9 +819,6 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
 		/* we already included this at the parent level */
 		warning(_("your sparse-checkout file may have issues: pattern '%s' is repeated"),
 			given->pattern);
-		hashmap_remove(&pl->parent_hashmap, &translated->ent, &data);
-		free(data);
-		free(translated);
 	}
 
 	return;

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 272ba1b566b..c72b8ee2e7b 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -708,4 +708,19 @@ test_expect_success 'cone mode clears ignored subdirectories' '
 	test_cmp expect out
 '
 
+test_expect_success 'malformed cone-mode patterns' '
+	git -C repo sparse-checkout init --cone &&
+	mkdir -p repo/foo/bar &&
+	touch repo/foo/bar/x repo/foo/y &&
+	cat >repo/.git/info/sparse-checkout <<-\EOF &&
+	/*
+	!/*/
+	/foo/
+	!/foo/*/
+	/foo/\*/
+	EOF
+	cat repo/.git/info/sparse-checkout &&
+	git -C repo sparse-checkout list
+'
+
 test_done

-- 
gitgitgadget

From: Derrick Stolee <redacted>

Add a test to t1091-sparse-checkout-builtin.sh that would result in an
infinite loop and out-of-memory error before this change. The issue
relies on having non-cone-mode patterns while trying to modify the
patterns in cone-mode.

The fix is simple, allowing us to break from the loop when the input
path does not contain a slash, as the "dir" pattern we added does not.

This is only a fix to the critical out-of-memory error. A better
response to such a strange state will follow in a later change.

Reported-by: Calbabreaker <redacted>
Helped-by: Taylor Blau [off-list ref]
Reviewed-by: Elijah Newren <redacted>
Signed-off-by: Derrick Stolee <redacted>
---
 builtin/sparse-checkout.c          |  2 +-
 t/t1091-sparse-checkout-builtin.sh | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index d0f5c4702be..9ccdcde9832 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c

@@ -483,7 +483,7 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat
 		char *oldpattern = e->pattern;
 		size_t newlen;
 
-		if (slash == e->pattern)
+		if (!slash || slash == e->pattern)
 			break;
 
 		newlen = slash - e->pattern;

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index c72b8ee2e7b..67ce24c9c83 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -103,6 +103,17 @@ test_expect_success 'clone --sparse' '
 	check_files clone a
 '
 
+test_expect_success 'switching to cone mode with non-cone mode patterns' '
+	git init bad-patterns &&
+	(
+		cd bad-patterns &&
+		git sparse-checkout init &&
+		git sparse-checkout add dir &&
+		git config core.sparseCheckoutCone true &&
+		git sparse-checkout add dir
+	)
+'
+
 test_expect_success 'interaction with clone --no-checkout (unborn index)' '
 	git clone --no-checkout "file://$(pwd)/repo" clone_no_checkout &&
 	git -C clone_no_checkout sparse-checkout init --cone &&

-- 
gitgitgadget

From: Derrick Stolee <redacted>

When in cone mode sparse-checkout, it is unclear how 'git
sparse-checkout add <dir1> ...' should behave if the existing
sparse-checkout file does not match the cone mode patterns. Change the
behavior to fail with an error message about the existing patterns.

Also, all cone mode patterns start with a '/' character, so add that
restriction. This is necessary for our example test 'cone mode: warn on
bad pattern', but also requires modifying the example sparse-checkout
file we use to test the warnings related to recognizing cone mode
patterns.

This error checking would cause a failure further down the test script
because of a test that adds non-cone mode patterns without cleaning them
up. Perform that cleanup as part of the test now.

Reviewed-by: Elijah Newren <redacted>
Signed-off-by: Derrick Stolee <redacted>
---
 builtin/sparse-checkout.c          | 3 +++
 dir.c                              | 2 +-
 t/t1091-sparse-checkout-builtin.sh | 7 +++++--
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 9ccdcde9832..6580075a631 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c

@@ -598,6 +598,9 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 		die(_("unable to load existing sparse-checkout patterns"));
 	free(sparse_filename);
 
+	if (!existing.use_cone_patterns)
+		die(_("existing sparse-checkout patterns do not use cone mode"));
+
 	hashmap_for_each_entry(&existing.recursive_hashmap, &iter, pe, ent) {
 		if (!hashmap_contains_parent(&pl->recursive_hashmap,
 					pe->pattern, &buffer) ||

diff --git a/dir.c b/dir.c
index 0693c7cb3ee..a5dddafa16d 100644
--- a/dir.c
+++ b/dir.c

@@ -727,7 +727,7 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
 	}
 
 	if (given->patternlen < 2 ||
-	    *given->pattern == '*' ||
+	    *given->pattern != '/' ||
 	    strstr(given->pattern, "**")) {
 		/* Not a cone pattern. */
 		warning(_("unrecognized pattern: '%s'"), given->pattern);

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 67ce24c9c83..2ed247d75a5 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -110,7 +110,8 @@ test_expect_success 'switching to cone mode with non-cone mode patterns' '
 		git sparse-checkout init &&
 		git sparse-checkout add dir &&
 		git config core.sparseCheckoutCone true &&
-		git sparse-checkout add dir
+		test_must_fail git sparse-checkout add dir 2>err &&
+		grep "existing sparse-checkout patterns do not use cone mode" err
 	)
 '
 

@@ -176,12 +177,14 @@ test_expect_success 'set sparse-checkout using --stdin' '
 '
 
 test_expect_success 'add to sparse-checkout' '
-	cat repo/.git/info/sparse-checkout >expect &&
+	cat repo/.git/info/sparse-checkout >old &&
+	test_when_finished cp old repo/.git/info/sparse-checkout &&
 	cat >add <<-\EOF &&
 	pattern1
 	/folder1/
 	pattern2
 	EOF
+	cat old >expect &&
 	cat add >>expect &&
 	git -C repo sparse-checkout add --stdin <add &&
 	git -C repo sparse-checkout list >actual &&

-- 
gitgitgadget

This series fixes some issues with parsing sparse-checkout patterns when
core.sparseCheckoutCone is enabled but the sparse-checkout file itself
contains patterns that don't match the cone mode format.

The first patch fixes a segfault first reported in [1]. The other two
patches are from an earlier submission [2] that never got picked up and I
lost track of. There was another patch involving 'git sparse-checkout init
--cone' that isn't necessary, especially with Elijah doing some work in that
space right now.

[1] https://github.com/git-for-windows/git/issues/3498 [2]
https://lore.kernel.org/git/pull.1043.git.1632160658.gitgitgadget@gmail.com

Thanks, -Stolee


Update in v4
============

 * For added precaution, this kind of unexpected duplicate pattern will
   disable cone mode matching.
 * Tests are updated to verify this new behavior.


Updates in v2 and v3
====================

 * I intended to fix a typo in a patch, but accidentally sent the amend!
   commit in v2
 * v3 has the typo fix properly squashed in.
 * Added Elijah's review.

Derrick Stolee (3):
  sparse-checkout: fix segfault on malformed patterns
  sparse-checkout: fix OOM error with mixed patterns
  sparse-checkout: refuse to add to bad patterns

 builtin/sparse-checkout.c          |  5 +++-
 dir.c                              |  6 ++---
 t/t1091-sparse-checkout-builtin.sh | 37 +++++++++++++++++++++++++++++-
 3 files changed, 42 insertions(+), 6 deletions(-)


base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1069%2Fderrickstolee%2Fsparse-checkout%2Finput-bug-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1069/derrickstolee/sparse-checkout/input-bug-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1069

Range-diff vs v3:

 1:  1744a26845f ! 1:  5353c541d9f sparse-checkout: fix segfault on malformed patterns
     @@ Commit message
          list' command because it iterates over the contents of the hashset, which is
          now invalid.
      
     -    The fix here is to stop trying to remove from the hashset. Better to leave
     -    bad data in the sparse-checkout matching logic (with a warning) than to
     -    segfault. If we are in this state, then we are already traversing into
     -    undefined behavior, so this change to keep the entry in the hashset is no
     -    worse than removing it.
     +    The fix here is to stop trying to remove from the hashset. In addition,
     +    we disable cone mode sparse-checkout because of the malformed data. This
     +    results in the pattern-matching working with a possibly-slower
     +    algorithm, but using the patterns as they are in the sparse-checkout
     +    file.
     +
     +    This also changes the behavior of commands such as 'git sparse-checkout
     +    list' because the output patterns will be the contents of the
     +    sparse-checkout file instead of the list of directories. This is an
     +    existing behavior for other types of bad patterns.
      
          Add a test that triggers the segfault without the code change.
      
     @@ dir.c: static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_
      -		hashmap_remove(&pl->parent_hashmap, &translated->ent, &data);
      -		free(data);
      -		free(translated);
     ++		goto clear_hashmaps;
       	}
       
       	return;
     @@ t/t1091-sparse-checkout-builtin.sh: test_expect_success 'cone mode clears ignore
      +	!/foo/*/
      +	/foo/\*/
      +	EOF
     -+	cat repo/.git/info/sparse-checkout &&
     -+	git -C repo sparse-checkout list
     ++
     ++	# Listing the patterns will notice the duplicate pattern and
     ++	# emit a warning. It will list the patterns directly instead
     ++	# of using the cone-mode translation to a set of directories.
     ++	git -C repo sparse-checkout list >actual 2>err &&
     ++	test_cmp repo/.git/info/sparse-checkout actual &&
     ++	grep "warning: your sparse-checkout file may have issues: pattern .* is repeated" err &&
     ++	grep "warning: disabling cone pattern matching" err
      +'
      +
       test_done
 2:  a2fe867222e = 2:  3fd625290a3 sparse-checkout: fix OOM error with mixed patterns
 3:  a0e5a942ae0 = 3:  f5f7b8b8e04 sparse-checkout: refuse to add to bad patterns

-- 
gitgitgadget

From: Derrick Stolee <redacted>

Then core.sparseCheckoutCone is enabled, the sparse-checkout patterns are
used to populate two hashsets that accelerate pattern matching. If the user
modifies the sparse-checkout file outside of the 'sparse-checkout' builtin,
then strange patterns can happen, triggering some error checks.

One of these error checks is possible to hit when some special characters
exist in a line. A warning message is correctly written to stderr, but then
there is additional logic that attempts to remove the line from the hashset
and free the data. This leads to a segfault in the 'git sparse-checkout
list' command because it iterates over the contents of the hashset, which is
now invalid.

The fix here is to stop trying to remove from the hashset. In addition,
we disable cone mode sparse-checkout because of the malformed data. This
results in the pattern-matching working with a possibly-slower
algorithm, but using the patterns as they are in the sparse-checkout
file.

This also changes the behavior of commands such as 'git sparse-checkout
list' because the output patterns will be the contents of the
sparse-checkout file instead of the list of directories. This is an
existing behavior for other types of bad patterns.

Add a test that triggers the segfault without the code change.

Reported-by: John Burnett <redacted>
Reviewed-by: Elijah Newren <redacted>
Signed-off-by: Derrick Stolee <redacted>
---
 dir.c                              |  4 +---
 t/t1091-sparse-checkout-builtin.sh | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/dir.c b/dir.c
index 5aa6fbad0b7..7e72958d51d 100644
--- a/dir.c
+++ b/dir.c

@@ -819,9 +819,7 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
 		/* we already included this at the parent level */
 		warning(_("your sparse-checkout file may have issues: pattern '%s' is repeated"),
 			given->pattern);
-		hashmap_remove(&pl->parent_hashmap, &translated->ent, &data);
-		free(data);
-		free(translated);
+		goto clear_hashmaps;
 	}
 
 	return;

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 272ba1b566b..3921ea80138 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -708,4 +708,25 @@ test_expect_success 'cone mode clears ignored subdirectories' '
 	test_cmp expect out
 '
 
+test_expect_success 'malformed cone-mode patterns' '
+	git -C repo sparse-checkout init --cone &&
+	mkdir -p repo/foo/bar &&
+	touch repo/foo/bar/x repo/foo/y &&
+	cat >repo/.git/info/sparse-checkout <<-\EOF &&
+	/*
+	!/*/
+	/foo/
+	!/foo/*/
+	/foo/\*/
+	EOF
+
+	# Listing the patterns will notice the duplicate pattern and
+	# emit a warning. It will list the patterns directly instead
+	# of using the cone-mode translation to a set of directories.
+	git -C repo sparse-checkout list >actual 2>err &&
+	test_cmp repo/.git/info/sparse-checkout actual &&
+	grep "warning: your sparse-checkout file may have issues: pattern .* is repeated" err &&
+	grep "warning: disabling cone pattern matching" err
+'
+
 test_done

-- 
gitgitgadget

From: Derrick Stolee <redacted>

Add a test to t1091-sparse-checkout-builtin.sh that would result in an
infinite loop and out-of-memory error before this change. The issue
relies on having non-cone-mode patterns while trying to modify the
patterns in cone-mode.

The fix is simple, allowing us to break from the loop when the input
path does not contain a slash, as the "dir" pattern we added does not.

This is only a fix to the critical out-of-memory error. A better
response to such a strange state will follow in a later change.

Reported-by: Calbabreaker <redacted>
Helped-by: Taylor Blau [off-list ref]
Reviewed-by: Elijah Newren <redacted>
Signed-off-by: Derrick Stolee <redacted>
---
 builtin/sparse-checkout.c          |  2 +-
 t/t1091-sparse-checkout-builtin.sh | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index d0f5c4702be..9ccdcde9832 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c

@@ -483,7 +483,7 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat
 		char *oldpattern = e->pattern;
 		size_t newlen;
 
-		if (slash == e->pattern)
+		if (!slash || slash == e->pattern)
 			break;
 
 		newlen = slash - e->pattern;

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 3921ea80138..1f877ced0c8 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -103,6 +103,17 @@ test_expect_success 'clone --sparse' '
 	check_files clone a
 '
 
+test_expect_success 'switching to cone mode with non-cone mode patterns' '
+	git init bad-patterns &&
+	(
+		cd bad-patterns &&
+		git sparse-checkout init &&
+		git sparse-checkout add dir &&
+		git config core.sparseCheckoutCone true &&
+		git sparse-checkout add dir
+	)
+'
+
 test_expect_success 'interaction with clone --no-checkout (unborn index)' '
 	git clone --no-checkout "file://$(pwd)/repo" clone_no_checkout &&
 	git -C clone_no_checkout sparse-checkout init --cone &&

-- 
gitgitgadget

From: Derrick Stolee <redacted>

When in cone mode sparse-checkout, it is unclear how 'git
sparse-checkout add <dir1> ...' should behave if the existing
sparse-checkout file does not match the cone mode patterns. Change the
behavior to fail with an error message about the existing patterns.

Also, all cone mode patterns start with a '/' character, so add that
restriction. This is necessary for our example test 'cone mode: warn on
bad pattern', but also requires modifying the example sparse-checkout
file we use to test the warnings related to recognizing cone mode
patterns.

This error checking would cause a failure further down the test script
because of a test that adds non-cone mode patterns without cleaning them
up. Perform that cleanup as part of the test now.

Reviewed-by: Elijah Newren <redacted>
Signed-off-by: Derrick Stolee <redacted>
---
 builtin/sparse-checkout.c          | 3 +++
 dir.c                              | 2 +-
 t/t1091-sparse-checkout-builtin.sh | 7 +++++--
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 9ccdcde9832..6580075a631 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c

@@ -598,6 +598,9 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 		die(_("unable to load existing sparse-checkout patterns"));
 	free(sparse_filename);
 
+	if (!existing.use_cone_patterns)
+		die(_("existing sparse-checkout patterns do not use cone mode"));
+
 	hashmap_for_each_entry(&existing.recursive_hashmap, &iter, pe, ent) {
 		if (!hashmap_contains_parent(&pl->recursive_hashmap,
 					pe->pattern, &buffer) ||

diff --git a/dir.c b/dir.c
index 7e72958d51d..eca75e89213 100644
--- a/dir.c
+++ b/dir.c

@@ -727,7 +727,7 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
 	}
 
 	if (given->patternlen < 2 ||
-	    *given->pattern == '*' ||
+	    *given->pattern != '/' ||
 	    strstr(given->pattern, "**")) {
 		/* Not a cone pattern. */
 		warning(_("unrecognized pattern: '%s'"), given->pattern);

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 1f877ced0c8..34b97fd3ee0 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh

@@ -110,7 +110,8 @@ test_expect_success 'switching to cone mode with non-cone mode patterns' '
 		git sparse-checkout init &&
 		git sparse-checkout add dir &&
 		git config core.sparseCheckoutCone true &&
-		git sparse-checkout add dir
+		test_must_fail git sparse-checkout add dir 2>err &&
+		grep "existing sparse-checkout patterns do not use cone mode" err
 	)
 '
 

@@ -176,12 +177,14 @@ test_expect_success 'set sparse-checkout using --stdin' '
 '
 
 test_expect_success 'add to sparse-checkout' '
-	cat repo/.git/info/sparse-checkout >expect &&
+	cat repo/.git/info/sparse-checkout >old &&
+	test_when_finished cp old repo/.git/info/sparse-checkout &&
 	cat >add <<-\EOF &&
 	pattern1
 	/folder1/
 	pattern2
 	EOF
+	cat old >expect &&
 	cat add >>expect &&
 	git -C repo sparse-checkout add --stdin <add &&
 	git -C repo sparse-checkout list >actual &&

-- 
gitgitgadget