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.

diff --git a/include/git2/commit.h b/include/git2/commit.h
index a922774175a0d8b8944841d420db3c5b9a2d625f..3488c7440e330fcac34139417fcabe0d2851d0a8 100644 (file)
--- 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 8faef07dfe97f828ebc18113f956acc194b342fc..5a05098035b980028f956f7e676a4fe1e1d510e8 100644 (file)
--- 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 3e1670ec4c9fe5a4104b09559224d25bc026e84d..838cfb46742166998fcee07ba5ac3a597eda0384 100644 (file)
--- 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);
+
 }