Re: [PATCH v2 06/11] serve: drop "keys" strvec

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> 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.
>
> 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.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  serve.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/serve.c b/serve.c
> index 6bbf54cbbe..5ea6c915cb 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,7 +263,7 @@ 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;

OK, we no longer "collect" request in the keys strvec, but I guess
what receive_client_capability() does still counts as "collecting",
so the "tentatively stale" comment is not wrong after all at the end
(we have tentatively been collecting in two different places and one
of them is dropped here).

> @@ -275,7 +275,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 +309,6 @@ static int process_request(void)
>  
>  	command->command(the_repository, &reader);
>  
> -	strvec_clear(&keys);
>  	return 0;
>  }

Nice.