Subject: Re: [PATCH v6 2/7] archive --add-virtual-file: allow paths containing colons

From: Junio C Hamano <[email-masked]>
Date: Wed, 25 May 2022 15:34:05 -0700

Junio C Hamano <[email-masked]> writes:

>> 	# The PATHNAME variable holds a filename encoded like a
>> 	# string constant in C language (e.g. "\060" is digit "0")
>> 	if test_have_prereq FUNNYNAMES
>> 	then
>> 		PATHNAME=quoted:colon:\\060zero
>> 	...
> Actually, I _think_ that pushes us beyond the "reasonably defensive
> for the current need".  We'd need to prepare how the pathname is
> expected to be unquoted for the later test
>
> 	test_path_is_file "$PATHNAME"
>
> to work.

IOW, I would need to add a new test-tool (attached) and then start
this test like so:

	if ...
	then
		PATHNAME=quoted:colon:\\060zero
	else
		PATHNAME=quoted\\060zero
	fi
	UQPATHNAME=$(test-tool unquote-c-style \""$PATHNAME"\")

and change the last test to

	test_path_is_file "$UQPATHNAME"

if we really wanted to test that the the PATHNAME is treated as a
c-style quoted string.

I am on the fence.  We do not have an immediate need, in the sense
that nobody needs to encode "0" as "\060" and trigger the unquote
codepath in real life.  But it does feel prudent to make sure we can
grok C-quoted pathname as we claim in the documentation.

And the resulting change to the test does not look _too_ bad (and
the new test-tool certainly does not hurt, either).

So...

 Makefile               |  1 +
 t/helper/test-quoted.c | 34 ++++++++++++++++++++++++++++++++++
 t/helper/test-tool.c   |  2 ++
 t/helper/test-tool.h   |  2 ++
 4 files changed, 39 insertions(+)

diff --git c/Makefile w/Makefile
index 298becd5a5..1d544ad46a 100644
--- c/Makefile
+++ w/Makefile
@@ -749,6 +749,7 @@ TEST_BUILTINS_OBJS += test-pkt-line.o
 TEST_BUILTINS_OBJS += test-prio-queue.o
 TEST_BUILTINS_OBJS += test-proc-receive.o
 TEST_BUILTINS_OBJS += test-progress.o
+TEST_BUILTINS_OBJS += test-quoted.o
 TEST_BUILTINS_OBJS += test-reach.o
 TEST_BUILTINS_OBJS += test-read-cache.o
 TEST_BUILTINS_OBJS += test-read-graph.o
diff --git c/t/helper/test-quoted.c w/t/helper/test-quoted.c
new file mode 100644
index 0000000000..15baa55e43
--- /dev/null
+++ w/t/helper/test-quoted.c
@@ -0,0 +1,34 @@
+#include "test-tool.h"
+#include "cache.h"
+#include "quote.h"
+
+int cmd__unquote_c_style(int argc, const char **argv)
+{
+	struct strbuf buf = STRBUF_INIT;
+
+	while (*++argv) {
+		const char *p = *argv;
+
+		if (unquote_c_style(&buf, p, &p) < 0)
+			error("cannot unquote '%s'", *argv);
+		else
+			printf("%s\n", buf.buf);
+		strbuf_reset(&buf);
+	}
+	return 0;
+}
+
+int cmd__quote_c_style(int argc, const char **argv)
+{
+	struct strbuf buf = STRBUF_INIT;
+
+	while (*++argv) {
+		const char *p = *argv;
+
+		quote_c_style(p, &buf, NULL, 0);
+		printf("%s\n", buf.buf);
+		strbuf_reset(&buf);
+	}
+	return 0;
+}
+
diff --git c/t/helper/test-tool.c w/t/helper/test-tool.c
index d2eacd302d..5633c98569 100644
--- c/t/helper/test-tool.c
+++ w/t/helper/test-tool.c
@@ -58,6 +58,7 @@ static struct test_cmd cmds[] = {
 	{ "prio-queue", cmd__prio_queue },
 	{ "proc-receive", cmd__proc_receive },
 	{ "progress", cmd__progress },
+	{ "quote-c-style", cmd__quote_c_style },
 	{ "reach", cmd__reach },
 	{ "read-cache", cmd__read_cache },
 	{ "read-graph", cmd__read_graph },
@@ -81,6 +82,7 @@ static struct test_cmd cmds[] = {
 	{ "submodule-nested-repo-config", cmd__submodule_nested_repo_config },
 	{ "subprocess", cmd__subprocess },
 	{ "trace2", cmd__trace2 },
+	{ "unquote-c-style", cmd__unquote_c_style },
 	{ "userdiff", cmd__userdiff },
 	{ "urlmatch-normalization", cmd__urlmatch_normalization },
 	{ "xml-encode", cmd__xml_encode },
diff --git c/t/helper/test-tool.h w/t/helper/test-tool.h
index 960cc27ef7..f5e8929009 100644
--- c/t/helper/test-tool.h
+++ w/t/helper/test-tool.h
@@ -48,6 +48,7 @@ int cmd__pkt_line(int argc, const char **argv);
 int cmd__prio_queue(int argc, const char **argv);
 int cmd__proc_receive(int argc, const char **argv);
 int cmd__progress(int argc, const char **argv);
+int cmd__quote_c_style(int argc, const char **argv);
 int cmd__reach(int argc, const char **argv);
 int cmd__read_cache(int argc, const char **argv);
 int cmd__read_graph(int argc, const char **argv);
@@ -71,6 +72,7 @@ int cmd__submodule_config(int argc, const char **argv);
 int cmd__submodule_nested_repo_config(int argc, const char **argv);
 int cmd__subprocess(int argc, const char **argv);
 int cmd__trace2(int argc, const char **argv);
+int cmd__unquote_c_style(int argc, const char **argv);
 int cmd__userdiff(int argc, const char **argv);
 int cmd__urlmatch_normalization(int argc, const char **argv);
 int cmd__xml_encode(int argc, const char **argv);