Re: [PATCH 1/1] upload-pack.c: fix filter spec quoting bug

Jeff King [off-list ref] writes:

On Fri, Jan 22, 2021 at 03:21:37PM +0100, Jacob Vosmaer wrote:

quoted

This fixes a bug that occurs when you combine partial clone and
uploadpack.packobjectshook. You can reproduce it as follows:

...
I'm somewhat embarrassed to say that despite being the one who added the
pack-objects hook 4 years ago, we still have not switched over to it at
GitHub from our custom patch (the reason is just mundane; there's some
other adjustments that would have to happen and nobody has ever quite
gotten around to it). Presumably you are looking to use it at GitLab.
Just beware that you are probably treading new-ish ground, so there may
be other bugs like this lurking.

quoted

diff --git a/upload-pack.c b/upload-pack.c
index 3b66bf92ba..eae1fdbc55 100644
--- a/upload-pack.c
+++ b/upload-pack.c

@@ -305,14 +305,7 @@ static void create_pack_file(struct upload_pack_data *pack_data,
 	if (pack_data->filter_options.choice) {
 		const char *spec =
 			expand_list_objects_filter_spec(&pack_data->filter_options);
-		if (pack_objects.use_shell) {
-			struct strbuf buf = STRBUF_INIT;
-			sq_quote_buf(&buf, spec);
-			strvec_pushf(&pack_objects.args, "--filter=%s", buf.buf);
-			strbuf_release(&buf);
-		} else {
-			strvec_pushf(&pack_objects.args, "--filter=%s", spec);
-		}
+		strvec_pushf(&pack_objects.args, "--filter=%s", spec);

Yep, this looks like the right fix. I think with an addition to the test
suite, this will be good to go.

Yeah, that looks simpler and better.  It does deserve new tests.

Thanks, both.