odb: Handle corner cases in `git_odb_expand_ids`

The old implementation had two issues:

1. OIDs that were too short as to be ambiguous were not being handled
properly.

2. If the last OID to expand in the array was missing from the ODB, we
would leak a `GIT_ENOTFOUND` error code from the function.

2 files changed, 36 insertions, 22 deletions

diff --git a/src/odb.c b/src/odb.c
index 90336c082..9c35fd0f9 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -748,54 +748,59 @@ int git_odb_expand_ids(
 	size_t count)
 {
 	size_t len, i;
-	int error;
 
 	assert(db && ids);
 
 	for (i = 0; i < count; i++) {
 		git_odb_expand_id *query = &ids[i];
-		git_oid *actual_id = NULL, tmp;
+		git_oid actual_id;
 		git_otype query_type = (query->type == GIT_OBJ_ANY) ? 0 : query->type;
 		git_otype actual_type = 0;
+		int error = GIT_EAMBIGUOUS;
 
 		/* if we were given a full object ID, simply look it up */
 		if (query->length >= GIT_OID_HEXSZ) {
 			error = git_odb_read_header(&len, &actual_type, db, &query->id);
+			git_oid_cpy(&actual_id, &query->id);
 		}
 
-		/* otherwise, resolve the short id to full, then (optionally)
-		 * read the header.
+		/*
+		 * otherwise, resolve the short id to full if it's long enough, then
+		 * (optionally) read the header
 		 */
 		else if (query->length >= GIT_OID_MINPREFIXLEN) {
-	    	error = odb_exists_prefix_1(&tmp,
-				db, &query->id, query->length, false);
-
-			if (!error) {
-				actual_id = &tmp;
-				error = git_odb_read_header(&len, &actual_type, db, &tmp);
-			}
+			error = odb_exists_prefix_1(&actual_id, db, &query->id, query->length, false);
+			if (!error)
+				error = git_odb_read_header(&len, &actual_type, db, &actual_id);
 		}
 
-		if (error < 0 && error != GIT_ENOTFOUND && error != GIT_EAMBIGUOUS)
-			break;
-
-		if (error == 0 && (query_type == actual_type || !query_type)) {
-			if (actual_id)
-				git_oid_cpy(&query->id, actual_id);
+		/* Ensure that the looked up type matches the type we were expecting */
+		if (query_type && query_type != actual_type)
+			error = GIT_ENOTFOUND;
 
+		switch (error) {
+		case 0:
+			git_oid_cpy(&query->id, &actual_id);
 			query->length = GIT_OID_HEXSZ;
 			query->type = actual_type;
-		} else {
+			break;
+
+		/* the object is missing or ambiguous */
+		case GIT_ENOTFOUND:
+		case GIT_EAMBIGUOUS:
 			memset(&query->id, 0, sizeof(git_oid));
 			query->length = 0;
 			query->type = 0;
+			break;
+
+		/* something went very wrong with the ODB; bail hard */
+		default:
+			return error;
 		}
 	}
 
-	if (!error)
-		giterr_clear();
-
-	return error;
+	giterr_clear();
+	return 0;
 }
 
 int git_odb_read_header(size_t *len_p, git_otype *type_p, git_odb *db, const git_oid *id)
diff --git a/tests/odb/mixed.c b/tests/odb/mixed.c
index 0e728b8fe..515eadfde 100644
--- a/tests/odb/mixed.c
+++ b/tests/odb/mixed.c
@@ -130,6 +130,9 @@ struct expand_id_test_data expand_id_test_data[] = {
 	{ "0ddeaded9", "0ddeaded9502971eefe1e41e34d0e536853ae20f", GIT_OBJ_BLOB },
 	{ "f00b4e",    NULL, GIT_OBJ_ANY },
 
+	/* this OID is too short and should be ambiguous! */
+	{ "f00",    NULL, GIT_OBJ_ANY },
+
 	/* some full-length object ids */
 	{ "0000000000000000000000000000000000000000", NULL, GIT_OBJ_ANY },
 	{
@@ -143,6 +146,12 @@ struct expand_id_test_data expand_id_test_data[] = {
 	  "4d5979b468252190cb572ae758aca36928e8a91e",
 	  GIT_OBJ_TREE
 	},
+
+	 /*
+	  * ensure we're not leaking the return error code for the
+	  * last lookup if the last object is invalid
+	  */
+	{ "0ddeadedfff",  NULL, GIT_OBJ_ANY },
 };
 
 static void setup_prefix_query(