git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 3/9] make unpack_object_header() non fatal

From
Nicolas Pitre <nico@cam.org>
Date
Oct 29, 2008, 23:02 UTC
Message-ID
<1225321372-6570-4-git-send-email-nico@cam.org>
In-Reply-To
<1225321372-6570-3-git-send-email-nico@cam.org>

It is possible to have pack corruption in the object header. Currently unpack_object_header() simply die() on them instead of letting the caller deal with that gracefully.

So let's have unpack_object_header() return an error instead, and find a better name for unpack_object_header_gently() in that context. All callers of unpack_object_header() are ready for it.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 builtin-pack-objects.c |    2 +-
 cache.h                |    2 +-
 sha1_file.c            |   20 +++++++++++---------
 3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index d4c721b..9e249c9 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1002,7 +1002,7 @@ static void check_object(struct object_entry *entry)
 		 * We want in_pack_type even if we do not reuse delta
 		 * since non-delta representations could still be reused.
 		 */
-		used = unpack_object_header_gently(buf, avail,
+		used = unpack_object_header_buffer(buf, avail,
 						   &entry->in_pack_type,
 						   &entry->size);
 
diff --git a/cache.h b/cache.h
index a3c77f0..1a9edf3 100644
--- a/cache.h
+++ b/cache.h
@@ -751,7 +751,7 @@ extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t
 extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t);
 extern off_t find_pack_entry_one(const unsigned char *, struct packed_git *);
 extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsigned long *);
-extern unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
+extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
 extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
 extern const char *packed_object_info_detail(struct packed_git *, off_t, unsigned long *, unsigned long *, unsigned int *, unsigned char *);
 extern int matches_pack_name(struct packed_git *p, const char *name);
diff --git a/sha1_file.c b/sha1_file.c
index e57949b..7698177 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1110,7 +1110,8 @@ static int legacy_loose_object(unsigned char *map)
 		return 0;
 }
 
-unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep)
+unsigned long unpack_object_header_buffer(const unsigned char *buf,
+		unsigned long len, enum object_type *type, unsigned long *sizep)
 {
 	unsigned shift;
 	unsigned char c;
@@ -1122,10 +1123,10 @@ unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned lon
 	size = c & 15;
 	shift = 4;
 	while (c & 0x80) {
-		if (len <= used)
-			return 0;
-		if (sizeof(long) * 8 <= shift)
+		if (len <= used || sizeof(long) * 8 <= shift) {
+			error("bad object header");
 			return 0;
+		}
 		c = buf[used++];
 		size += (c & 0x7f) << shift;
 		shift += 7;
@@ -1164,7 +1165,7 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon
 	 * really worth it and we don't write it any longer.  But we
 	 * can still read it.
 	 */
-	used = unpack_object_header_gently(map, mapsize, &type, &size);
+	used = unpack_object_header_buffer(map, mapsize, &type, &size);
 	if (!used || !valid_loose_object_type[type])
 		return -1;
 	map += used;
@@ -1411,10 +1412,11 @@ static int unpack_object_header(struct packed_git *p,
 	 * insane, so we know won't exceed what we have been given.
 	 */
 	base = use_pack(p, w_curs, *curpos, &left);
-	used = unpack_object_header_gently(base, left, &type, sizep);
-	if (!used)
-		die("object offset outside of pack file");
-	*curpos += used;
+	used = unpack_object_header_buffer(base, left, &type, sizep);
+	if (!used) {
+		type = OBJ_BAD;
+	} else
+		*curpos += used;
 
 	return type;
 }
-- 
1.6.0.3.757.g01be.dirty
Previous: Nicolas PitreNext: Nicolas Pitre
Message 4 of 12 in “more robustness against pack corruptions”
  1. 0/9 more robustness against pack corruptionsNicolas Pitre, Oct 29, 2008
  2. 1/9 close another possibility for propagating pack corruptionNicolas Pitre, Oct 29, 2008
  3. 2/9 better validation on delta base object offsetsNicolas Pitre, Oct 29, 2008
  4. 3/9 make unpack_object_header() non fatalNicolas Pitre, Oct 29, 2008
  5. 4/9 make packed_object_info() resilient to pack corruptionsNicolas Pitre, Oct 29, 2008
  6. 5/9 make check_object() resilient to pack corruptionsNicolas Pitre, Oct 29, 2008
  7. 6/9 make find_pack_revindex() aware of the nasty worldNicolas Pitre, Oct 29, 2008
  8. 7/9 pack-objects: allow "fixing" a corrupted pack without a full repackNicolas Pitre, Oct 29, 2008
  9. 8/9 extend test coverage for latest pack corruption resilience improvementsNicolas Pitre, Oct 29, 2008
  10. 9/9 pack-objects: don't leak pack window reference when splitting packsNicolas Pitre, Oct 29, 2008
  11. Junio C HamanoOct 31, 2008
  12. Nicolas PitreOct 31, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.