Add lax parsing for commit and tag objectsrb/object-parse-flexibility

This changes the behavior of object parsing for commits and tags
so that even when bad data is found inside the object, we will
continue to try to parse as much of the object as we can.  The
existing functions (`git_object_lookup` for example) will still
delete the partially parsed object before returning an error, but
this also adds a new function `git_object_lookup_lax` that will
still return the error, but will also return the object with the
partial data (if we got far enough along in the parsing process to
even create the base object).

13 files changed, 460 insertions, 343 deletions

diff --git a/src/commit.c b/src/commit.c
index 227d5c4a5..1388cb3f5 100644
--- a/src/commit.c
+++ b/src/commit.c
@@ -328,87 +328,41 @@ int git_commit_amend(
 	return error;
 }
 
-int git_commit__parse(void *_commit, git_odb_object *odb_obj)
+int git_commit__parse(void *obj, git_odb_object *odb_obj)
 {
-	git_commit *commit = _commit;
-	const char *buffer_start = git_odb_object_data(odb_obj), *buffer;
-	const char *buffer_end = buffer_start + git_odb_object_size(odb_obj);
-	git_oid parent_id;
-	size_t header_len;
-
-	buffer = buffer_start;
+	int error = 0;
+	const char *start = git_odb_object_data(odb_obj);
+	const char *end = start + git_odb_object_size(odb_obj);
+	const char *body = NULL;
+	git_commit *commit = obj;
+	git_object_parse_t parser[] = {
+		{ "tree", 4, GIT_PARSE_OID, { .id = &commit->tree_id } },
+		{ "parent", 6, GIT_PARSE_OID_ARRAY, { .ids = &commit->parent_ids } },
+		{ "author", 6, GIT_PARSE_SIGNATURE, { .sig = &commit->author } },
+		{ "committer", 9, GIT_PARSE_SIGNATURE, { .sig = &commit->committer } },
+		{ NULL, 0, GIT_PARSE_MODE_OPTIONAL },
+		{ "encoding", 8, GIT_PARSE_TO_EOL, { .text = &commit->message_encoding } },
+		{ NULL, 0, GIT_PARSE_BODY, { .body = &body } },
+	};
 
 	/* Allocate for one, which will allow not to realloc 90% of the time  */
 	git_array_init_to_size(commit->parent_ids, 1);
 	GITERR_CHECK_ARRAY(commit->parent_ids);
 
-	/* The tree is always the first field */
-	if (git_oid__parse(&commit->tree_id, &buffer, buffer_end, "tree ") < 0)
-		goto bad_buffer;
-
-	/*
-	 * TODO: commit grafts!
-	 */
-
-	while (git_oid__parse(&parent_id, &buffer, buffer_end, "parent ") == 0) {
-		git_oid *new_id = git_array_alloc(commit->parent_ids);
-		GITERR_CHECK_ALLOC(new_id);
-
-		git_oid_cpy(new_id, &parent_id);
-	}
-
-	commit->author = git__malloc(sizeof(git_signature));
-	GITERR_CHECK_ALLOC(commit->author);
-
-	if (git_signature__parse(commit->author, &buffer, buffer_end, "author ", '\n') < 0)
-		return -1;
-
-	/* Always parse the committer; we need the commit time */
-	commit->committer = git__malloc(sizeof(git_signature));
-	GITERR_CHECK_ALLOC(commit->committer);
-
-	if (git_signature__parse(commit->committer, &buffer, buffer_end, "committer ", '\n') < 0)
-		return -1;
-
-	/* Parse add'l header entries */
-	while (buffer < buffer_end) {
-		const char *eoln = buffer;
-		if (buffer[-1] == '\n' && buffer[0] == '\n')
-			break;
-
-		while (eoln < buffer_end && *eoln != '\n')
-			++eoln;
+	error = git_object__parse_lines(GIT_OBJ_COMMIT, parser, start, end);
 
-		if (git__prefixcmp(buffer, "encoding ") == 0) {
-			buffer += strlen("encoding ");
-
-			commit->message_encoding = git__strndup(buffer, eoln - buffer);
-			GITERR_CHECK_ALLOC(commit->message_encoding);
+	/* strdup raw version of header data and commit message */
+	if (body != NULL) {
+		if (body > start) {
+			commit->raw_header = git__strndup(start, (body - 1) - start);
+			GITERR_CHECK_ALLOC(commit->raw_header);
 		}
 
-		if (eoln < buffer_end && *eoln == '\n')
-			++eoln;
-		buffer = eoln;
-	}
-
-	header_len = buffer - buffer_start;
-	commit->raw_header = git__strndup(buffer_start, header_len);
-	GITERR_CHECK_ALLOC(commit->raw_header);
-
-	/* point "buffer" to data after header, +1 for the final LF */
-	buffer = buffer_start + header_len + 1;
-
-	/* extract commit message */
-	if (buffer <= buffer_end) {
-		commit->raw_message = git__strndup(buffer, buffer_end - buffer);
+		commit->raw_message = git__strndup(body, end - body);
 		GITERR_CHECK_ALLOC(commit->raw_message);
 	}
 
-	return 0;
-
-bad_buffer:
-	giterr_set(GITERR_OBJECT, "Failed to parse bad commit object");
-	return -1;
+	return error;
 }
 
 #define GIT_COMMIT_GETTER(_rvalue, _name, _return) \
diff --git a/src/commit.h b/src/commit.h
index efb080b50..45d7e29e3 100644
--- a/src/commit.h
+++ b/src/commit.h
@@ -17,7 +17,7 @@
 struct git_commit {
 	git_object object;
 
-	git_array_t(git_oid) parent_ids;
+	git_oid_array parent_ids;
 	git_oid tree_id;
 
 	git_signature *author;
diff --git a/src/diff_file.c b/src/diff_file.c
index f2a1d5099..a53dfab91 100644
--- a/src/diff_file.c
+++ b/src/diff_file.c
@@ -240,7 +240,7 @@ static int diff_file_content_load_blob(git_diff_file_content *fc)
 
 	if (odb_obj != NULL) {
 		error = git_object__from_odb_object(
-			(git_object **)&fc->blob, fc->repo, odb_obj, GIT_OBJ_BLOB);
+			(git_object **)&fc->blob, fc->repo, odb_obj, GIT_OBJ_BLOB, true);
 		git_odb_object_free(odb_obj);
 	} else {
 		error = git_blob_lookup(
diff --git a/src/diff_tform.c b/src/diff_tform.c
index a2dab0ae2..4d5359edc 100644
--- a/src/diff_tform.c
+++ b/src/diff_tform.c
@@ -508,7 +508,7 @@ static int similarity_sig(
 		if (info->odb_obj != NULL)
 			error = git_object__from_odb_object(
 				(git_object **)&info->blob, info->repo,
-				info->odb_obj, GIT_OBJ_BLOB);
+				info->odb_obj, GIT_OBJ_BLOB, true);
 		else
 			error = git_blob_lookup(&info->blob, info->repo, &file->id);
 
diff --git a/src/object.c b/src/object.c
index 93068b85f..2e0f0e5d4 100644
--- a/src/object.c
+++ b/src/object.c
@@ -13,6 +13,7 @@
 #include "tree.h"
 #include "blob.h"
 #include "tag.h"
+#include "signature.h"
 
 static const int OBJECT_BASE_SIZE = 4096;
 
@@ -48,26 +49,36 @@ static git_object_def git_objects_table[] = {
 	{ "REF_DELTA", 0, NULL, NULL },
 };
 
+static int git_object__match_cache(git_otype type, git_otype cached)
+{
+	if (type == GIT_OBJ_ANY || type == cached)
+		return 0;
+
+	giterr_set(
+		GITERR_INVALID,
+		"Requested object type (%s) does not match type in ODB (%s)",
+		git_object_type2string(type), git_object_type2string(cached));
+	return GIT_ENOTFOUND;
+}
+
 int git_object__from_odb_object(
-	git_object **object_out,
+	git_object **out,
 	git_repository *repo,
 	git_odb_object *odb_obj,
-	git_otype type)
+	git_otype type,
+	bool lax)
 {
 	int error;
 	size_t object_size;
 	git_object_def *def;
 	git_object *object = NULL;
 
-	assert(object_out);
-	*object_out = NULL;
+	assert(out);
+	*out = NULL;
 
 	/* Validate type match */
-	if (type != GIT_OBJ_ANY && type != odb_obj->cached.type) {
-		giterr_set(GITERR_INVALID,
-			"The requested type does not match the type in the ODB");
-		return GIT_ENOTFOUND;
-	}
+	if ((error = git_object__match_cache(type, odb_obj->cached.type)) < 0)
+		return error;
 
 	if ((object_size = git_object__size(odb_obj->cached.type)) == 0) {
 		giterr_set(GITERR_INVALID, "The requested type is invalid");
@@ -87,10 +98,14 @@ int git_object__from_odb_object(
 	def = &git_objects_table[odb_obj->cached.type];
 	assert(def->free && def->parse);
 
-	if ((error = def->parse(object, odb_obj)) < 0)
-		def->free(object);
-	else
-		*object_out = git_cache_store_parsed(&repo->objects, object);
+	if ((error = def->parse(object, odb_obj)) < 0) {
+		if (lax) /* do not put invalid objects into cache */
+			*out = object;
+		else
+			def->free(object);
+	} else {
+		*out = git_cache_store_parsed(&repo->objects, object);
+	}
 
 	return error;
 }
@@ -106,27 +121,33 @@ void git_object__free(void *obj)
 		git_objects_table[type].free(obj);
 }
 
-int git_object_lookup_prefix(
-	git_object **object_out,
+static int object_lookup(
+	git_object **out,
 	git_repository *repo,
 	const git_oid *id,
 	size_t len,
-	git_otype type)
+	git_otype type,
+	bool lax)
 {
-	git_object *object = NULL;
+	int error = 0;
 	git_odb *odb = NULL;
 	git_odb_object *odb_obj = NULL;
-	int error = 0;
 
-	assert(repo && object_out && id);
+	assert(repo && out && id);
 
 	if (len < GIT_OID_MINPREFIXLEN) {
-		giterr_set(GITERR_OBJECT, "Ambiguous lookup - OID prefix is too short");
+		giterr_set(GITERR_OBJECT,
+			"Ambiguous lookup - OID prefix is too short (%d)", (int)len);
 		return GIT_EAMBIGUOUS;
 	}
 
-	error = git_repository_odb__weakptr(&odb, repo);
-	if (error < 0)
+	if (type != GIT_OBJ_ANY && !git_object__size(type)) {
+		giterr_set(
+			GITERR_INVALID, "The requested type (%d) is invalid", (int)type);
+		return GIT_ENOTFOUND;
+	}
+
+	if ((error = git_repository_odb__weakptr(&odb, repo)) < 0)
 		return error;
 
 	if (len > GIT_OID_HEXSZ)
@@ -135,77 +156,88 @@ int git_object_lookup_prefix(
 	if (len == GIT_OID_HEXSZ) {
 		git_cached_obj *cached = NULL;
 
-		/* We want to match the full id : we can first look up in the cache,
-		 * since there is no need to check for non ambiguousity
-		 */
+		/* Full id: first look in cache, since there is no ambiguity */
 		cached = git_cache_get_any(&repo->objects, id);
-		if (cached != NULL) {
-			if (cached->flags == GIT_CACHE_STORE_PARSED) {
-				object = (git_object *)cached;
-
-				if (type != GIT_OBJ_ANY && type != object->cached.type) {
-					git_object_free(object);
-					giterr_set(GITERR_INVALID,
-						"The requested type does not match the type in ODB");
-					return GIT_ENOTFOUND;
-				}
-
-				*object_out = object;
-				return 0;
-			} else if (cached->flags == GIT_CACHE_STORE_RAW) {
-				odb_obj = (git_odb_object *)cached;
-			} else {
-				assert(!"Wrong caching type in the global object cache");
-			}
-		} else {
-			/* Object was not found in the cache, let's explore the backends.
-			 * We could just use git_odb_read_unique_short_oid,
-			 * it is the same cost for packed and loose object backends,
-			 * but it may be much more costly for sqlite and hiredis.
-			 */
+
+		if (!cached)
+			/* Object not found in cache, so search backends */
 			error = git_odb_read(&odb_obj, odb, id);
+		else if (cached->flags == GIT_CACHE_STORE_PARSED) {
+			if ((error = git_object__match_cache(type, cached->type)) < 0)
+				git_object_free((git_object *)cached);
+			else
+				*out = (git_object *)cached;
+			return error;
 		}
+		else if (cached->flags == GIT_CACHE_STORE_RAW)
+			odb_obj = (git_odb_object *)cached;
+		else
+			assert(!"Wrong caching type in the global object cache");
 	} else {
-		git_oid short_oid;
+		git_oid short_oid = {{0}};
 
-		/* We copy the first len*4 bits from id and fill the remaining with 0s */
+		/* Copy first len*4 bits from id and fill the remaining with 0s */
 		memcpy(short_oid.id, id->id, (len + 1) / 2);
 		if (len % 2)
 			short_oid.id[len / 2] &= 0xF0;
-		memset(short_oid.id + (len + 1) / 2, 0, (GIT_OID_HEXSZ - len) / 2);
-
-		/* If len < GIT_OID_HEXSZ (a strict short oid was given), we have
-		 * 2 options :
-		 * - We always search in the cache first. If we find that short oid is
-		 *	ambiguous, we can stop. But in all the other cases, we must then
-		 *	explore all the backends (to find an object if there was match,
-		 *	or to check that oid is not ambiguous if we have found 1 match in
-		 *	the cache)
-		 * - We never explore the cache, go right to exploring the backends
-		 * We chose the latter : we explore directly the backends.
+
+		/* If len < GIT_OID_HEXSZ (short oid), we have 2 options:
+		 *
+		 * - We always search in the cache first. If we find that short
+		 *	 oid is ambiguous, we can stop. But in all the other cases, we
+		 *	 must then explore all the backends (to find an object if
+		 *	 there was match, or to check that oid is not ambiguous if we
+		 *	 have found 1 match in the cache)
+		 *
+		 * - We never explore the cache, go right to exploring the
+		 *   backends We chose the latter : we explore directly the
+		 *   backends.
 		 */
 		error = git_odb_read_prefix(&odb_obj, odb, &short_oid, len);
 	}
 
-	if (error < 0)
-		return error;
-
-	error = git_object__from_odb_object(object_out, repo, odb_obj, type);
+	if (!error) {
+		error = git_object__from_odb_object(out, repo, odb_obj, type, lax);
 
-	git_odb_object_free(odb_obj);
+		git_odb_object_free(odb_obj);
+	}
 
 	return error;
 }
 
-int git_object_lookup(git_object **object_out, git_repository *repo, const git_oid *id, git_otype type) {
-	return git_object_lookup_prefix(object_out, repo, id, GIT_OID_HEXSZ, type);
+int git_object_lookup(
+	git_object **out,
+	git_repository *repo,
+	const git_oid *id,
+	git_otype type)
+{
+	return object_lookup(out, repo, id, GIT_OID_HEXSZ, type, false);
+}
+
+int git_object_lookup_prefix(
+	git_object **out,
+	git_repository *repo,
+	const git_oid *id,
+	size_t len,
+	git_otype type)
+{
+	return object_lookup(out, repo, id, len, type, false);
+}
+
+int git_object_lookup_lax(
+	git_object **out,
+	git_repository *repo,
+	const git_oid *id,
+	size_t len,
+	git_otype type)
+{
+	return object_lookup(out, repo, id, len, type, true);
 }
 
 void git_object_free(git_object *object)
 {
 	if (object == NULL)
 		return;
-
 	git_cached_obj_decref(object);
 }
 
@@ -235,16 +267,21 @@ const char *git_object_type2string(git_otype type)
 	return git_objects_table[type].str;
 }
 
-git_otype git_object_string2type(const char *str)
+git_otype git_object_string2type(const char *str, size_t len)
 {
 	size_t i;
 
 	if (!str || !*str)
 		return GIT_OBJ_BAD;
+	if (!len)
+		len = strlen(str);
+
+	for (i = 0; i < ARRAY_SIZE(git_objects_table); i++) {
+		size_t typelen = strlen(git_objects_table[i].str);
 
-	for (i = 0; i < ARRAY_SIZE(git_objects_table); i++)
-		if (!strcmp(str, git_objects_table[i].str))
+		if (len >= typelen && !memcmp(str, git_objects_table[i].str, len))
 			return (git_otype)i;
+	}
 
 	return GIT_OBJ_BAD;
 }
@@ -364,28 +401,25 @@ int git_object_dup(git_object **dest, git_object *source)
 }
 
 int git_object_lookup_bypath(
-		git_object **out,
-		const git_object *treeish,
-		const char *path,
-		git_otype type)
+	git_object **out,
+	const git_object *treeish,
+	const char *path,
+	git_otype type)
 {
-	int error = -1;
-	git_tree *tree = NULL;
+	int error = 0;
+	git_object *tree = NULL;
 	git_tree_entry *entry = NULL;
 
 	assert(out && treeish && path);
 
-	if ((error = git_object_peel((git_object**)&tree, treeish, GIT_OBJ_TREE)) < 0 ||
-		 (error = git_tree_entry_bypath(&entry, tree, path)) < 0)
-	{
+	if ((error = git_object_peel(&tree, treeish, GIT_OBJ_TREE)) < 0 ||
+		(error = git_tree_entry_bypath(&entry, (git_tree *)tree, path)) < 0)
 		goto cleanup;
-	}
 
-	if (type != GIT_OBJ_ANY && git_tree_entry_type(entry) != type)
-	{
-		giterr_set(GITERR_OBJECT,
-				"object at path '%s' is not of the asked-for type %d",
-				path, type);
+	if (type != GIT_OBJ_ANY && git_tree_entry_type(entry) != type) {
+		giterr_set(
+			GITERR_OBJECT, "object at path '%s' is not a %s (%d)",
+			path, git_object_type2string(type), type);
 		error = GIT_EINVALIDSPEC;
 		goto cleanup;
 	}
@@ -394,7 +428,8 @@ int git_object_lookup_bypath(
 
 cleanup:
 	git_tree_entry_free(entry);
-	git_tree_free(tree);
+	git_object_free(tree);
+
 	return error;
 }
 
@@ -440,3 +475,170 @@ int git_object_short_id(git_buf *out, const git_object *obj)
 	return error;
 }
 
+static int object_parse_error(
+	git_otype otype, git_object_parse_t *item, const char *msg)
+{
+	const char *typestr = git_object_type2string(otype);
+
+	if (item->tag)
+		giterr_set(GITERR_OBJECT, "Failed to parse %s - %s '%s'",
+			typestr, msg, item->tag);
+	else
+		giterr_set(GITERR_OBJECT, "Failed to parse %s - %s", typestr, msg);
+
+	return -1;
+}
+
+static int object_parse_line(
+	git_otype otype,
+	git_object_parse_t *item,
+	const char *buf,
+	const char *eol,
+	int error)
+{
+	size_t len;
+	const char *msg = NULL;
+
+	buf += item->taglen + 1;
+
+	if (eol <= buf) {
+		msg = "insufficient data for";
+		goto done;
+	} else
+		len = (size_t)(eol - buf);
+
+	switch (item->type) {
+	case GIT_PARSE_OID:
+	case GIT_PARSE_OID_ARRAY: {
+		git_oid *id = (item->type == GIT_PARSE_OID) ?
+			item->value.id : git_array_alloc(*item->value.ids);
+
+		if (!id)
+			msg = "out of memory";
+		else if (len < GIT_OID_HEXSZ)
+			msg = "insufficient data for";
+		else if (git_oid_fromstr(id, buf) < 0)
+			msg = "invalid OID in";
+		else if (len > GIT_OID_HEXSZ + 1)
+			msg = "extra data after";
+		else if (buf[GIT_OID_HEXSZ] != '\n')
+			msg = "improper termination for";
+		break;
+	}
+	case GIT_PARSE_OTYPE:
+		if ((*item->value.otype = git_object_string2type(buf, len)) ==
+			GIT_OBJ_BAD)
+			msg = "invalid value for";
+		break;
+	case GIT_PARSE_SIGNATURE:
+		*item->value.sig = git__calloc(1, sizeof(git_signature));
+		if (!*item->value.sig)
+			msg = "out of memory";
+		else if (git_signature__parse(
+				*item->value.sig, &buf, eol + 1, NULL, '\n') < 0)
+			msg = "invalid signature for";
+		break;
+	case GIT_PARSE_TO_EOL:
+		if (eol[-1] == '\r')
+			--len;
+		if ((*item->value.text = git__strndup(buf, len)) == NULL)
+			msg = "out of memory";
+		break;
+	default:
+		msg = "unexpected parse type";
+		break;
+	}
+
+done:
+	if (msg && !error)
+		error = object_parse_error(otype, item, msg);
+	return error;
+}
+
+int git_object__parse_lines(
+	git_otype otype,
+	git_object_parse_t *parse,
+	const char *buf,
+	const char *buf_end)
+{
+	int error = 0;
+	bool optional = false;
+	char *eol;
+	git_object_parse_t *scan = parse, *next = parse + 1;
+	size_t len;
+
+	/* process required and optional lines */
+	for (; buf < buf_end && scan->type > GIT_PARSE_BODY; scan = (next++)) {
+		len = buf_end - buf;
+
+		if (scan->type == GIT_PARSE_MODE_OPTIONAL) {
+			optional = true;
+			continue;
+		}
+
+		if (git__iseol(buf, buf_end - buf))
+			goto body;
+
+		if ((eol = memchr(buf, '\n', buf_end - buf)) == NULL) {
+			if (!error)
+				error = object_parse_error(otype, scan, "unterminated line");
+			break;
+		}
+		len = (size_t)(eol - buf);
+
+		if (len > scan->taglen &&
+			!memcmp(scan->tag, buf, scan->taglen) &&
+			buf[scan->taglen] == ' ')
+		{
+			error = object_parse_line(otype, scan, buf, eol, error);
+
+			if (scan->type == GIT_PARSE_OID_ARRAY) /* don't advance yet */
+				next = scan;
+		}
+		else if (optional)
+			/* for now, skip this tag - eventually search tags? */
+			next = scan;
+		else if (scan->type == GIT_PARSE_OID_ARRAY)
+			continue;
+		else if (!error)
+			error = object_parse_error(
+				otype, scan, "missing required field");
+
+		buf = eol + 1; /* advance to next line */
+	}
+
+body:
+
+	if (scan->type > GIT_PARSE_BODY) {
+		if (!optional && !error)
+			error = object_parse_error
+				(otype, scan, "missing required field");
+
+		while (scan->type > GIT_PARSE_BODY)
+			scan++;
+	}
+
+	if (scan->type > GIT_PARSE_BODY)
+		return error;
+
+	while (buf < buf_end && !git__iseol(buf, buf_end - buf)) {
+		if ((eol = memchr(buf, '\n', buf_end - buf)) == NULL)
+			buf = buf_end;
+		else
+			buf = eol + 1;
+	}
+
+	if (buf < buf_end)
+		buf += (*buf == '\n') ? 1 : 2;
+	else {
+		buf = buf_end;
+
+		if (!error && scan->type != GIT_PARSE_BODY_OPTIONAL)
+			error = object_parse_error(otype, scan, "missing message body");
+	}
+
+	if (scan->value.body)
+		*scan->value.body = buf;
+
+	return error;
+}
diff --git a/src/object.h b/src/object.h
index d187c55b7..47f82f36d 100644
--- a/src/object.h
+++ b/src/object.h
@@ -7,6 +7,9 @@
 #ifndef INCLUDE_object_h__
 #define INCLUDE_object_h__
 
+#include "common.h"
+#include "array.h"
+
 /** Base git object for inheritance */
 struct git_object {
 	git_cached_obj cached;
@@ -17,15 +20,49 @@ struct git_object {
 void git_object__free(void *object);
 
 int git_object__from_odb_object(
-	git_object **object_out,
+	git_object **out,
 	git_repository *repo,
 	git_odb_object *odb_obj,
-	git_otype type);
+	git_otype type,
+	bool lax);
 
 int git_object__resolve_to_type(git_object **obj, git_otype type);
 
-int git_oid__parse(git_oid *oid, const char **buffer_out, const char *buffer_end, const char *header);
-
 void git_oid__writebuf(git_buf *buf, const char *header, const git_oid *oid);
 
+enum {
+	GIT_PARSE_BODY_OPTIONAL = -2,
+	GIT_PARSE_BODY = -1,
+	GIT_PARSE_MODE_OPTIONAL = 0,
+	GIT_PARSE_OID = 1,
+	GIT_PARSE_OID_ARRAY = 2,
+	GIT_PARSE_OTYPE = 3,
+	GIT_PARSE_SIGNATURE = 4,
+	GIT_PARSE_TO_EOL = 5,
+};
+
+typedef git_array_t(git_oid) git_oid_array;
+
+typedef struct {
+	const char *tag;
+	size_t taglen;
+	int type;
+	union {
+		git_oid *id;
+		git_otype *otype;
+		char **text;
+		git_signature **sig;
+		git_oid_array *ids;
+		const char **body;
+	} value;
+} git_object_parse_t;
+
+/* parse tagged lines followed by blank line and message body */
+int git_object__parse_lines(
+	git_otype type,
+	git_object_parse_t *parse,
+	const char *buf,
+	const char *buf_end);
+
 #endif
+
diff --git a/src/odb.c b/src/odb.c
index 20a3f6c6e..6e8ce3d1d 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -778,7 +778,7 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id)
 	}
 
 	if (error && error != GIT_PASSTHROUGH) {
-		if (!reads)
+		if (!reads || error == GIT_ENOTFOUND)
 			return git_odb__error_notfound("no match for id", id);
 		return error;
 	}
diff --git a/src/odb_loose.c b/src/odb_loose.c
index b2e8bed4d..4e23a9629 100644
--- a/src/odb_loose.c
+++ b/src/odb_loose.c
@@ -129,7 +129,7 @@ static size_t get_object_header(obj_hdr *hdr, unsigned char *data)
 	typename[used] = 0;
 	if (used == 0)
 		return 0;
-	hdr->type = git_object_string2type(typename);
+	hdr->type = git_object_string2type(typename, used);
 	used++; /* consume the space */
 
 	/*
diff --git a/src/oid.c b/src/oid.c
index b640cadd1..be4d857cf 100644
--- a/src/oid.c
+++ b/src/oid.c
@@ -122,32 +122,6 @@ char *git_oid_tostr(char *out, size_t n, const git_oid *oid)
 	return out;
 }
 
-int git_oid__parse(
-	git_oid *oid, const char **buffer_out,
-	const char *buffer_end, const char *header)
-{
-	const size_t sha_len = GIT_OID_HEXSZ;
-	const size_t header_len = strlen(header);
-
-	const char *buffer = *buffer_out;
-
-	if (buffer + (header_len + sha_len + 1) > buffer_end)
-		return -1;
-
-	if (memcmp(buffer, header, header_len) != 0)
-		return -1;
-
-	if (buffer[header_len + sha_len] != '\n')
-		return -1;
-
-	if (git_oid_fromstr(oid, buffer + header_len) < 0)
-		return -1;
-
-	*buffer_out = buffer + (header_len + sha_len + 1);
-
-	return 0;
-}
-
 void git_oid__writebuf(git_buf *buf, const char *header, const git_oid *oid)
 {
 	char hex_oid[GIT_OID_HEXSZ];
diff --git a/src/signature.c b/src/signature.c
index 2545b7519..f6d50c8ba 100644
--- a/src/signature.c
+++ b/src/signature.c
@@ -11,7 +11,7 @@
 #include "git2/common.h"
 #include "posix.h"
 
-void git_signature_free(git_signature *sig)
+void git_signature__clear(git_signature *sig)
 {
 	if (sig == NULL)
 		return;
@@ -20,6 +20,11 @@ void git_signature_free(git_signature *sig)
 	sig->name = NULL;
 	git__free(sig->email);
 	sig->email = NULL;
+}
+
+void git_signature_free(git_signature *sig)
+{
+	git_signature__clear(sig);
 	git__free(sig);
 }
 
@@ -176,22 +181,39 @@ int git_signature__parse(git_signature *sig, const char **buffer_out,
 	}
 
 	email_start = git__memrchr(buffer, '<', buffer_end - buffer);
-	email_end = git__memrchr(buffer, '>', buffer_end - buffer);
+	if (!email_start) {
+		/* just stop now with everything as name */
+		sig->name = extract_trimmed(buffer, buffer_end - buffer);
+		sig->email = git__strdup("");
+		*buffer_out = buffer_end + 1;
+		return signature_error("missing e-mail");
+	}
 
-	if (!email_start || !email_end || email_end <= email_start)
+	sig->name = extract_trimmed(buffer, email_start - buffer);
+	email_start += 1;
+
+	email_end = git__memrchr(email_start, '>', buffer_end - email_start);
+	if (!email_end) {
+		sig->email = extract_trimmed(email_start, buffer_end - email_start);
 		return signature_error("malformed e-mail");
+	}
 
-	email_start += 1;
-	sig->name = extract_trimmed(buffer, email_start - buffer - 1);
 	sig->email = extract_trimmed(email_start, email_end - email_start);
 
 	/* Do we even have a time at the end of the signature? */
-	if (email_end + 2 < buffer_end) {
+	if (email_end != NULL && email_end + 2 < buffer_end) {
 		const char *time_start = email_end + 2;
 		const char *time_end;
 
-		if (git__strtol64(&sig->when.time, time_start, &time_end, 10) < 0)
-			return signature_error("invalid Unix timestamp");
+		if (git__strtol64(&sig->when.time, time_start, &time_end, 10) < 0) {
+			/* set timestamp to max value */
+			sig->when.time = (uint64_t)-1L;
+
+			/* skip over invalid timestamp data */
+			time_end = time_start;
+			while (git__isspace(*time_end)) ++time_end;
+			while (*time_end && !git__isspace(*time_end)) ++time_end;
+		}
 
 		/* do we have a timezone? */
 		if (time_end + 1 < buffer_end) {
@@ -202,7 +224,7 @@ int git_signature__parse(git_signature *sig, const char **buffer_out,
 
 			if ((tz_start[0] != '-' && tz_start[0] != '+') ||
 				git__strtol32(&offset, tz_start + 1, &tz_end, 10) < 0) {
-				//malformed timezone, just assume it's zero
+				/* malformed timezone, just assume it's zero */
 				offset = 0;
 			}
 
diff --git a/src/signature.h b/src/signature.h
index 24655cbf5..b2cfe2c6e 100644
--- a/src/signature.h
+++ b/src/signature.h
@@ -12,6 +12,7 @@
 #include "repository.h"
 #include <time.h>
 
+void git_signature__clear(git_signature *sig);
 int git_signature__parse(git_signature *sig, const char **buffer_out, const char *buffer_end, const char *header, char ender);
 void git_signature__writebuf(git_buf *buf, const char *header, const git_signature *sig);
 
diff --git a/src/tag.c b/src/tag.c
index d7b531d34..438a03b6f 100644
--- a/src/tag.c
+++ b/src/tag.c
@@ -59,104 +59,33 @@ const char *git_tag_message(const git_tag *t)
 	return t->message;
 }
 
-static int tag_error(const char *str)
+static int tag_parse(git_tag *tag, const char *buf, const char *buf_end)
 {
-	giterr_set(GITERR_TAG, "Failed to parse tag. %s", str);
-	return -1;
-}
-
-static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end)
-{
-	static const char *tag_types[] = {
-		NULL, "commit\n", "tree\n", "blob\n", "tag\n"
+	int error = 0;
+	const char *body = NULL;
+	git_object_parse_t parser[] = {
+		{ "object", 6, GIT_PARSE_OID, { .id = &tag->target } },
+		{ "type", 4, GIT_PARSE_OTYPE, { .otype = &tag->type } },
+		{ NULL, 0, GIT_PARSE_MODE_OPTIONAL },
+		{ "tag", 3, GIT_PARSE_TO_EOL, { .text = &tag->tag_name } },
+		{ "tagger", 6, GIT_PARSE_SIGNATURE, { .sig = &tag->tagger } },
+		{ NULL, 0, GIT_PARSE_BODY_OPTIONAL, { .body = &body } },
 	};
 
-	unsigned int i;
-	size_t text_len;
-	char *search;
-
-	if (git_oid__parse(&tag->target, &buffer, buffer_end, "object ") < 0)
-		return tag_error("Object field invalid");
-
-	if (buffer + 5 >= buffer_end)
-		return tag_error("Object too short");
-
-	if (memcmp(buffer, "type ", 5) != 0)
-		return tag_error("Type field not found");
-	buffer += 5;
-
-	tag->type = GIT_OBJ_BAD;
-
-	for (i = 1; i < ARRAY_SIZE(tag_types); ++i) {
-		size_t type_length = strlen(tag_types[i]);
-
-		if (buffer + type_length >= buffer_end)
-			return tag_error("Object too short");
-
-		if (memcmp(buffer, tag_types[i], type_length) == 0) {
-			tag->type = i;
-			buffer += type_length;
-			break;
-		}
-	}
-
-	if (tag->type == GIT_OBJ_BAD)
-		return tag_error("Invalid object type");
-
-	if (buffer + 4 >= buffer_end)
-		return tag_error("Object too short");
-
-	if (memcmp(buffer, "tag ", 4) != 0)
-		return tag_error("Tag field not found");
-
-	buffer += 4;
-
-	search = memchr(buffer, '\n', buffer_end - buffer);
-	if (search == NULL)
-		return tag_error("Object too short");
-
-	text_len = search - buffer;
-
-	tag->tag_name = git__malloc(text_len + 1);
-	GITERR_CHECK_ALLOC(tag->tag_name);
-
-	memcpy(tag->tag_name, buffer, text_len);
-	tag->tag_name[text_len] = '\0';
-
-	buffer = search + 1;
+	error = git_object__parse_lines(GIT_OBJ_TAG, parser, buf, buf_end);
 
-	tag->tagger = NULL;
-	if (buffer < buffer_end && *buffer != '\n') {
-		tag->tagger = git__malloc(sizeof(git_signature));
-		GITERR_CHECK_ALLOC(tag->tagger);
-
-		if (git_signature__parse(tag->tagger, &buffer, buffer_end, "tagger ", '\n') < 0)
-			return -1;
-	}
-
-	tag->message = NULL;
-	if (buffer < buffer_end) {
-		if( *buffer != '\n' )
-			return tag_error("No new line before message");
-
-		text_len = buffer_end - ++buffer;
-
-		tag->message = git__malloc(text_len + 1);
+	if (body != NULL && body < buf_end) {
+		tag->message = git__strndup(body, buf_end - body);
 		GITERR_CHECK_ALLOC(tag->message);
-
-		memcpy(tag->message, buffer, text_len);
-		tag->message[text_len] = '\0';
 	}
 
-	return 0;
+	return error;
 }
 
-int git_tag__parse(void *_tag, git_odb_object *odb_obj)
+int git_tag__parse(void *tag, git_odb_object *odb_obj)
 {
-	git_tag *tag = _tag;
 	const char *buffer = git_odb_object_data(odb_obj);
 	const char *buffer_end = buffer + git_odb_object_size(odb_obj);
-
 	return tag_parse(tag, buffer, buffer_end);
 }
 
@@ -196,12 +125,12 @@ static int retrieve_tag_reference_oid(
 }
 
 static int write_tag_annotation(
-		git_oid *oid,
-		git_repository *repo,
-		const char *tag_name,
-		const git_object *target,
-		const git_signature *tagger,
-		const char *message)
+	git_oid *oid,
+	git_repository *repo,
+	const char *tag_name,
+	const git_object *target,
+	const git_signature *tagger,
+	const char *message)
 {
 	git_buf tag = GIT_BUF_INIT;
 	git_odb *odb;
@@ -231,14 +160,14 @@ on_error:
 }
 
 static int git_tag_create__internal(
-		git_oid *oid,
-		git_repository *repo,
-		const char *tag_name,
-		const git_object *target,
-		const git_signature *tagger,
-		const char *message,
-		int allow_ref_overwrite,
-		int create_tag_annotation)
+	git_oid *oid,
+	git_repository *repo,
+	const char *tag_name,
+	const git_object *target,
+	const git_signature *tagger,
+	const char *message,
+	int allow_ref_overwrite,
+	int create_tag_annotation)
 {
 	git_reference *new_ref = NULL;
 	git_buf ref_name = GIT_BUF_INIT;
@@ -320,77 +249,70 @@ int git_tag_create_frombuffer(git_oid *oid, git_repository *repo, const char *bu
 	int error;
 	git_odb *odb;
 	git_odb_stream *stream;
-	git_odb_object *target_obj;
-
-	git_reference *new_ref = NULL;
+	git_odb_object *target_obj = NULL;
 	git_buf ref_name = GIT_BUF_INIT;
+	size_t buflen;
 
 	assert(oid && buffer);
 
 	memset(&tag, 0, sizeof(tag));
-
-	if (git_repository_odb__weakptr(&odb, repo) < 0)
-		return -1;
+	buflen = strlen(buffer);
 
 	/* validate the buffer */
-	if (tag_parse(&tag, buffer, buffer + strlen(buffer)) < 0)
-		return -1;
+	if ((error = tag_parse(&tag, buffer, buffer + buflen)) < 0)
+		goto cleanup;
 
 	/* validate the target */
-	if (git_odb_read(&target_obj, odb, &tag.target) < 0)
-		goto on_error;
+	if ((error = git_repository_odb__weakptr(&odb, repo)) < 0 ||
+		(error = git_odb_read(&target_obj, odb, &tag.target)) < 0)
+		goto cleanup;
 
 	if (tag.type != target_obj->cached.type) {
 		giterr_set(GITERR_TAG, "The type for the given target is invalid");
-		goto on_error;
+		error = -1;
+		goto cleanup;
 	}
 
 	error = retrieve_tag_reference_oid(oid, &ref_name, repo, tag.tag_name);
 	if (error < 0 && error != GIT_ENOTFOUND)
-		goto on_error;
-
-	/* We don't need these objects after this */
-	git_signature_free(tag.tagger);
-	git__free(tag.tag_name);
-	git__free(tag.message);
-	git_odb_object_free(target_obj);
+		goto cleanup;
 
 	/** Ensure the tag name doesn't conflict with an already existing
 	 *	reference unless overwriting has explictly been requested **/
-	if (error == 0 && !allow_ref_overwrite) {
+	if (!error && !allow_ref_overwrite) {
 		giterr_set(GITERR_TAG, "Tag already exists");
-		return GIT_EEXISTS;
+		error = GIT_EEXISTS;
+		goto cleanup;
 	}
 
 	/* write the buffer */
-	if ((error = git_odb_open_wstream(
-			&stream, odb, strlen(buffer), GIT_OBJ_TAG)) < 0)
-		return error;
+	if (!(error = git_odb_open_wstream(&stream, odb, buflen, GIT_OBJ_TAG))) {
 
-	if (!(error = git_odb_stream_write(stream, buffer, strlen(buffer))))
-		error = git_odb_stream_finalize_write(oid, stream);
+		if (!(error = git_odb_stream_write(stream, buffer, buflen)))
+			error = git_odb_stream_finalize_write(oid, stream);
 
-	git_odb_stream_free(stream);
-
-	if (error < 0) {
-		git_buf_free(&ref_name);
-		return error;
+		git_odb_stream_free(stream);
 	}
 
-	error = git_reference_create(
-		&new_ref, repo, ref_name.ptr, oid, allow_ref_overwrite, NULL, NULL);
+	/* update the reference */
+	if (!error) {
+		git_reference *new_ref = NULL;
 
-	git_reference_free(new_ref);
-	git_buf_free(&ref_name);
+		error = git_reference_create(
+			&new_ref, repo, ref_name.ptr, oid,
+			allow_ref_overwrite, NULL, NULL);
 
-	return error;
+		git_reference_free(new_ref);
+	}
 
-on_error:
+cleanup:
 	git_signature_free(tag.tagger);
 	git__free(tag.tag_name);
 	git__free(tag.message);
 	git_odb_object_free(target_obj);
-	return -1;
+	git_buf_free(&ref_name);
+
+	return error;
 }
 
 int git_tag_delete(git_repository *repo, const char *tag_name)
@@ -403,11 +325,10 @@ int git_tag_delete(git_repository *repo, const char *tag_name)
 
 	git_buf_free(&ref_name);
 
-	if (error < 0)
-		return error;
+	if (!error)
+		error = git_reference_delete(tag_ref);
 
-	if ((error = git_reference_delete(tag_ref)) == 0)
-		git_reference_free(tag_ref);
+	git_reference_free(tag_ref);
 
 	return error;
 }
diff --git a/src/util.h b/src/util.h
index 6fb2dc0f4..8d300987f 100644
--- a/src/util.h
+++ b/src/util.h
@@ -322,6 +322,12 @@ GIT_INLINE(bool) git__iswildcard(int c)
 	return (c == '*' || c == '?' || c == '[');
 }
 
+GIT_INLINE(bool) git__iseol(const char *ptr, size_t len)
+{
+	char c = *ptr;
+	return (c == '\n' || (c == '\r' && len > 1 && *(ptr + 1) == '\n'));
+}
+
 /*
  * Parse a string value as a boolean, just like Core Git does.
  *