Re: [PATCH 6/9] serve: drop "keys" strvec
From: Taylor Blau <hidden>
Date: 2021-09-14 16:59:24
On Tue, Sep 14, 2021 at 11:33:18AM -0400, Jeff King wrote:
We collect the set of capabilities the client sends us in a strvec. While this is usually small, there's no limit to the number of capabilities the client can send us (e.g., they could just send us "agent" pkt-lines over and over, and we'd keep adding them to the list). Since all code has been converted away from using this list, let's get rid of it. This avoids a potential attack where clients waste our memory.
...because now we only bother to tell capabilities about information the client sent as it happened, instead of accumulating an unbounded set of strings together into a single strvec.
Note that we do have to replace it with a flag, because some of the flush-packet logic checks whether we've seen any valid commands or keys.
Makes sense.
quoted hunk ↗ jump to hunk
Signed-off-by: Jeff King <redacted> --- serve.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)diff --git a/serve.c b/serve.c index 6bbf54cbbe..baa0a17502 100644 --- a/serve.c +++ b/serve.c@@ -239,7 +239,7 @@ static int process_request(void) { enum request_state state = PROCESS_REQUEST_KEYS; struct packet_reader reader; - struct strvec keys = STRVEC_INIT; + int seen_capability_or_command = 0; struct protocol_capability *command = NULL; packet_reader_init(&reader, 0, NULL, 0,@@ -263,10 +263,11 @@ static int process_request(void) /* collect request; a sequence of keys and values */ if (parse_command(reader.line, &command) || receive_client_capability(reader.line)) - strvec_push(&keys, reader.line); + seen_capability_or_command = 1; else die("unknown capability '%s'", reader.line); +
Nit; unnecessary whitespace change (but obviously not worth a re-roll on its own).
quoted hunk ↗ jump to hunk
/* Consume the peeked line */ packet_reader_read(&reader); break;@@ -275,7 +276,7 @@ static int process_request(void) * If no command and no keys were given then the client * wanted to terminate the connection. */ - if (!keys.nr) + if (!seen_capability_or_command) return 1; /*@@ -309,7 +310,6 @@ static int process_request(void) command->command(the_repository, &reader); - strvec_clear(&keys); return 0; } --2.33.0.887.g5b1f44e68d
The rest of this change looks obviously good to me. Thanks, Taylor