From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
"Taylor Blau" <me@ttaylorr.com>,
"Martin Ågren" <martin.agren@gmail.com>
Subject: Re: [PATCH v2 09/11] serve: reject bogus v2 "command=ls-refs=foo"
Date: Wed, 15 Sep 2021 02:27:35 +0200 [thread overview]
Message-ID: <87wnnin0ix.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <YUE1ym1dALRQLztq@coredump.intra.peff.net>
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@peff.net>
> ---
> 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
next prev parent reply other threads:[~2021-09-15 0:30 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-14 15:29 [PATCH 0/9] reducing memory allocations for v2 servers Jeff King
2021-09-14 15:30 ` [PATCH 1/9] serve: rename is_command() to parse_command() Jeff King
2021-09-14 15:30 ` [PATCH 2/9] serve: return capability "value" from get_capability() Jeff King
2021-09-14 15:31 ` [PATCH 3/9] serve: add "receive" method for v2 capabilities table Jeff King
2021-09-14 15:31 ` [PATCH 4/9] serve: provide "receive" function for object-format capability Jeff King
2021-09-14 18:59 ` Martin Ågren
2021-09-14 15:33 ` [PATCH 5/9] serve: provide "receive" function for session-id capability Jeff King
2021-09-14 16:55 ` Taylor Blau
2021-09-14 17:06 ` Jeff King
2021-09-14 17:12 ` Taylor Blau
2021-09-14 19:02 ` Martin Ågren
2021-09-14 19:14 ` Jeff King
2021-09-14 15:33 ` [PATCH 6/9] serve: drop "keys" strvec Jeff King
2021-09-14 16:59 ` Taylor Blau
2021-09-14 17:16 ` Jeff King
2021-09-14 15:37 ` [PATCH 7/9] ls-refs: ignore very long ref-prefix counts Jeff King
2021-09-14 17:18 ` Taylor Blau
2021-09-14 17:27 ` Jeff King
2021-09-14 17:23 ` Jeff King
2021-09-14 19:06 ` Martin Ågren
2021-09-14 19:22 ` Jeff King
2021-09-14 22:09 ` Jeff King
2021-09-14 22:11 ` Taylor Blau
2021-09-14 22:15 ` Jeff King
2021-09-14 15:37 ` [PATCH 8/9] serve: reject bogus v2 "command=ls-refs=foo" Jeff King
2021-09-14 17:21 ` Taylor Blau
2021-09-14 15:37 ` [PATCH 9/9] serve: reject commands used as capabilities Jeff King
2021-09-14 17:30 ` [PATCH 0/9] reducing memory allocations for v2 servers Taylor Blau
2021-09-14 18:00 ` Junio C Hamano
2021-09-14 18:38 ` Jeff King
2021-09-14 23:51 ` [PATCH v2 0/11] limit " Jeff King
2021-09-14 23:51 ` [PATCH v2 01/11] serve: rename is_command() to parse_command() Jeff King
2021-09-14 23:51 ` [PATCH v2 02/11] serve: return capability "value" from get_capability() Jeff King
2021-09-14 23:51 ` [PATCH v2 03/11] serve: add "receive" method for v2 capabilities table Jeff King
2021-09-15 0:31 ` Ævar Arnfjörð Bjarmason
2021-09-15 16:35 ` Jeff King
2021-09-15 16:41 ` Junio C Hamano
2021-09-15 16:57 ` Jeff King
2021-09-14 23:51 ` [PATCH v2 04/11] serve: provide "receive" function for object-format capability Jeff King
2021-09-15 16:54 ` Junio C Hamano
2021-09-14 23:51 ` [PATCH v2 05/11] serve: provide "receive" function for session-id capability Jeff King
2021-09-15 16:56 ` Junio C Hamano
2021-09-14 23:51 ` [PATCH v2 06/11] serve: drop "keys" strvec Jeff King
2021-09-15 17:01 ` Junio C Hamano
2021-09-14 23:51 ` [PATCH v2 07/11] ls-refs: ignore very long ref-prefix counts Jeff King
2021-09-15 4:16 ` Taylor Blau
2021-09-15 16:39 ` Jeff King
2021-09-15 5:00 ` Eric Sunshine
2021-09-15 16:40 ` Jeff King
2021-09-14 23:52 ` [PATCH v2 08/11] docs/protocol-v2: clarify some ls-refs ref-prefix details Jeff King
2021-09-14 23:52 ` [PATCH v2 09/11] serve: reject bogus v2 "command=ls-refs=foo" Jeff King
2021-09-15 0:27 ` Ævar Arnfjörð Bjarmason [this message]
2021-09-15 16:28 ` Jeff King
2021-09-15 5:09 ` Eric Sunshine
2021-09-15 16:32 ` Jeff King
2021-09-15 17:33 ` Junio C Hamano
2021-09-15 17:39 ` Jeff King
2021-09-14 23:52 ` [PATCH v2 10/11] serve: reject commands used as capabilities Jeff King
2021-09-14 23:54 ` [PATCH v2 11/11] ls-refs: reject unknown arguments Jeff King
2021-09-15 0:09 ` Ævar Arnfjörð Bjarmason
2021-09-15 16:25 ` Jeff King
2021-09-15 4:17 ` [PATCH v2 0/11] limit memory allocations for v2 servers Taylor Blau
2021-09-15 18:33 ` Jeff King
2021-09-15 18:34 ` [PATCH v3 " Jeff King
2021-09-15 18:35 ` [PATCH v3 01/11] serve: rename is_command() to parse_command() Jeff King
2021-09-15 18:35 ` [PATCH v3 02/11] serve: return capability "value" from get_capability() Jeff King
2021-09-15 18:35 ` [PATCH v3 03/11] serve: add "receive" method for v2 capabilities table Jeff King
2021-09-15 18:35 ` [PATCH v3 04/11] serve: provide "receive" function for object-format capability Jeff King
2021-09-15 18:35 ` [PATCH v3 05/11] serve: provide "receive" function for session-id capability Jeff King
2021-09-15 18:35 ` [PATCH v3 06/11] serve: drop "keys" strvec Jeff King
2021-09-15 18:35 ` [PATCH v3 07/11] ls-refs: ignore very long ref-prefix counts Jeff King
2021-09-15 18:35 ` [PATCH v3 08/11] docs/protocol-v2: clarify some ls-refs ref-prefix details Jeff King
2021-09-15 18:36 ` [PATCH v3 09/11] serve: reject bogus v2 "command=ls-refs=foo" Jeff King
2021-09-15 18:36 ` [PATCH v3 10/11] serve: reject commands used as capabilities Jeff King
2021-09-15 18:36 ` [PATCH v3 11/11] ls-refs: reject unknown arguments Jeff King
2021-09-15 0:25 ` [PATCH 0/9] reducing memory allocations for v2 servers Ævar Arnfjörð Bjarmason
2021-09-15 16:41 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wnnin0ix.fsf@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=martin.agren@gmail.com \
--cc=me@ttaylorr.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).