From 4060c1990a9cceb710808bd4b4ab94d2355aefff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 17 Aug 2019 18:23:52 +0200 Subject: [PATCH 1/4] archive-tar: report wrong pax extended header length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extended header entries contain a length value that is a bit tricky to calculate because it includes its own length (number of decimal digits) as well. We get it wrong in corner cases. Add a check, report wrong results as a warning and add a test for exercising it. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- archive-tar.c | 6 ++++++ t/t5004-archive-corner-cases.sh | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/archive-tar.c b/archive-tar.c index 4aabd566fb..181da4e843 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -144,6 +144,7 @@ static int stream_blocked(const struct object_id *oid) static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword, const char *value, unsigned int valuelen) { + size_t orig_len = sb->len; int len, tmp; /* "%u %s=%s\n" */ @@ -155,6 +156,11 @@ static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword, strbuf_addf(sb, "%u %s=", len, keyword); strbuf_add(sb, value, valuelen); strbuf_addch(sb, '\n'); + + if (len != sb->len - orig_len) + warning("pax extended header length miscalculated as %d" + ", should be %"PRIuMAX, + len, (uintmax_t)(sb->len - orig_len)); } /* diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh index 271eb5a1fd..2f15d1899d 100755 --- a/t/t5004-archive-corner-cases.sh +++ b/t/t5004-archive-corner-cases.sh @@ -204,4 +204,24 @@ test_expect_success EXPENSIVE,LONG_IS_64BIT,UNZIP,UNZIP_ZIP64_SUPPORT,ZIPINFO \ grep $size big.lst ' +build_tree() { + perl -e ' + my $hash = $ARGV[0]; + foreach my $order (2..6) { + $first = 10 ** $order; + foreach my $i (-13..-9) { + my $name = "a" x ($first + $i); + print "100644 blob $hash\t$name\n" + } + } + ' "$1" +} + +test_expect_failure 'tar archive with long paths' ' + blob=$(echo foo | git hash-object -w --stdin) && + tree=$(build_tree $blob | git mktree) && + git archive -o long_paths.tar $tree 2>stderr && + test_must_be_empty stderr +' + test_done From 82a46af13eeed4b73a077b50edb90f559460267f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 17 Aug 2019 18:24:01 +0200 Subject: [PATCH 2/4] archive-tar: fix pax extended header length calculation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pax extended header record starts with a decimal number. Its value is the length of the whole record, including its own length. The calculation of that number in strbuf_append_ext_header() is off by one in case the length of the rest is close to a higher order of magnitude. This affects paths and link targets a bit shorter than 1000, 10000, 100000 etc. characters -- paths with a length of up to 100 fit into the tar header and don't need a pax extended header. The mistake has been present since the function was added by ae64bbc18c ("tar-tree: Introduce write_entry()", 2006-03-25). Account for digits added to len during the loop and keep incrementing until we have enough space for len and the rest. The crucial change is to check against the current value of len before each iteration, instead of against its value before the loop. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- archive-tar.c | 2 +- t/t5004-archive-corner-cases.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/archive-tar.c b/archive-tar.c index 181da4e843..3d76977c3f 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -149,7 +149,7 @@ static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword, /* "%u %s=%s\n" */ len = 1 + 1 + strlen(keyword) + 1 + valuelen + 1; - for (tmp = len; tmp > 9; tmp /= 10) + for (tmp = 1; len / 10 >= tmp; tmp *= 10) len++; strbuf_grow(sb, len); diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh index 2f15d1899d..4966a74b4d 100755 --- a/t/t5004-archive-corner-cases.sh +++ b/t/t5004-archive-corner-cases.sh @@ -217,7 +217,7 @@ build_tree() { ' "$1" } -test_expect_failure 'tar archive with long paths' ' +test_expect_success 'tar archive with long paths' ' blob=$(echo foo | git hash-object -w --stdin) && tree=$(build_tree $blob | git mktree) && git archive -o long_paths.tar $tree 2>stderr && From 17e9ef00d29b349727e3c710869217a36ad08224 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 17 Aug 2019 18:24:13 +0200 Subject: [PATCH 3/4] archive-tar: use size_t in strbuf_append_ext_header() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One of its callers already passes in a size_t value. Use it consistently in this function. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- archive-tar.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/archive-tar.c b/archive-tar.c index 3d76977c3f..29ca21649e 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -142,10 +142,10 @@ static int stream_blocked(const struct object_id *oid) * string and appends it to a struct strbuf. */ static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword, - const char *value, unsigned int valuelen) + const char *value, size_t valuelen) { size_t orig_len = sb->len; - int len, tmp; + size_t len, tmp; /* "%u %s=%s\n" */ len = 1 + 1 + strlen(keyword) + 1 + valuelen + 1; @@ -153,14 +153,14 @@ static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword, len++; strbuf_grow(sb, len); - strbuf_addf(sb, "%u %s=", len, keyword); + strbuf_addf(sb, "%"PRIuMAX" %s=", (uintmax_t)len, keyword); strbuf_add(sb, value, valuelen); strbuf_addch(sb, '\n'); if (len != sb->len - orig_len) - warning("pax extended header length miscalculated as %d" + warning("pax extended header length miscalculated as %"PRIuMAX ", should be %"PRIuMAX, - len, (uintmax_t)(sb->len - orig_len)); + (uintmax_t)len, (uintmax_t)(sb->len - orig_len)); } /* From 71d41ff651a54028856c4224777ef405de48ae66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 17 Aug 2019 18:24:23 +0200 Subject: [PATCH 4/4] archive-tar: turn length miscalculation warning into BUG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that we're confident our pax extended header calculation is correct, turn the criticality of the assertion up to the maximum, from warning right up to BUG. Simplify the test, as the stderr comparison step would not be reached in case the BUG message is triggered. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- archive-tar.c | 6 +++--- t/t5004-archive-corner-cases.sh | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/archive-tar.c b/archive-tar.c index 29ca21649e..e7153c69e7 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -158,9 +158,9 @@ static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword, strbuf_addch(sb, '\n'); if (len != sb->len - orig_len) - warning("pax extended header length miscalculated as %"PRIuMAX - ", should be %"PRIuMAX, - (uintmax_t)len, (uintmax_t)(sb->len - orig_len)); + BUG("pax extended header length miscalculated as %"PRIuMAX + ", should be %"PRIuMAX, + (uintmax_t)len, (uintmax_t)(sb->len - orig_len)); } /* diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh index 4966a74b4d..3e7b23cb32 100755 --- a/t/t5004-archive-corner-cases.sh +++ b/t/t5004-archive-corner-cases.sh @@ -220,8 +220,7 @@ build_tree() { test_expect_success 'tar archive with long paths' ' blob=$(echo foo | git hash-object -w --stdin) && tree=$(build_tree $blob | git mktree) && - git archive -o long_paths.tar $tree 2>stderr && - test_must_be_empty stderr + git archive -o long_paths.tar $tree ' test_done