On Tue, Sep 14 2021, Jeff King wrote:
> 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...
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> serve.c | 2 +-
> t/t5701-git-serve.sh | 10 ++++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/serve.c b/serve.c
> index 5ea6c915cb..63ee1be7ff 100644
> --- a/serve.c
> +++ b/serve.c
> @@ -220,7 +220,7 @@ static int parse_command(const char *key, struct protocol_capability **command)
> if (*command)
> die("command '%s' requested after already requesting command '%s'",
> out, (*command)->name);
> - if (!cmd || !cmd->advertise(the_repository, NULL) || !cmd->command)
> + if (!cmd || !cmd->advertise(the_repository, NULL) || !cmd->command || value)
> die("invalid command '%s'", out);
>
> *command = cmd;
> diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
> index 3bc96ebcde..ab15078bc0 100755
> --- a/t/t5701-git-serve.sh
> +++ b/t/t5701-git-serve.sh
> @@ -72,6 +72,16 @@ test_expect_success 'request invalid command' '
> test_i18ngrep "invalid command" err
> '
>
> +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
> + test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
> + grep invalid.command.*ls-refs=whatever err
> +'
> +
> test_expect_success 'wrong object-format' '
> test-tool pkt-line pack >in <<-EOF &&
> command=fetch
