commit: expose the different kinds of errors

We should be checking whether the object we're looking up is a commit,
and we should let the caller know whether the not-found return code
comes from a bad object type or just a missing signature.

3 files changed, 24 insertions, 1 deletions

diff --git a/include/git2/commit.h b/include/git2/commit.h
index a92277417..3488c7440 100644
--- a/include/git2/commit.h
+++ b/include/git2/commit.h
@@ -266,12 +266,18 @@ GIT_EXTERN(int) git_commit_header_field(git_buf *out, const git_commit *commit,
 /**
  * Extract the signature from a commit
  *
+ * If the id is not for a commit, the error class will be
+ * `GITERR_INVALID`. If the commit does not have a signature, the
+ * error class will be `GITERR_OBJECT`.
+ *
  * @param signature the signature block
  * @param signed_data signed data; this is the commit contents minus the signature block
  * @param repo the repository in which the commit exists
  * @param commit_id the commit from which to extract the data
  * @param field the name of the header field containing the signature
  * block; pass `NULL` to extract the default 'gpgsig'
+ * @return 0 on success, GIT_ENOTFOUND if the id is not for a commit
+ * or the commit does not have a signature.
  */
 GIT_EXTERN(int) git_commit_extract_signature(git_buf *signature, git_buf *signed_data, git_repository *repo, git_oid *commit_id, const char *field);
 
diff --git a/src/commit.c b/src/commit.c
index 8faef07df..5a0509803 100644
--- a/src/commit.c
+++ b/src/commit.c
@@ -642,6 +642,12 @@ int git_commit_extract_signature(git_buf *signature, git_buf *signed_data, git_r
 	if ((error = git_odb_read(&obj, odb, commit_id)) < 0)
 		return error;
 
+	if (obj->cached.type != GIT_OBJ_COMMIT) {
+		giterr_set(GITERR_INVALID, "the requested type does not match the type in ODB");
+		error = GIT_ENOTFOUND;
+		goto cleanup;
+	}
+
 	buf = git_odb_object_data(obj);
 
 	while ((h = strchr(buf, '\n')) && h[1] != '\0' && h[1] != '\n') {
@@ -688,7 +694,7 @@ int git_commit_extract_signature(git_buf *signature, git_buf *signed_data, git_r
 		return git_buf_puts(signed_data, eol+1);
 	}
 
-	giterr_set(GITERR_INVALID, "this commit is not signed");
+	giterr_set(GITERR_OBJECT, "this commit is not signed");
 	error = GIT_ENOTFOUND;
 	goto cleanup;
 
diff --git a/tests/commit/parse.c b/tests/commit/parse.c
index 3e1670ec4..838cfb467 100644
--- a/tests/commit/parse.c
+++ b/tests/commit/parse.c
@@ -513,6 +513,17 @@ a simple commit which works\n";
 	cl_assert_equal_s(gpgsig, signature.ptr);
 	cl_assert_equal_s(data, signed_data.ptr);
 
+	/* Try to parse a tree */
+	cl_git_pass(git_oid_fromstr(&commit_id, "45dd856fdd4d89b884c340ba0e047752d9b085d6"));
+	cl_git_fail_with(GIT_ENOTFOUND, git_commit_extract_signature(&signature, &signed_data, g_repo, &commit_id, NULL));
+	cl_assert_equal_i(GITERR_INVALID, giterr_last()->klass);
+
+	/* Try to parse an unsigned commit */
+	cl_git_pass(git_odb_write(&commit_id, odb, passing_commit_cases[1], strlen(passing_commit_cases[1]), GIT_OBJ_COMMIT));
+	cl_git_fail_with(GIT_ENOTFOUND, git_commit_extract_signature(&signature, &signed_data, g_repo, &commit_id, NULL));
+	cl_assert_equal_i(GITERR_OBJECT, giterr_last()->klass);
+
 	git_buf_free(&signature);
 	git_buf_free(&signed_data);
+
 }