[PATCH 2/2] stash: reject time-based selectors in drop and pop

[Junio C Hamano <gitster@pobox.com>]

get_stash_info_assert() verifies that a revision is a stash
reference ('.is_stash_ref'), but it does not verify whether the
reference is a positional reflog index as opposed to a time-based
selector (such as 'stash@{2.days}').

When subcommands such as 'git stash drop' or 'git stash pop' pass
time-based selectors to reflog_delete(), reflog_delete() treats
non-integer selectors as expiration cutoff timestamps.
Consequently,

 - 'git stash drop stash@{2.days.ago}' deletes all stash entries
   older than two days instead of dropping a single entry, and

 - 'git stash pop stash@{2.days.ago}' applies a single stash entry
   at that timestamp and then deletes all stash entries older than
   two days.

While the former might be remotely useful, the latter is certainly
not.  In get_stash_info_assert(), reject references where
'.stash_idx' is negative (i.e., a time-based reference was used),
ensuring that 'git stash drop' and 'git stash pop' fail early on
invalid or date-based stash references.

Document that 'git reflog expire --expire=<time> refs/stash' should
be used to prune stashes by age, and add unit tests covering
rejection of time-based selectors for 'drop' and 'pop'.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-stash.adoc |  8 ++++++++
 builtin/stash.c              |  3 +++
 t/t3903-stash.sh             | 13 +++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 50bb89f483..6711157421 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -106,6 +106,10 @@ command to control what is shown and how. See linkgit:git-log[1].
 	operation of `git stash push`. The working directory must
 	match the index.
 +
+When _<stash>_ is specified, it must be a positional stash index
+of the form `stash@{<n>}` or `<n>`. Time-based reflog selectors
+(e.g. `stash@{2.days.ago}`) are not accepted.
++
 Applying the state can fail with conflicts; in this case, it is not
 removed from the stash list. You need to resolve the conflicts by hand
 and call `git stash drop` manually afterwards.
@@ -137,6 +141,10 @@ with no conflicts.
 
 `drop [-q | --quiet] [<stash>]`::
 	Remove a single stash entry from the list of stash entries.
+	When _<stash>_ is specified, it must be a positional stash index
+	of the form `stash@{<n>}` or `<n>`. Time-based reflog selectors
+	(e.g. `stash@{2.days.ago}`) are not accepted. To prune stashes older
+	than a given timestamp, use `git reflog expire --expire=<time> refs/stash`.
 
 `create`::
 	Create a stash entry (which is a regular commit object) and
diff --git a/builtin/stash.c b/builtin/stash.c
index 5041a9ba81..6f9561ee3a 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -865,6 +865,9 @@ static int get_stash_info_assert(struct stash_info *info, int argc,
 	if (!info->is_stash_ref)
 		return error(_("'%s' is not a stash reference"), info->revision.buf);
 
+	if (info->stash_idx < 0)
+		return error(_("'%s' is not a valid stash index"), info->revision.buf);
+
 	return 0;
 }
 
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index da27a6599a..01d59c8ef4 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -808,6 +808,19 @@ test_expect_success 'pop: fail early if specified stash is not a stash ref' '
 	git reset --hard HEAD
 '
 
+test_expect_success 'drop and pop reject time-based reflog selectors' '
+	git stash clear &&
+	test_when_finished "git reset --hard HEAD && git stash clear" &&
+	git reset --hard &&
+	echo foo >file &&
+	git stash &&
+	test_must_fail git stash drop stash@{2.days.ago} 2>err &&
+	test_grep "is not a valid stash index" err &&
+	test_must_fail git stash pop stash@{2.days.ago} 2>err &&
+	test_grep "is not a valid stash index" err &&
+	git stash drop
+'
+
 test_expect_success 'ref with non-existent reflog' '
 	git stash clear &&
 	echo bar5 >file &&
-- 
2.55.0-597-ge6126a35d6