[BUG] 'git bisect start' fails to detect bad revisions without '--'

[BUG] 'git bisect start' fails to detect bad revisions without '--'

[Philippe Blain]

Hello all,

I've found a small bug in the argument parsing of 'git bisect 'start',
i.e builtin/bisect--helper.c::bisect_start.

If one mistypes bad (or good) revisions but does not write the double-dash,
the code does not check that the revisions exist and it goes on and checks out HEAD.
 From what I understand the mistyped revisions are (in some cases) interpreted as paths
(they are written to '.git/BISECT_NAMES), even if they do not match any paths...

For example, these invocations all check out HEAD and do not give any error in git.git:

	git bisect start BAD GOOD
	git bisect start v2.31.0 GOOD
	git bisect start v2.31.0 GOOD ANOTHER

But these correctly error:

	git bisect start BAD GOOD --
	git bisect start v2.31.0 GOOD --
	git bisect start v2.31.0 GOOD ANOTHER --
	git bisect start v2.31.0 v2.30.0 GOOD BAD

This one does not error and correctly checks out the mid point between 2.31 and 2.30,
but does not check that BAD exists (in this case it's not written to BISECT_NAMES):

	git bisect start v2.31.0 v2.30.0 BAD


This is on the latest master, 670b81a890 (The second batch, 2021-06-14). I tried older versions
before the start of the conversion of 'git bisect' from shell to C and they seem to behave the same.

I'm CC-ing Christian and Dscho since I noticed 73c6de06af (bisect: don't use invalid oid
as rev when starting, 2020-09-25) touched the argument parsing in 'bisect start' and is tangentially
related.

Cheers,

Philippe.

Re: [BUG] 'git bisect start' fails to detect bad revisions without '--'

[Christian Couder]

Hi Philippe,

On Wed, Jun 30, 2021 at 8:47 PM Philippe Blain
<levraiphilippeblain@gmail.com> wrote:
>
> Hello all,
>
> I've found a small bug in the argument parsing of 'git bisect 'start',
> i.e builtin/bisect--helper.c::bisect_start.

Sorry for the late reply to this, and thanks for the report!

> If one mistypes bad (or good) revisions but does not write the double-dash,
> the code does not check that the revisions exist and it goes on and checks out HEAD.
>  From what I understand the mistyped revisions are (in some cases) interpreted as paths
> (they are written to '.git/BISECT_NAMES), even if they do not match any paths...
>
> For example, these invocations all check out HEAD and do not give any error in git.git:
>
>         git bisect start BAD GOOD
>         git bisect start v2.31.0 GOOD
>         git bisect start v2.31.0 GOOD ANOTHER
>
> But these correctly error:
>
>         git bisect start BAD GOOD --
>         git bisect start v2.31.0 GOOD --
>         git bisect start v2.31.0 GOOD ANOTHER --
>         git bisect start v2.31.0 v2.30.0 GOOD BAD
>
> This one does not error and correctly checks out the mid point between 2.31 and 2.30,
> but does not check that BAD exists (in this case it's not written to BISECT_NAMES):
>
>         git bisect start v2.31.0 v2.30.0 BAD
>
> This is on the latest master, 670b81a890 (The second batch, 2021-06-14). I tried older versions
> before the start of the conversion of 'git bisect' from shell to C and they seem to behave the same.

Yeah, I agree that it looks like the behavior could be improved. It
could be tricky though, as, even if something does not match any paths
in the current commit nor in the BAD and GOOD commits, it could
perhaps match a path that is in one of the in-between commits.

Best,
Christian.