cat-file: add support for "-allow-unknown-type"

'git cat-file' throws an error while trying to print the type or size of a broken/corrupt object. This is because these objects are usually of unknown types.

Teach git cat-file a '--allow-unknown-type' option where it prints the type or size of a broken/corrupt object without throwing an error.

Modify '-t' and '-s' options to call sha1_object_info_extended() directly to support the '--allow-unknown-type' option.

Add documentation for 'cat-file --allow-unknown-type'.

Helped-by Junio C Hamano <[email protected]>

Helped-by Eric Sunshine <[email protected]>

Signed-off-by Karthik Nayak <[email protected]>

cat-file: add documentation for '--allow-unknown-type' option.

Signed-off-by Karthik Nayak <[email protected]>

---

builtin/cat-file.c +2 -1

index 53b5376..ecb4888 100644
@@ -9,13 +9,20 @@
 #include "userdiff.h"
 #include "streaming.h"
 
-static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
+static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
+                       int unknown_type)
 {
        unsigned char sha1[20];

builtin/cat-file.c +5 -2

        const struct option options[] = {
                OPT_GROUP(N_("<type> can be one of: blob, tree, commit, tag")),

@@ -369,6 +379,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
                OPT_CMDMODE('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
                OPT_CMDMODE(0, "textconv", &opt,
                            N_("for blob objects, run textconv on object's content"), 'c'),
+               OPT_BOOL( 0, "allow-unknown-type", &unknown_type,
+                         N_("allow -s and -t to work with broken/corrupt objects")),
                { OPTION_CALLBACK, 0, "batch", &batch, "format",
                        N_("show info and content of objects fed from the standard input"),
                        PARSE_OPT_OPTARG, batch_option_callback },

@@ -402,5 +414,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
        if (batch.enabled)
                return batch_objects(&batch);
 
-       return cat_one_file(opt, exp_type, obj_name);
+       if (unknown_type && opt != 't' && opt != 's')
+               die("git cat-file --allow-unknown-type: use with -s or -t");
+       return cat_one_file(opt, exp_type, obj_name, unknown_type);
 }
--

2.4.0.rc1.250.gfbd73bd

Signed-off-by Karthik Nayak <[email protected]>

---
 t/t1006-cat-file.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

t/t1006-cat-file.sh +0 -0

index ab36b1e..de8eaf1 100755

t/t1006-cat-file.sh +42 -1

+       test_cmp expect actual
+    '
+
+    test_expect_success "Size of $type is correct using --allow-unknown-type" '
+       echo $size >expect &&
+    git cat-file -s --allow-unknown-type $sha1 >actual &&
+       test_cmp expect actual
+    '
+
     test -z "$content" ||
     test_expect_success "Content of $type is correct" '
        maybe_remove_timestamp "$content" $no_ts >expect &&

@@ -296,4 +308,37 @@ test_expect_success '%(deltabase) reports packed delta bases' '
        }
 '
 
+bogus_type="bogus"
+bogus_content="bogus"
+bogus_size=$(strlen "$bogus_content")
+bogus_sha1=$(echo_without_newline "$bogus_content" | git hash-object -t $bogus_type --literally -w --stdin)
+
+test_expect_success "Type of broken object is correct" '
+       echo $bogus_type >expect &&
+       git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success "Size of broken object is correct" '
+       echo $bogus_size >expect &&
+       git cat-file -s --allow-unknown-type $bogus_sha1 >actual &&
+       test_cmp expect actual
+'
+bogus_type="abcdefghijklmnopqrstuvwxyz1234679"
+bogus_content="bogus"
+bogus_size=$(strlen "$bogus_content")
+bogus_sha1=$(echo_without_newline "$bogus_content" | git hash-object -t $bogus_type --literally -w --stdin)
+
+test_expect_success "Type of broken object is correct when type is large" '
+       echo $bogus_type >expect &&
+       git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success "Size of large broken object is correct when type is large" '
+       echo $bogus_size >expect &&
+       git cat-file -s --allow-unknown-type $bogus_sha1 >actual &&
+       test_cmp expect actual
+'
+
 test_done
--

2.4.0.rc1.250.gfbd73bd

Quoted 13 lines · Karthik Nayak

Signed-off-by Karthik Nayak <[email protected]>

---
  t/t1006-cat-file.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 45 insertions(+)

t/t1006-cat-file.sh +0 -0

index ab36b1e..de8eaf1 100755
@@ -47,6 +47,18 @@ $content"
         test_cmp expect actual
      '
 

t/t1006-cat-file.sh $content" · line 52

Quoted 3 lines

t/t1006-cat-file.sh +3 -0

+    test_expect_success "Type of $type is correct using --allow-unknown-type" '
+       echo $type >expect &&
+    git cat-file -t --allow-unknown-type $sha1 >actual &&

Indentation is still botched in this test and the next one (as mentioned previously [1]).

[1]: http://article.gmane.org/gmane.comp.version-control.git/268005

Quoted 53 lines

t/t1006-cat-file.sh +42 -1

+       test_cmp expect actual
+    '
+
+    test_expect_success "Size of $type is correct using --allow-unknown-type" '
+       echo $size >expect &&
+    git cat-file -s --allow-unknown-type $sha1 >actual &&
+       test_cmp expect actual
+    '
+
      test -z "$content" ||
      test_expect_success "Content of $type is correct" '
         maybe_remove_timestamp "$content" $no_ts >expect &&

@@ -296,4 +308,37 @@ test_expect_success '%(deltabase) reports packed delta bases' '
         }
  '
 
+bogus_type="bogus"
+bogus_content="bogus"
+bogus_size=$(strlen "$bogus_content")
+bogus_sha1=$(echo_without_newline "$bogus_content" | git hash-object -t $bogus_type --literally -w --stdin)
+
+test_expect_success "Type of broken object is correct" '
+       echo $bogus_type >expect &&
+       git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success "Size of broken object is correct" '
+       echo $bogus_size >expect &&
+       git cat-file -s --allow-unknown-type $bogus_sha1 >actual &&
+       test_cmp expect actual
+'
+bogus_type="abcdefghijklmnopqrstuvwxyz1234679"
+bogus_content="bogus"
+bogus_size=$(strlen "$bogus_content")
+bogus_sha1=$(echo_without_newline "$bogus_content" | git hash-object -t $bogus_type --literally -w --stdin)
+
+test_expect_success "Type of broken object is correct when type is large" '
+       echo $bogus_type >expect &&
+       git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success "Size of large broken object is correct when type is large" '
+       echo $bogus_size >expect &&
+       git cat-file -s --allow-unknown-type $bogus_sha1 >actual &&
+       test_cmp expect actual
+'
+
  test_done
--

2.4.0.rc1.250.gfbd73bd

Quoted 8 lines

t/t1006-cat-file.sh $content" · line 52

Quoted 3 lines

t/t1006-cat-file.sh +3 -0

+    test_expect_success "Type of $type is correct using --allow-unknown-type" '
+       echo $type >expect &&
+    git cat-file -t --allow-unknown-type $sha1 >actual &&

Indentation is still botched in this test and the next one (as mentioned previously [1]).

[1]: http://article.gmane.org/gmane.comp.version-control.git/268005

It only seems to have affected the test file, I made sure the indentation was correct after your previous suggestion, I have to see why this is happening. Thanks

Update sha1_loose_object_info() to optionally allow it to read from a loose object file of unknown/bogus type; as the function usually returns the type of the object it read in the form of enum for known types, add an optional "typename" field to receive the name of the type in textual form and a flag to indicate the reading of a loose object file of unknown/bogus type.

Add parse_sha1_header_extended() which acts as a wrapper around parse_sha1_header() allowing more information to be obtained.

Add unpack_sha1_header_to_strbuf() to unpack sha1 headers of unknown/corrupt objects which have a unknown sha1 header size to a strbuf structure. This was written by Junio C Hamano but tested by me.
---

cache.h +2 -0

index 3d3244b..38419c3 100644
@@ -1564,6 +1564,33 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma
+static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
+                                       unsigned long mapsize, void *buffer,

cache.h +6 -1

+       if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
+               return 0;
+

@@ -1614,27 +1641,38 @@ static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long s
  * too permissive for what we want to check. So do an anal
  * object header parse by hand.
  */
-int parse_sha1_header(const char *hdr, unsigned long *sizep)
+static int parse_sha1_header_extended(const char *hdr, struct object_info *oi,
+                              unsigned int flags)
 {
[...]
+

cache.h +4 -0

+       else if (type < 0)
+               die("invalid object type");
+       if (oi->typep)
+               *oi->typep = type;
 
        /*
         * The length must follow immediately, and be in canonical

@@ -1652,12 +1690,24 @@ int parse_sha1_header(const char *hdr, unsigned long *sizep)
                        size = size * 10 + c;

cache.h +3 -1

-       return *hdr ? -1 : type_from_string(type);
+       return *hdr ? -1 : type;
+}
+

Quoted 22 lines · Karthik Nayak

Update sha1_loose_object_info() to optionally allow it to read from a loose object file of unknown/bogus type; as the function usually returns the type of the object it read in the form of enum for known types, add an optional "typename" field to receive the name of the type in textual form and a flag to indicate the reading of a loose object file of unknown/bogus type.

Add parse_sha1_header_extended() which acts as a wrapper around parse_sha1_header() allowing more information to be obtained.

Add unpack_sha1_header_to_strbuf() to unpack sha1 headers of unknown/corrupt objects which have a unknown sha1 header size to a strbuf structure. This was written by Junio C Hamano but tested by me.
---

cache.h +2 -0

index 3d3244b..38419c3 100644
@@ -1564,6 +1564,33 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma
+static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
+                                       unsigned long mapsize, void *buffer,

cache.h int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma · line 1575

Quoted 10 lines

cache.h +10 -0

+                                       unsigned long bufsiz, struct strbuf *header)
+{
+       unsigned char *cp;
+       int status;
+
+       status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);
+
+       /*
+        * Check if entire header is unpacked in the first iteration.
+        */

Nit: You could save some precious vertical screen real-estate by using one-line /* comment style */.

Quoted 13 lines

cache.h +6 -1

+       if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
+               return 0;
+

@@ -1614,27 +1641,38 @@ static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long s
   * too permissive for what we want to check. So do an anal
   * object header parse by hand.
   */
-int parse_sha1_header(const char *hdr, unsigned long *sizep)
+static int parse_sha1_header_extended(const char *hdr, struct object_info *oi,
+                              unsigned int flags)
  {
[...]
+

cache.h static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long s

Quoted 10 lines

cache.h +10 -0

+       type = type_from_string_gently(type_buf, type_len, 1);
+       if (oi->typename)
+               strbuf_add(oi->typename, type_buf, type_len);
+       /*
+        * Set type to 0 if its an unknown object and
+        * we're obtaining the type using '--allow-unkown-type'
+        * option.
+        */
+       if ((flags & LOOKUP_UNKNOWN_OBJECT) && (type < 0))
+               type = 0;

The comment says exactly what the code already says, thus it adds no value. A better comment would explain _why_ type is set to 0 under these conditions.

Quoted 9 lines

cache.h +4 -0

+       else if (type < 0)
+               die("invalid object type");
+       if (oi->typep)
+               *oi->typep = type;
 
         /*
          * The length must follow immediately, and be in canonical

@@ -1652,12 +1690,24 @@ int parse_sha1_header(const char *hdr, unsigned long *sizep)
                         size = size * 10 + c;

cache.h int parse_sha1_header(const char *hdr, unsigned long *sizep) · line 1699

Quoted 10 lines

cache.h +3 -1

                 }
         }
-       *sizep = size;
+
+       if (oi->sizep)
+               *oi->sizep = size;
 
         /*
          * The length must be followed by a zero byte
          */

Nit: Save precious vertical screen real-estate with one-line /* comment style */.

cache.h int parse_sha1_header(const char *hdr, unsigned long *sizep) · line 1702

Quoted 4 lines

cache.h +3 -1

-       return *hdr ? -1 : type_from_string(type);
+       return *hdr ? -1 : type;
+}
+

Hmmmm... you do not seem to pass your own test.

expecting success:

        echo $bogus_type >expect &&
        git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
        test_cmp expect actual

actual 2015-05-04 00:09:24.335335473 +0000 renamed from expect 2015-05-04 00:09:24.327335512 +0000 +1 -1

@@ -1 +1 @@
-abcdefghijklmnopqrstuvwxyz1234679
+abcdefghijklmnopqrstuvwxyz123467abcdefghijklmnopqrstuvwxyz1234679

not ok 86 - Type of broken object is correct when type is large

Quoted 12 lines · Junio C Hamano

Hmmmm... you do not seem to pass your own test.

expecting success:

         echo $bogus_type >expect &&
         git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
         test_cmp expect actual

actual 2015-05-04 00:09:24.335335473 +0000 renamed from expect 2015-05-04 00:09:24.327335512 +0000 +1 -1

@@ -1 +1 @@
-abcdefghijklmnopqrstuvwxyz1234679
+abcdefghijklmnopqrstuvwxyz123467abcdefghijklmnopqrstuvwxyz1234679

Quoted 1 line

not ok 86 - Type of broken object is correct when type is large

Perhaps it would have a better chance of being correct with this squashed in.

-- >8 --
[PATCH] fixup! sha1_file: support reading from a loose object of unknown type

---
  sha1_file.c | 2 --
  1 file changed, 2 deletions(-)

sha1_file.c +0 -1

index f65bf90..e010e7c 100644
@@ -1568,7 +1568,6 @@ static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
  					unsigned long mapsize, void *buffer,
  					unsigned long bufsiz, struct strbuf *header)
  {
-	unsigned char *cp;
  	int status;
 
  	status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);

sha1_file.c +0 -0

  	do {
  		status = git_inflate(stream, 0);
  		strbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);

Review static int unpack_sha1_heade · line 1582

Quoted 5 lines

(quoted diff) +0 -1

@@ -1579,7 +1578,6 @@ static int unpack_sha1_heade
  	if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
  		return 0;
 
-	strbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);

Ok this works, weird, I test before sending patches, anyways getting to the point of discussion, shouldn't we have add the buf, since we did inflate once, before doing inflate again?

Quoted 7 lines · Junio C Hamano

Hmmmm... you do not seem to pass your own test.

expecting success:

        echo $bogus_type >expect &&
        git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
        test_cmp expect actual

Quoted 6 lines

actual 2015-05-04 00:09:24.335335473 +0000 renamed from expect 2015-05-04 00:09:24.327335512 +0000 +1 -1

@@ -1 +1 @@
-abcdefghijklmnopqrstuvwxyz1234679
+abcdefghijklmnopqrstuvwxyz123467abcdefghijklmnopqrstuvwxyz1234679

not ok 86 - Type of broken object is correct when type is large

Perhaps it would have a better chance of being correct with this squashed in.

-- >8 --
[PATCH] fixup! sha1_file: support reading from a loose object of unknown type

---
 sha1_file.c | 2 --
 1 file changed, 2 deletions(-)

sha1_file.c +0 -3

index f65bf90..e010e7c 100644
@@ -1568,7 +1568,6 @@ static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
                                        unsigned long mapsize, void *buffer,
                                        unsigned long bufsiz, struct strbuf *header)
 {
-       unsigned char *cp;
        int status;
 
        status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);

@@ -1579,7 +1578,6 @@ static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
        if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
                return 0;
 
-       strbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);
        do {
                status = git_inflate(stream, 0);
                strbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);
--

2.4.0-282-g4432dd4

Quoted 9 lines · Junio C Hamano

Quoted 7 lines · Junio C Hamano

Hmmmm... you do not seem to pass your own test.

expecting success:

        echo $bogus_type >expect &&
        git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
        test_cmp expect actual

Quoted 9 lines

Quoted 6 lines

actual 2015-05-04 00:09:24.335335473 +0000 renamed from expect 2015-05-04 00:09:24.327335512 +0000 +1 -1

@@ -1 +1 @@
-abcdefghijklmnopqrstuvwxyz1234679
+abcdefghijklmnopqrstuvwxyz123467abcdefghijklmnopqrstuvwxyz1234679

not ok 86 - Type of broken object is correct when type is large

Perhaps it would have a better chance of being correct with this squashed in.

Interestingly, neither test passes on Mac OS X even with this fixup. In fact, the git-hash-object invocation which computes/retrieves 'bogus_sha1' with the extra long bogus type crashes with SIGABRT in write_sha1_file_prepare(). Still investigating.

Quoted 20 lines · Eric Sunshine

Quoted 18 lines · Junio C Hamano

Quoted 12 lines · Junio C Hamano

Hmmmm... you do not seem to pass your own test.

expecting success:

         echo $bogus_type >expect &&
         git cat-file -t --allow-unknown-type $bogus_sha1 >actual &&
         test_cmp expect actual

actual 2015-05-04 00:09:24.335335473 +0000 renamed from expect 2015-05-04 00:09:24.327335512 +0000 +1 -1

@@ -1 +1 @@
-abcdefghijklmnopqrstuvwxyz1234679
+abcdefghijklmnopqrstuvwxyz123467abcdefghijklmnopqrstuvwxyz1234679

Quoted 1 line

not ok 86 - Type of broken object is correct when type is large

Perhaps it would have a better chance of being correct with this squashed in.

Quoted 4 lines

Interestingly, neither test passes on Mac OS X even with this fixup. In fact, the git-hash-object invocation which computes/retrieves 'bogus_sha1' with the extra long bogus type crashes with SIGABRT in write_sha1_file_prepare(). Still investigating.

It's a buffer overflow problem. I'm preparing a patch.

No other changes for now, apart from the changes suggested by you and Eric. You can merge it into 'next'.

Quoted 2 lines · karthik nayak

No other changes for now, apart from the changes suggested by you and Eric. You can merge it into 'next'.

Do you mean by "the changes suggested" the SQUASH queued on the topic? If it is more than that, then I'd prefer you to send an

updated series, as I do not want to guess how you'd plan to respond to these suggestions.

Thanks.

By saying "the changes suggested" I meant the SQUASH queued on topic