Re: [PATCH v3 1/3] tree-walk: be more specific about corrupt tree errors

[Jeff King]

On Tue, Sep 27, 2016 at 11:23:24AM -0400, David Turner wrote:

> +test_expect_success 'malformed mode in tree' '
> + hex_sha1=$(echo foo | git hash-object --stdin -w) &&
> + bin_sha1=$(echo $hex_sha1 | perl -ne "printf \"%03o\", ord for 
> /../g") &&

Sorry, the perl snippet I posted earlier was totally braindead. It gives
you the octal for the raw bytes, but we really just want to convert the
hex to octal (we could also print the raw bytes from the hex, but I
didn't want to take chances on shells that can't handle NULs in
environment variables).

I also find it helps to define a helper function outside of the test
block to avoid quoting hell. So something like:

  hex2oct () {
perl -ne 'printf "\\%03o", hex for /../g'
  }

  test_expect_success ... '
bin_sha1=$(echo $hex_sha1 | hex2oct)
  '

-Peff

Re: [PATCH v3 1/3] tree-walk: be more specific about corrupt tree errors

[Junio C Hamano]

David Turner  writes:

> From: Jeff King 
>
> When the tree-walker runs into an error, it just calls
> die(), and the message is always "corrupt tree file".
> However, we are actually covering several cases here; let's
> give the user a hint about what happened.
>
> Let's also avoid using the word "corrupt", which makes it
> seem like the data bit-rotted on disk. Our sha1 check would
> already have found that. These errors are ones of data that
> is malformed in the first place.
>
> Signed-off-by: David Turner 
> Signed-off-by: Jeff King 
> ---
>  t/t1007-hash-object.sh | 21 +++--
>  tree-walk.c| 12 +++-
>  2 files changed, 26 insertions(+), 7 deletions(-)

Nice that we now prepare the test data ourselves without shipping as
part of the source.

[PATCH v3 1/3] tree-walk: be more specific about corrupt tree errors

[David Turner]

From: Jeff King 

When the tree-walker runs into an error, it just calls
die(), and the message is always "corrupt tree file".
However, we are actually covering several cases here; let's
give the user a hint about what happened.

Let's also avoid using the word "corrupt", which makes it
seem like the data bit-rotted on disk. Our sha1 check would
already have found that. These errors are ones of data that
is malformed in the first place.

Signed-off-by: David Turner 
Signed-off-by: Jeff King 
---
 t/t1007-hash-object.sh | 21 +++--
 tree-walk.c| 12 +++-
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index acca9ac..0691b88 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -183,9 +183,26 @@ for args in "-w --stdin-paths" "--stdin-paths -w"; do
pop_repo
 done
 
-test_expect_success 'corrupt tree' '
+test_expect_success 'too-short tree' '
echo abc >malformed-tree &&
-   test_must_fail git hash-object -t tree malformed-tree
+   test_must_fail git hash-object -t tree malformed-tree 2>err &&
+   test_i18ngrep "too-short tree object" err
+'
+
+test_expect_success 'malformed mode in tree' '
+   hex_sha1=$(echo foo | git hash-object --stdin -w) &&
+   bin_sha1=$(echo $hex_sha1 | perl -ne "printf \"%03o\", ord for 
/../g") &&
+   printf "9100644 \0$bin_sha1" >tree-with-malformed-mode &&
+   test_must_fail git hash-object -t tree tree-with-malformed-mode 2>err &&
+   test_i18ngrep "malformed mode in tree entry" err
+'
+
+test_expect_success 'empty filename in tree' '
+   hex_sha1=$(echo foo | git hash-object --stdin -w) &&
+   bin_sha1=$(echo $hex_sha1 | perl -ne "printf \"%03o\", ord for 
/../g") &&
+   printf "100644 \0$bin_sha1" >tree-with-empty-filename &&
+   test_must_fail git hash-object -t tree tree-with-empty-filename 2>err &&
+   test_i18ngrep "empty filename in tree entry" err
 '
 
 test_expect_success 'corrupt commit' '
diff --git a/tree-walk.c b/tree-walk.c
index ce27842..24f9a0f 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -27,12 +27,14 @@ static void decode_tree_entry(struct tree_desc *desc, const 
char *buf, unsigned
const char *path;
unsigned int mode, len;
 
-   if (size < 24 || buf[size - 21])
-   die("corrupt tree file");
+   if (size < 23 || buf[size - 21])
+   die(_("too-short tree object"));
 
path = get_mode(buf, );
-   if (!path || !*path)
-   die("corrupt tree file");
+   if (!path)
+   die(_("malformed mode in tree entry for tree"));
+   if (!*path)
+   die(_("empty filename in tree entry for tree"));
len = strlen(path) + 1;
 
/* Initialize the descriptor entry */
@@ -81,7 +83,7 @@ void update_tree_entry(struct tree_desc *desc)
unsigned long len = end - (const unsigned char *)buf;
 
if (size < len)
-   die("corrupt tree file");
+   die(_("too-short tree file"));
buf = end;
size -= len;
desc->buffer = buf;
-- 
2.8.0.rc4.22.g8ae061a