object: return GIT_EINVALID on parse errors

Return `GIT_EINVALID` on parse errors so that direct callers of parse
functions can determine when there was a failure to parse the object.

The object parser functions will swallow this error code to prevent it
from propagating down the chain to end-users.  (`git_merge` should not
return `GIT_EINVALID` when a commit it tries to look up is not valid,
this would be too vague to be useful.)

The only public function that this affects is
`git_signature_from_buffer`, which is now documented as returning
`GIT_EINVALID` when appropriate.

Author: ethomson

include/git2/signature.h

@@ -71,7 +71,7 @@ GIT_EXTERN(int) git_signature_default(git_signature **out, git_repository *repo)
  *
  * @param out new signature
  * @param buf signature string
- * @return 0 on success, or an error code
+ * @return 0 on success, GIT_EINVALID if the signature is not parseable, or an error code
  */
 GIT_EXTERN(int) git_signature_from_buffer(git_signature **out, const char *buf);
 

src/commit.c

@@ -395,6 +395,7 @@ static int commit_parse(git_commit *commit, const char *data, size_t size, unsig
 	git_oid parent_id;
 	size_t header_len;
 	git_signature dummy_sig;
+	int error;
 
 	GIT_ASSERT_ARG(commit);
 	GIT_ASSERT_ARG(data);
@@ -431,14 +432,14 @@ static int commit_parse(git_commit *commit, const char *data, size_t size, unsig
 		commit->author = git__malloc(sizeof(git_signature));
 		GIT_ERROR_CHECK_ALLOC(commit->author);
 
-		if (git_signature__parse(commit->author, &buffer, buffer_end, "author ", '\n') < 0)
-			return -1;
+		if ((error = git_signature__parse(commit->author, &buffer, buffer_end, "author ", '\n')) < 0)
+			return error;
 	}
 
 	/* Some tools create multiple author fields, ignore the extra ones */
 	while (!git__prefixncmp(buffer, buffer_end - buffer, "author ")) {
-		if (git_signature__parse(&dummy_sig, &buffer, buffer_end, "author ", '\n') < 0)
-			return -1;
+		if ((error = git_signature__parse(&dummy_sig, &buffer, buffer_end, "author ", '\n')) < 0)
+			return error;
 
 		git__free(dummy_sig.name);
 		git__free(dummy_sig.email);
@@ -448,8 +449,8 @@ static int commit_parse(git_commit *commit, const char *data, size_t size, unsig
 	commit->committer = git__malloc(sizeof(git_signature));
 	GIT_ERROR_CHECK_ALLOC(commit->committer);
 
-	if (git_signature__parse(commit->committer, &buffer, buffer_end, "committer ", '\n') < 0)
-		return -1;
+	if ((error = git_signature__parse(commit->committer, &buffer, buffer_end, "committer ", '\n')) < 0)
+		return error;
 
 	if (flags & GIT_COMMIT_PARSE_QUICK)
 		return 0;
@@ -493,7 +494,7 @@ static int commit_parse(git_commit *commit, const char *data, size_t size, unsig
 
 bad_buffer:
 	git_error_set(GIT_ERROR_OBJECT, "failed to parse bad commit object");
-	return -1;
+	return GIT_EINVALID;
 }
 
 int git_commit__parse_raw(void *commit, const char *data, size_t size)
@@ -971,8 +972,10 @@ int git_commit_create_with_signature(
 	/* The first step is to verify that all the tree and parents exist */
 	parsed = git__calloc(1, sizeof(git_commit));
 	GIT_ERROR_CHECK_ALLOC(parsed);
-	if ((error = commit_parse(parsed, commit_content, strlen(commit_content), 0)) < 0)
+	if (commit_parse(parsed, commit_content, strlen(commit_content), 0) < 0) {
+		error = -1;
 		goto cleanup;
+	}
 
 	if ((error = validate_tree_and_parents(&parents, repo, &parsed->tree_id, commit_parent_from_commit, parsed, NULL, true)) < 0)
 		goto cleanup;

src/commit_list.c

@@ -124,16 +124,15 @@ static int commit_quick_parse(
 {
 	git_oid *parent_oid;
 	git_commit *commit;
-	int error;
 	size_t i;
 
 	commit = git__calloc(1, sizeof(*commit));
 	GIT_ERROR_CHECK_ALLOC(commit);
 	commit->object.repo = walk->repo;
 
-	if ((error = git_commit__parse_ext(commit, obj, GIT_COMMIT_PARSE_QUICK)) < 0) {
+	if (git_commit__parse_ext(commit, obj, GIT_COMMIT_PARSE_QUICK) < 0) {
 		git__free(commit);
-		return error;
+		return -1;
 	}
 
 	if (!git__is_uint16(git_array_size(commit->parent_ids))) {

src/indexer.c

@@ -340,16 +340,22 @@ static int check_object_connectivity(git_indexer *idx, const git_rawobj *obj)
 {
 	git_object *object;
 	git_oid *expected;
-	int error;
+	int error = 0;
 
 	if (obj->type != GIT_OBJECT_BLOB &&
 	    obj->type != GIT_OBJECT_TREE &&
 	    obj->type != GIT_OBJECT_COMMIT &&
 	    obj->type != GIT_OBJECT_TAG)
 		return 0;
 
-	if ((error = git_object__from_raw(&object, obj->data, obj->len, obj->type)) < 0)
+	if (git_object__from_raw(&object, obj->data, obj->len, obj->type) < 0) {
+		/*
+		 * parse_raw returns EINVALID on invalid data; downgrade
+		 * that to a normal -1 error code.
+		 */
+		error = -1;
 		goto out;
+	}
 
 	if ((expected = git_oidmap_get(idx->expected_oids, &object->cached.oid)) != NULL) {
 		git_oidmap_delete(idx->expected_oids, &object->cached.oid);

src/object.c

@@ -144,12 +144,17 @@ int git_object__from_odb_object(
 	def = &git_objects_table[odb_obj->cached.type];
 	GIT_ASSERT(def->free && def->parse);
 
-	if ((error = def->parse(object, odb_obj)) < 0)
+	if ((error = def->parse(object, odb_obj)) < 0) {
+		/*
+		 * parse returns EINVALID on invalid data; downgrade
+		 * that to a normal -1 error code.
+		 */
 		def->free(object);
-	else
-		*object_out = git_cache_store_parsed(&repo->objects, object);
+		return -1;
+	}
 
-	return error;
+	*object_out = git_cache_store_parsed(&repo->objects, object);
+	return 0;
 }
 
 void git_object__free(void *obj)

src/signature.c

@@ -23,6 +23,12 @@ void git_signature_free(git_signature *sig)
 	git__free(sig);
 }
 
+static int signature_parse_error(const char *msg)
+{
+	git_error_set(GIT_ERROR_INVALID, "failed to parse signature - %s", msg);
+	return GIT_EINVALID;
+}
+
 static int signature_error(const char *msg)
 {
 	git_error_set(GIT_ERROR_INVALID, "failed to parse signature - %s", msg);
@@ -206,13 +212,13 @@ int git_signature__parse(git_signature *sig, const char **buffer_out,
 
 	if (ender &&
 		(buffer_end = memchr(buffer, ender, buffer_end - buffer)) == NULL)
-		return signature_error("no newline given");
+		return signature_parse_error("no newline given");
 
 	if (header) {
 		const size_t header_len = strlen(header);
 
 		if (buffer + header_len >= buffer_end || memcmp(buffer, header, header_len) != 0)
-			return signature_error("expected prefix doesn't match actual");
+			return signature_parse_error("expected prefix doesn't match actual");
 
 		buffer += header_len;
 	}
@@ -221,7 +227,7 @@ int git_signature__parse(git_signature *sig, const char **buffer_out,
 	email_end = git__memrchr(buffer, '>', buffer_end - buffer);
 
 	if (!email_start || !email_end || email_end <= email_start)
-		return signature_error("malformed e-mail");
+		return signature_parse_error("malformed e-mail");
 
 	email_start += 1;
 	sig->name = extract_trimmed(buffer, email_start - buffer - 1);
@@ -237,7 +243,7 @@ int git_signature__parse(git_signature *sig, const char **buffer_out,
 			git__free(sig->name);
 			git__free(sig->email);
 			sig->name = sig->email = NULL;
-			return signature_error("invalid Unix timestamp");
+			return signature_parse_error("invalid Unix timestamp");
 		}
 
 		/* do we have a timezone? */

src/tag.c

@@ -62,7 +62,7 @@ const char *git_tag_message(const git_tag *t)
 static int tag_error(const char *str)
 {
 	git_error_set(GIT_ERROR_TAG, "failed to parse tag: %s", str);
-	return -1;
+	return GIT_EINVALID;
 }
 
 static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end)
@@ -73,6 +73,7 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end)
 	size_t text_len, alloc_len;
 	const char *search;
 	unsigned int i;
+	int error;
 
 	if (git_oid__parse(&tag->target, &buffer, buffer_end, "object ") < 0)
 		return tag_error("object field invalid");
@@ -130,8 +131,8 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end)
 		tag->tagger = git__malloc(sizeof(git_signature));
 		GIT_ERROR_CHECK_ALLOC(tag->tagger);
 
-		if (git_signature__parse(tag->tagger, &buffer, buffer_end, "tagger ", '\n') < 0)
-			return -1;
+		if ((error = git_signature__parse(tag->tagger, &buffer, buffer_end, "tagger ", '\n')) < 0)
+			return error;
 	}
 
 	tag->message = NULL;

src/tree.c

@@ -351,15 +351,26 @@ size_t git_treebuilder_entrycount(git_treebuilder *bld)
 	return git_strmap_size(bld->map);
 }
 
-static int tree_error(const char *str, const char *path)
+GIT_INLINE(void) set_error(const char *str, const char *path)
 {
 	if (path)
 		git_error_set(GIT_ERROR_TREE, "%s - %s", str, path);
 	else
 		git_error_set(GIT_ERROR_TREE, "%s", str);
+}
+
+static int tree_error(const char *str, const char *path)
+{
+	set_error(str, path);
 	return -1;
 }
 
+static int tree_parse_error(const char *str, const char *path)
+{
+	set_error(str, path);
+	return GIT_EINVALID;
+}
+
 static int parse_mode(uint16_t *mode_out, const char *buffer, size_t buffer_len, const char **buffer_out)
 {
 	int32_t mode;
@@ -399,19 +410,19 @@ int git_tree__parse_raw(void *_tree, const char *data, size_t size)
 		uint16_t attr;
 
 		if (parse_mode(&attr, buffer, buffer_end - buffer, &buffer) < 0 || !buffer)
-			return tree_error("failed to parse tree: can't parse filemode", NULL);
+			return tree_parse_error("failed to parse tree: can't parse filemode", NULL);
 
 		if (buffer >= buffer_end || (*buffer++) != ' ')
-			return tree_error("failed to parse tree: missing space after filemode", NULL);
+			return tree_parse_error("failed to parse tree: missing space after filemode", NULL);
 
 		if ((nul = memchr(buffer, 0, buffer_end - buffer)) == NULL)
-			return tree_error("failed to parse tree: object is corrupted", NULL);
+			return tree_parse_error("failed to parse tree: object is corrupted", NULL);
 
 		if ((filename_len = nul - buffer) == 0 || filename_len > UINT16_MAX)
-			return tree_error("failed to parse tree: can't parse filename", NULL);
+			return tree_parse_error("failed to parse tree: can't parse filename", NULL);
 
 		if ((buffer_end - (nul + 1)) < GIT_OID_RAWSZ)
-			return tree_error("failed to parse tree: can't parse OID", NULL);
+			return tree_parse_error("failed to parse tree: can't parse OID", NULL);
 
 		/* Allocate the entry */
 		{
@@ -434,16 +445,15 @@ int git_tree__parse_raw(void *_tree, const char *data, size_t size)
 int git_tree__parse(void *_tree, git_odb_object *odb_obj)
 {
 	git_tree *tree = _tree;
+	const char *data = git_odb_object_data(odb_obj);
+	size_t size = git_odb_object_size(odb_obj);
+	int error;
 
-	if ((git_tree__parse_raw(tree,
-	    git_odb_object_data(odb_obj),
-	    git_odb_object_size(odb_obj))) < 0)
-		return -1;
-
-	if (git_odb_object_dup(&tree->odb_obj, odb_obj) < 0)
-		return -1;
+	if ((error = git_tree__parse_raw(tree, data, size)) < 0 ||
+	    (error = git_odb_object_dup(&tree->odb_obj, odb_obj)) < 0)
+		return error;
 
-	return 0;
+	return error;
 }
 
 static size_t find_next_dir(const char *dirname, git_index *index, size_t start)