[2/4] archive-tar: fix pax extended header length calculation
diff mbox series

Message ID 4b04cf3f-966a-3aa9-6a30-7ac3a234e1ed@web.de
State New
Headers show
Series
  • fix pax extended header length calculation
Related show

Commit Message

René Scharfe Aug. 17, 2019, 4:24 p.m. UTC
A pax extended header records starts with a decimal number.  Its value
is the length of the whole record, including its own length.

The calculation of that number if 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 <l.s.r@web.de>
---
 archive-tar.c                   | 2 +-
 t/t5004-archive-corner-cases.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--
2.23.0

Comments

Eric Sunshine Aug. 17, 2019, 6:03 p.m. UTC | #1
On Sat, Aug 17, 2019 at 12:24 PM René Scharfe <l.s.r@web.de> wrote:
> A pax extended header records starts with a decimal number.  Its value

s/records/record/

> is the length of the whole record, including its own length.
>
> The calculation of that number if strbuf_append_ext_header() is off by

s/if/in/

> 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 <l.s.r@web.de>

Patch
diff mbox series

diff --git a/archive-tar.c b/archive-tar.c
index 355c8142c6..4395a29ffb 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 &&