diff mbox series

[v3,1/1] cat-file: quote-format name in error when using -z

Message ID 20230116190749.4141516-2-toon@iotcl.com (mailing list archive)
State New, archived
Headers show
Series cat-file: quote-format name in error when using -z | expand

Commit Message

Toon Claes Jan. 16, 2023, 7:07 p.m. UTC 
Since it's supported to have NUL-delimited input, introduced in
db9d67f2e9 (builtin/cat-file.c: support NUL-delimited input with `-z`,
2022-07-22), it's possible to pass paths that contain newlines. This
works great when the object is found, but when it's not, the input path
is returned in the error message. Because this can contain newlines, the
error message might get spread over multiple lines, making it harder to
machine-parse this error message.

With this change, the input is quote-formatted in the error message, if
needed. This ensures the error message is always on a single line and
makes parsing the error more straightforward.

Signed-off-by: Toon Claes <toon@iotcl.com>
---
 builtin/cat-file.c  | 19 +++++++++++++++++++
 t/t1006-cat-file.sh |  8 ++++++++
 2 files changed, 27 insertions(+)

--
2.39.0.rc0.57.g2e71cbbddd.dirty

Comments

Phillip Wood Jan. 17, 2023, 3:24 p.m. UTC | #1 
Hi Toon

On 16/01/2023 19:07, Toon Claes wrote:
> Since it's supported to have NUL-delimited input, introduced in
> db9d67f2e9 (builtin/cat-file.c: support NUL-delimited input with `-z`,
> 2022-07-22), it's possible to pass paths that contain newlines. This
> works great when the object is found, but when it's not, the input path
> is returned in the error message. Because this can contain newlines, the
> error message might get spread over multiple lines, making it harder to
> machine-parse this error message.
> 
> With this change, the input is quote-formatted in the error message, if
> needed. This ensures the error message is always on a single line and
> makes parsing the error more straightforward.

This looks good but it would be nice to have test coverage for 
batch_one_object() as well as batch_object_write()

Best Wishes

Phillip

> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
>   builtin/cat-file.c  | 19 +++++++++++++++++++
>   t/t1006-cat-file.sh |  8 ++++++++
>   2 files changed, 27 insertions(+)
> 
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index cc17635e76..b678f69773 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -14,6 +14,7 @@
>   #include "tree-walk.h"
>   #include "oid-array.h"
>   #include "packfile.h"
> +#include "quote.h"
>   #include "object-store.h"
>   #include "promisor-remote.h"
>   #include "mailmap.h"
> @@ -455,8 +456,17 @@ static void batch_object_write(const char *obj_name,
>   						       &data->oid, &data->info,
>   						       OBJECT_INFO_LOOKUP_REPLACE);
>   		if (ret < 0) {
> +			struct strbuf quoted = STRBUF_INIT;
> +
> +			if (opt->nul_terminated &&
> +			    obj_name) {
> +				quote_c_style(obj_name, &quoted, NULL, 0);
> +				obj_name = quoted.buf;
> +			}
> +
>   			printf("%s missing\n",
>   			       obj_name ? obj_name : oid_to_hex(&data->oid));
> +			strbuf_release(&quoted);
>   			fflush(stdout);
>   			return;
>   		}
> @@ -503,6 +513,13 @@ static void batch_one_object(const char *obj_name,
>   	result = get_oid_with_context(the_repository, obj_name,
>   				      flags, &data->oid, &ctx);
>   	if (result != FOUND) {
> +		struct strbuf quoted = STRBUF_INIT;
> +
> +		if (opt->nul_terminated) {
> +			quote_c_style(obj_name, &quoted, NULL, 0);
> +			obj_name = quoted.buf;
> +		}
> +
>   		switch (result) {
>   		case MISSING_OBJECT:
>   			printf("%s missing\n", obj_name);
> @@ -527,6 +544,8 @@ static void batch_one_object(const char *obj_name,
>   			       result);
>   			break;
>   		}
> +
> +		strbuf_release(&quoted);
>   		fflush(stdout);
>   		return;
>   	}
> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
> index 23b8942edb..e8ce20e739 100755
> --- a/t/t1006-cat-file.sh
> +++ b/t/t1006-cat-file.sh
> @@ -447,6 +447,14 @@ test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '
>   	test_cmp expect actual
>   '
> 
> +test_expect_success '--batch-check, -z with newline in non-existent named object' '
> +	printf "HEAD:newline${LF}missing" >in &&
> +	git cat-file --batch-check -z <in >actual &&
> +
> +	printf "\"HEAD:newline\\\\nmissing\" missing\n" >expect &&
> +	test_cmp expect actual
> +'
> +
>   batch_command_multiple_info="info $hello_sha1
>   info $tree_sha1
>   info $commit_sha1
> --
> 2.39.0.rc0.57.g2e71cbbddd.dirty
diff mbox series

Patch

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index cc17635e76..b678f69773 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -14,6 +14,7 @@ 
 #include "tree-walk.h"
 #include "oid-array.h"
 #include "packfile.h"
+#include "quote.h"
 #include "object-store.h"
 #include "promisor-remote.h"
 #include "mailmap.h"
@@ -455,8 +456,17 @@  static void batch_object_write(const char *obj_name,
 						       &data->oid, &data->info,
 						       OBJECT_INFO_LOOKUP_REPLACE);
 		if (ret < 0) {
+			struct strbuf quoted = STRBUF_INIT;
+
+			if (opt->nul_terminated &&
+			    obj_name) {
+				quote_c_style(obj_name, &quoted, NULL, 0);
+				obj_name = quoted.buf;
+			}
+
 			printf("%s missing\n",
 			       obj_name ? obj_name : oid_to_hex(&data->oid));
+			strbuf_release(&quoted);
 			fflush(stdout);
 			return;
 		}
@@ -503,6 +513,13 @@  static void batch_one_object(const char *obj_name,
 	result = get_oid_with_context(the_repository, obj_name,
 				      flags, &data->oid, &ctx);
 	if (result != FOUND) {
+		struct strbuf quoted = STRBUF_INIT;
+
+		if (opt->nul_terminated) {
+			quote_c_style(obj_name, &quoted, NULL, 0);
+			obj_name = quoted.buf;
+		}
+
 		switch (result) {
 		case MISSING_OBJECT:
 			printf("%s missing\n", obj_name);
@@ -527,6 +544,8 @@  static void batch_one_object(const char *obj_name,
 			       result);
 			break;
 		}
+
+		strbuf_release(&quoted);
 		fflush(stdout);
 		return;
 	}
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 23b8942edb..e8ce20e739 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -447,6 +447,14 @@  test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '
 	test_cmp expect actual
 '

+test_expect_success '--batch-check, -z with newline in non-existent named object' '
+	printf "HEAD:newline${LF}missing" >in &&
+	git cat-file --batch-check -z <in >actual &&
+
+	printf "\"HEAD:newline\\\\nmissing\" missing\n" >expect &&
+	test_cmp expect actual
+'
+
 batch_command_multiple_info="info $hello_sha1
 info $tree_sha1
 info $commit_sha1