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

On Wed, Sep 15, 2021 at 02:27:35AM +0200, Ævar Arnfjörð Bjarmason wrote:

On Tue, Sep 14 2021, Jeff King wrote:

quoted

When we see a line from the client like "command=ls-refs", we parse
everything after the equals sign as a capability, which we check against
our capabilities table. If we don't recognize the command (e.g.,
"command=foo"), we'll reject it. But we use the same parser that checks
for regular capabilities like "object-format=sha256". And so we'll
accept "ls-refs=foo", even though everything after the equals is bogus,
and simply ignored.

This isn't really hurting anything, but the request does violate the
spec. Let's tighten it up to prevent any surprising behavior.

Doesn't need a re-roll, but just for my own sanity:

By violating the spec you mean it doesn't coform to the "key" in the
"Capability Advertisement" section of protocol-v2.txt, one might skim
this and think values with "=" in them are OK, but a command=WHATEVER
has a WHATEVER as a "key", not a "value", that's for other parts of the
dialog.

But you could also have meant that it violates the spec because there's
no such command as "ls-refs=whatever", just like there isn't "foobar",
but I don't think that's what you mean...

Both, I think. :) The line "command=foo=bar" is obvious nonsense by the
spec, no matter which way you interpret it:

  - "foo" is a command name, but adding a value does not make sense when
    it is used in this context, so it's invalid

  - "foo=bar" is not a command name

Our parser does pick out "foo", so in terms of the code it is more like
the former. But since it is invalid either way, I'm not sure the
distinction matters.

-Peff