diff mbox series

[v2,10/11] serve: reject commands used as capabilities

Message ID YUE1zZ+DXBET15VS@coredump.intra.peff.net (mailing list archive)
State Superseded
Headers show
Series limit memory allocations for v2 servers | expand

Commit Message

Jeff King Sept. 14, 2021, 11:52 p.m. UTC
Our table of v2 "capabilities" contains everything we might tell the
client we support. But there are differences in how we expect the client
to respond. Some of the entries are true capabilities (i.e., we expect
the client to say "yes, I support this"), and some are ones we expect
them to send as commands (with "command=ls-refs" or similar).

When we receive a capability used as a command, we complain about that.
But when we receive a command used as a capability (e.g., just "ls-refs"
in a pkt-line by itself), we silently ignore it.

This isn't really hurting anything (clients shouldn't send it, and we'll
ignore it), but we can tighten up the protocol to match what we expect
to happen.

There are two new tests here. The first one checks a capability used as
a command, which already passes. The second tests a command as a
capability, which this patch fixes.

Signed-off-by: Jeff King <peff@peff.net>
---
 serve.c              |  2 +-
 t/t5701-git-serve.sh | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/serve.c b/serve.c
index 63ee1be7ff..0636b79f92 100644
--- a/serve.c
+++ b/serve.c
@@ -201,7 +201,7 @@  static int receive_client_capability(const char *key)
 	const char *value;
 	const struct protocol_capability *c = get_capability(key, &value);
 
-	if (!c || !c->advertise(the_repository, NULL))
+	if (!c || c->command || !c->advertise(the_repository, NULL))
 		return 0;
 
 	if (c->receive)
diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
index ab15078bc0..b027ba9b06 100755
--- a/t/t5701-git-serve.sh
+++ b/t/t5701-git-serve.sh
@@ -72,6 +72,25 @@  test_expect_success 'request invalid command' '
 	test_i18ngrep "invalid command" err
 '
 
+test_expect_success 'request capability as command' '
+	test-tool pkt-line pack >in <<-\EOF &&
+	command=agent
+	0000
+	EOF
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
+	grep invalid.command.*agent err
+'
+
+test_expect_success 'request command as capability' '
+	test-tool pkt-line pack >in <<-\EOF &&
+	command=ls-refs
+	fetch
+	0000
+	EOF
+	test_must_fail test-tool serve-v2 --stateless-rpc 2>err <in &&
+	grep unknown.capability err
+'
+
 test_expect_success 'requested command is command=value' '
 	test-tool pkt-line pack >in <<-\EOF &&
 	command=ls-refs=whatever