cat-file: add option '-Z' that delimits input and output with NUL
In db9d67f2e9 (builtin/cat-file.c: support NUL-delimited input with `-z`, 2022-07-22), we have introduced a new mode to read the input via NUL-delimited records instead of newline-delimited records. This allows the user to query for revisions that have newlines in their path component. While unusual, such queries are perfectly valid and thus it is clear that we should be able to support them properly. Unfortunately, the commit only changed the input to be NUL-delimited, but didn't change the output at the same time. While this is fine for queries that are processed successfully, it is less so for queries that aren't. In the case of missing commits for example the result can become entirely unparsable: ``` $ printf "7ce4f05bae8120d9fa258e854a8669f6ea9cb7b1 blob 10\n1234567890\n\n\commit000" | git cat-file --batch -z 7ce4f05bae8120d9fa258e854a8669f6ea9cb7b1 blob 10 1234567890 commit missing ``` This is of course a crafted query that is intentionally gaming the deficiency, but more benign queries that contain newlines would have similar problems. Ideally, we should have also changed the output to be NUL-delimited when `-z` is specified to avoid this problem. As the input is NUL-delimited, it is clear that the output in this case cannot ever contain NUL characters by itself. Furthermore, Git does not allow NUL characters in revisions anyway, further stressing the point that using NUL-delimited output is safe. The only exception is of course the object data itself, but as git-cat-file(1) prints the size of the object data clients should read until that specified size has been consumed. But even though `-z` has only been introduced a few releases ago in Git v2.38.0, changing the output format retroactively to also NUL-delimit output would be a backwards incompatible change. And while one could make the argument that the output is inherently broken already, we need to assume that there are existing users out there that use it just fine given that revisions containing newlines are quite exotic. Instead, introduce a new option `-Z` that switches to NUL-delimited input and output. While this new option could arguably only switch the output format to be NUL-delimited, the consequence would be that users have to always specify both `-z` and `-Z` when the input may contain newlines. On the other hand, if the user knows that there never will be newlines in the input, they don't have to use either of those options. There is thus no usecase that would warrant treating input and output format separately, which is why we instead opt to "do the right thing" and have `-Z` mean to NUL-terminate both formats. The old `-z` option is marked as deprecated with a hint that its output may become unparsable. It is thus hidden both from the synopsis as well as the command's help output. Co-authored-by: Toon Claes <toon@iotcl.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Jun 6, 2023 at 07:19 UTC
f79e18849b5dd8ad83f38744a2e08370f919491a
3 files changed
+139
-56
Documentation/git-cat-file.txt
+13
-2
@@ -14,7 +14,7 @@ SYNOPSIS
14
'git cat-file' (-t | -s) [--allow-unknown-type] <object>
15
'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]
16
[--buffer] [--follow-symlinks] [--unordered]
17
- [--textconv | --filters] [-z]
17
+ [--textconv | --filters] [-Z]
18
'git cat-file' (--textconv | --filters)
19
[<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]
20
@@ -243,10 +243,16 @@ respectively print:
243
/etc/passwd
244
--
245
246
+-Z::
247
+ Only meaningful with `--batch`, `--batch-check`, or
248
+ `--batch-command`; input and output is NUL-delimited instead of
249
+ newline-delimited.
250
+
251
-z::
252
Only meaningful with `--batch`, `--batch-check`, or
253
`--batch-command`; input is NUL-delimited instead of
249
- newline-delimited.
254
+ newline-delimited. This option is deprecated in favor of
255
+ `-Z` as the output can otherwise be ambiguous.
256
257
258
OUTPUT
@@ -384,6 +390,11 @@ notdir SP <size> LF
390
is printed when, during symlink resolution, a file is used as a
391
directory name.
392
393
+Alternatively, when `-Z` is passed, the line feeds in any of the above examples
394
+are replaced with NUL terminators. This ensures that output will be parsable if
395
+the output itself would contain a linefeed and is thus recommended for
396
+scripting purposes.
397
+
398
CAVEATS
399
-------
400
builtin/cat-file.c
+36
-21
@@ -43,6 +43,7 @@ struct batch_options {
43
int unordered;
44
int transform_mode; /* may be 'w' or 'c' for --filters or --textconv */
45
char input_delim;
46
+ char output_delim;
47
const char *format;
48
};
49
@@ -437,11 +438,12 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
438
}
439
}
440
440
-static void print_default_format(struct strbuf *scratch, struct expand_data *data)
441
+static void print_default_format(struct strbuf *scratch, struct expand_data *data,
442
+ struct batch_options *opt)
443
{
442
- strbuf_addf(scratch, "%s %s %"PRIuMAX"\n", oid_to_hex(&data->oid),
444
+ strbuf_addf(scratch, "%s %s %"PRIuMAX"%c", oid_to_hex(&data->oid),
445
type_name(data->type),
444
- (uintmax_t)data->size);
446
+ (uintmax_t)data->size, opt->output_delim);
447
}
448
449
/*
@@ -470,8 +472,8 @@ static void batch_object_write(const char *obj_name,
472
&data->oid, &data->info,
473
OBJECT_INFO_LOOKUP_REPLACE);
474
if (ret < 0) {
473
- printf("%s missing\n",
474
- obj_name ? obj_name : oid_to_hex(&data->oid));
475
+ printf("%s missing%c",
476
+ obj_name ? obj_name : oid_to_hex(&data->oid), opt->output_delim);
477
fflush(stdout);
478
return;
479
}
@@ -492,17 +494,17 @@ static void batch_object_write(const char *obj_name,
494
strbuf_reset(scratch);
495
496
if (!opt->format) {
495
- print_default_format(scratch, data);
497
+ print_default_format(scratch, data, opt);
498
} else {
499
strbuf_expand(scratch, opt->format, expand_format, data);
498
- strbuf_addch(scratch, '\n');
500
+ strbuf_addch(scratch, opt->output_delim);
501
}
502
503
batch_write(opt, scratch->buf, scratch->len);
504
505
if (opt->batch_mode == BATCH_MODE_CONTENTS) {
506
print_object_or_die(opt, data);
505
- batch_write(opt, "\n", 1);
507
+ batch_write(opt, &opt->output_delim, 1);
508
}
509
}
510
@@ -520,22 +522,25 @@ static void batch_one_object(const char *obj_name,
522
if (result != FOUND) {
523
switch (result) {
524
case MISSING_OBJECT:
523
- printf("%s missing\n", obj_name);
525
+ printf("%s missing%c", obj_name, opt->output_delim);
526
break;
527
case SHORT_NAME_AMBIGUOUS:
526
- printf("%s ambiguous\n", obj_name);
528
+ printf("%s ambiguous%c", obj_name, opt->output_delim);
529
break;
530
case DANGLING_SYMLINK:
529
- printf("dangling %"PRIuMAX"\n%s\n",
530
- (uintmax_t)strlen(obj_name), obj_name);
531
+ printf("dangling %"PRIuMAX"%c%s%c",
532
+ (uintmax_t)strlen(obj_name),
533
+ opt->output_delim, obj_name, opt->output_delim);
534
break;
535
case SYMLINK_LOOP:
533
- printf("loop %"PRIuMAX"\n%s\n",
534
- (uintmax_t)strlen(obj_name), obj_name);
536
+ printf("loop %"PRIuMAX"%c%s%c",
537
+ (uintmax_t)strlen(obj_name),
538
+ opt->output_delim, obj_name, opt->output_delim);
539
break;
540
case NOT_DIR:
537
- printf("notdir %"PRIuMAX"\n%s\n",
538
- (uintmax_t)strlen(obj_name), obj_name);
541
+ printf("notdir %"PRIuMAX"%c%s%c",
542
+ (uintmax_t)strlen(obj_name),
543
+ opt->output_delim, obj_name, opt->output_delim);
544
break;
545
default:
546
BUG("unknown get_sha1_with_context result %d\n",
@@ -547,9 +552,9 @@ static void batch_one_object(const char *obj_name,
552
}
553
554
if (ctx.mode == 0) {
550
- printf("symlink %"PRIuMAX"\n%s\n",
555
+ printf("symlink %"PRIuMAX"%c%s%c",
556
(uintmax_t)ctx.symlink_path.len,
552
- ctx.symlink_path.buf);
557
+ opt->output_delim, ctx.symlink_path.buf, opt->output_delim);
558
fflush(stdout);
559
return;
560
}
@@ -913,6 +918,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
918
struct batch_options batch = {0};
919
int unknown_type = 0;
920
int input_nul_terminated = 0;
921
+ int nul_terminated = 0;
922
923
const char * const usage[] = {
924
N_("git cat-file <type> <object>"),
@@ -920,7 +926,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
926
N_("git cat-file (-t | -s) [--allow-unknown-type] <object>"),
927
N_("git cat-file (--batch | --batch-check | --batch-command) [--batch-all-objects]\n"
928
" [--buffer] [--follow-symlinks] [--unordered]\n"
923
- " [--textconv | --filters] [-z]"),
929
+ " [--textconv | --filters] [-Z]"),
930
N_("git cat-file (--textconv | --filters)\n"
931
" [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]"),
932
NULL
@@ -949,7 +955,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
955
N_("like --batch, but don't emit <contents>"),
956
PARSE_OPT_OPTARG | PARSE_OPT_NONEG,
957
batch_option_callback),
952
- OPT_BOOL('z', NULL, &input_nul_terminated, N_("stdin is NUL-terminated")),
958
+ OPT_BOOL_F('z', NULL, &input_nul_terminated, N_("stdin is NUL-terminated"),
959
+ PARSE_OPT_HIDDEN),
960
+ OPT_BOOL('Z', NULL, &nul_terminated, N_("stdin and stdout is NUL-terminated")),
961
OPT_CALLBACK_F(0, "batch-command", &batch, N_("format"),
962
N_("read commands from stdin"),
963
PARSE_OPT_OPTARG | PARSE_OPT_NONEG,
@@ -1011,8 +1019,15 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
1019
else if (input_nul_terminated)
1020
usage_msg_optf(_("'%s' requires a batch mode"), usage, options,
1021
"-z");
1022
+ else if (nul_terminated)
1023
+ usage_msg_optf(_("'%s' requires a batch mode"), usage, options,
1024
+ "-Z");
1025
1015
- batch.input_delim = input_nul_terminated ? '\0' : '\n';
1026
+ batch.input_delim = batch.output_delim = '\n';
1027
+ if (input_nul_terminated)
1028
+ batch.input_delim = '\0';
1029
+ if (nul_terminated)
1030
+ batch.input_delim = batch.output_delim = '\0';
1031
1032
/* Batch defaults */
1033
if (batch.buffer_output < 0)
t/t1006-cat-file.sh
+90
-33
@@ -89,7 +89,8 @@ done
89
for opt in --buffer \
90
--follow-symlinks \
91
--batch-all-objects \
92
- -z
92
+ -z \
93
+ -Z
94
do
95
test_expect_success "usage: bad option combination: $opt without batch mode" '
96
test_incompatible_usage git cat-file $opt &&
@@ -392,17 +393,18 @@ deadbeef
393
394
"
395
395
-batch_output="$hello_sha1 blob $hello_size
396
-$hello_content
397
-$commit_sha1 commit $commit_size
398
-$commit_content
399
-$tag_sha1 tag $tag_size
400
-$tag_content
401
-deadbeef missing
402
- missing"
396
+printf "%s\0" \
397
+ "$hello_sha1 blob $hello_size" \
398
+ "$hello_content" \
399
+ "$commit_sha1 commit $commit_size" \
400
+ "$commit_content" \
401
+ "$tag_sha1 tag $tag_size" \
402
+ "$tag_content" \
403
+ "deadbeef missing" \
404
+ " missing" >batch_output
405
406
test_expect_success '--batch with multiple sha1s gives correct format' '
405
- echo "$batch_output" >expect &&
407
+ tr "\0" "\n" <batch_output >expect &&
408
echo_without_newline "$batch_input" >in &&
409
git cat-file --batch <in >actual &&
410
test_cmp expect actual
@@ -410,11 +412,17 @@ test_expect_success '--batch with multiple sha1s gives correct format' '
412
413
test_expect_success '--batch, -z with multiple sha1s gives correct format' '
414
echo_without_newline_nul "$batch_input" >in &&
413
- echo "$batch_output" >expect &&
415
+ tr "\0" "\n" <batch_output >expect &&
416
git cat-file --batch -z <in >actual &&
417
test_cmp expect actual
418
'
419
420
+test_expect_success '--batch, -Z with multiple sha1s gives correct format' '
421
+ echo_without_newline_nul "$batch_input" >in &&
422
+ git cat-file --batch -Z <in >actual &&
423
+ test_cmp batch_output actual
424
+'
425
+
426
batch_check_input="$hello_sha1
427
$tree_sha1
428
$commit_sha1
@@ -423,40 +431,55 @@ deadbeef
431
432
"
433
426
-batch_check_output="$hello_sha1 blob $hello_size
427
-$tree_sha1 tree $tree_size
428
-$commit_sha1 commit $commit_size
429
-$tag_sha1 tag $tag_size
430
-deadbeef missing
431
- missing"
434
+printf "%s\0" \
435
+ "$hello_sha1 blob $hello_size" \
436
+ "$tree_sha1 tree $tree_size" \
437
+ "$commit_sha1 commit $commit_size" \
438
+ "$tag_sha1 tag $tag_size" \
439
+ "deadbeef missing" \
440
+ " missing" >batch_check_output
441
442
test_expect_success "--batch-check with multiple sha1s gives correct format" '
434
- echo "$batch_check_output" >expect &&
443
+ tr "\0" "\n" <batch_check_output >expect &&
444
echo_without_newline "$batch_check_input" >in &&
445
git cat-file --batch-check <in >actual &&
446
test_cmp expect actual
447
'
448
449
test_expect_success "--batch-check, -z with multiple sha1s gives correct format" '
441
- echo "$batch_check_output" >expect &&
450
+ tr "\0" "\n" <batch_check_output >expect &&
451
echo_without_newline_nul "$batch_check_input" >in &&
452
git cat-file --batch-check -z <in >actual &&
453
test_cmp expect actual
454
'
455
447
-test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '
456
+test_expect_success "--batch-check, -Z with multiple sha1s gives correct format" '
457
+ echo_without_newline_nul "$batch_check_input" >in &&
458
+ git cat-file --batch-check -Z <in >actual &&
459
+ test_cmp batch_check_output actual
460
+'
461
+
462
+test_expect_success FUNNYNAMES 'setup with newline in input' '
463
touch -- "newline${LF}embedded" &&
464
git add -- "newline${LF}embedded" &&
465
git commit -m "file with newline embedded" &&
466
test_tick &&
467
453
- printf "HEAD:newline${LF}embedded" >in &&
454
- git cat-file --batch-check -z <in >actual &&
468
+ printf "HEAD:newline${LF}embedded" >in
469
+'
470
471
+test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '
472
+ git cat-file --batch-check -z <in >actual &&
473
echo "$(git rev-parse "HEAD:newline${LF}embedded") blob 0" >expect &&
474
test_cmp expect actual
475
'
476
477
+test_expect_success FUNNYNAMES '--batch-check, -Z with newline in input' '
478
+ git cat-file --batch-check -Z <in >actual &&
479
+ printf "%s\0" "$(git rev-parse "HEAD:newline${LF}embedded") blob 0" >expect &&
480
+ test_cmp expect actual
481
+'
482
+
483
batch_command_multiple_info="info $hello_sha1
484
info $tree_sha1
485
info $commit_sha1
@@ -480,7 +503,13 @@ test_expect_success '--batch-command with multiple info calls gives correct form
503
echo "$batch_command_multiple_info" | tr "\n" "\0" >in &&
504
git cat-file --batch-command --buffer -z <in >actual &&
505
483
- test_cmp expect actual
506
+ test_cmp expect actual &&
507
+
508
+ echo "$batch_command_multiple_info" | tr "\n" "\0" >in &&
509
+ tr "\n" "\0" <expect >expect_nul &&
510
+ git cat-file --batch-command --buffer -Z <in >actual &&
511
+
512
+ test_cmp expect_nul actual
513
'
514
515
batch_command_multiple_contents="contents $hello_sha1
@@ -490,15 +519,15 @@ contents deadbeef
519
flush"
520
521
test_expect_success '--batch-command with multiple command calls gives correct format' '
493
- cat >expect <<-EOF &&
494
- $hello_sha1 blob $hello_size
495
- $hello_content
496
- $commit_sha1 commit $commit_size
497
- $commit_content
498
- $tag_sha1 tag $tag_size
499
- $tag_content
500
- deadbeef missing
501
- EOF
522
+ printf "%s\0" \
523
+ "$hello_sha1 blob $hello_size" \
524
+ "$hello_content" \
525
+ "$commit_sha1 commit $commit_size" \
526
+ "$commit_content" \
527
+ "$tag_sha1 tag $tag_size" \
528
+ "$tag_content" \
529
+ "deadbeef missing" >expect_nul &&
530
+ tr "\0" "\n" <expect_nul >expect &&
531
532
echo "$batch_command_multiple_contents" >in &&
533
git cat-file --batch-command --buffer <in >actual &&
@@ -508,7 +537,12 @@ test_expect_success '--batch-command with multiple command calls gives correct f
537
echo "$batch_command_multiple_contents" | tr "\n" "\0" >in &&
538
git cat-file --batch-command --buffer -z <in >actual &&
539
511
- test_cmp expect actual
540
+ test_cmp expect actual &&
541
+
542
+ echo "$batch_command_multiple_contents" | tr "\n" "\0" >in &&
543
+ git cat-file --batch-command --buffer -Z <in >actual &&
544
+
545
+ test_cmp expect_nul actual
546
'
547
548
test_expect_success 'setup blobs which are likely to delta' '
@@ -848,6 +882,13 @@ test_expect_success 'git cat-file --batch-check --follow-symlinks works for brok
882
test_cmp expect actual
883
'
884
885
+test_expect_success 'git cat-file --batch-check --follow-symlinks -Z works for broken in-repo, same-dir links' '
886
+ printf "HEAD:broken-same-dir-link\0" >in &&
887
+ printf "dangling 25\0HEAD:broken-same-dir-link\0" >expect &&
888
+ git cat-file --batch-check --follow-symlinks -Z <in >actual &&
889
+ test_cmp expect actual
890
+'
891
+
892
test_expect_success 'git cat-file --batch-check --follow-symlinks works for same-dir links-to-links' '
893
echo HEAD:link-to-link | git cat-file --batch-check --follow-symlinks >actual &&
894
test_cmp found actual
@@ -862,6 +903,15 @@ test_expect_success 'git cat-file --batch-check --follow-symlinks works for pare
903
test_cmp expect actual
904
'
905
906
+test_expect_success 'git cat-file --batch-check --follow-symlinks -Z works for parent-dir links' '
907
+ echo HEAD:dir/parent-dir-link | git cat-file --batch-check --follow-symlinks >actual &&
908
+ test_cmp found actual &&
909
+ printf "notdir 29\0HEAD:dir/parent-dir-link/nope\0" >expect &&
910
+ printf "HEAD:dir/parent-dir-link/nope\0" >in &&
911
+ git cat-file --batch-check --follow-symlinks -Z <in >actual &&
912
+ test_cmp expect actual
913
+'
914
+
915
test_expect_success 'git cat-file --batch-check --follow-symlinks works for .. links' '
916
echo dangling 22 >expect &&
917
echo HEAD:dir/link-dir/nope >>expect &&
@@ -976,6 +1026,13 @@ test_expect_success 'git cat-file --batch-check --follow-symlink breaks loops' '
1026
test_cmp expect actual
1027
'
1028
1029
+test_expect_success 'git cat-file --batch-check --follow-symlink -Z breaks loops' '
1030
+ printf "loop 10\0HEAD:loop1\0" >expect &&
1031
+ printf "HEAD:loop1\0" >in &&
1032
+ git cat-file --batch-check --follow-symlinks -Z <in >actual &&
1033
+ test_cmp expect actual
1034
+'
1035
+
1036
test_expect_success 'git cat-file --batch --follow-symlink returns correct sha and mode' '
1037
echo HEAD:morx | git cat-file --batch >expect &&
1038
echo HEAD:morx | git cat-file --batch --follow-symlinks >actual &&