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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 15, 2021, 17:33 UTC
Message-ID: <xmqq8rzxyc9f.fsf@gitster.g>
In-Reply-To: <YUE1ym1dALRQLztq@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 7 quoted lines

> 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.

Maybe I am slow but I had to read the above a few times and finally look at the implementation of parse_command() to realize that what the last sentence describes is:

    When parse_command() is fed "command=ls-refs=foo", it strips
    "command=", feeds "ls-refs=foo" to get_capability(), and because
    we do not ensure value is NULL, we silently ignore "=foo" that
    is bogus.

And it makes sense. It would probably have helped if I peeked the updated test ;-)

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

Good.