Re: [PATCH v2 09/11] serve: reject bogus v2 "command=ls-refs=foo"

[Jeff King <peff@peff.net>]

On Wed, Sep 15, 2021 at 01:09:19AM -0400, Eric Sunshine wrote:

> > diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
> > @@ -72,6 +72,16 @@ test_expect_success 'request invalid command' '
> > +test_expect_success 'requested command is command=value' '
> > +       test-tool pkt-line pack >in <<-\EOF &&
> > +       command=ls-refs=whatever
> > +       object-format=$(test_oid algo)
> > +       0000
> > +       EOF
> 
> This here-doc uses <<-\EOF yet (presumably) expects $(test_oid algo)
> to be expanded. I'm guessing that you meant <<-EOF but never
> noticed...

Ah, thanks. It's muscle memory to inhibit expansion in my here-docs. :)

> > +       test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
> 
> ... because of this test_must_fail().
> 
> > +       grep invalid.command.*ls-refs=whatever err
> > +'

Actually, it's a little more complicated. The "grep" here would notice
if we failed for the wrong reason. But we never actually look at the
object-format line at all, because we barf as soon as we see the bogus
command line.

So the object-format line is superfluous in this test. I added it mostly
for consistency with the other tests (including the existing
unknown-command test, which is in the same boat). But it also makes the
test slightly more robust, in that the command is more likely to succeed
(and thus fail the test) if the code accidentally did not notice the
wrong command line.

But obviously it's worth fixing. Thanks for noticing.

-Peff