Fixed bug caused by incorrect comparison of 'filled-up' binary shas, as it would compare even the filling, instead of taking the canonical length into account and hence ignore the last 4 bytes if the original hex sha had an odd length

Author: Byron

db/base.py

@@ -287,7 +287,8 @@ def partial_to_complete_sha_hex(self, partial_hexsha):
 		databases = list()
 		_databases_recursive(self, databases)
 
-		if len(partial_hexsha) % 2 != 0:
+		len_partial_hexsha = len(partial_hexsha)
+		if len_partial_hexsha % 2 != 0:
 			partial_binsha = hex_to_bin(partial_hexsha + "0")
 		else:
 			partial_binsha = hex_to_bin(partial_hexsha)
@@ -300,7 +301,7 @@ def partial_to_complete_sha_hex(self, partial_hexsha):
 				if hasattr(db, 'partial_to_complete_sha_hex'):
 					full_bin_sha = db.partial_to_complete_sha_hex(partial_hexsha)
 				else:
-					full_bin_sha = db.partial_to_complete_sha(partial_binsha)
+					full_bin_sha = db.partial_to_complete_sha(partial_binsha, len_partial_hexsha)
 				# END handle database type
 			except BadObject:
 				continue

db/pack.py

@@ -177,13 +177,17 @@ def entities(self):
 		""":return: list of pack entities operated upon by this database"""
 		return [ item[1] for item in self._entities ]
 
-	def partial_to_complete_sha(self, partial_binsha):
+	def partial_to_complete_sha(self, partial_binsha, canonical_length):
 		""":return: 20 byte sha as inferred by the given partial binary sha
+		:param partial_binsha: binary sha with less than 20 bytes 
+		:param canonical_length: length of the corresponding canonical representation.
+			It is required as binary sha's cannot display whether the original hex sha
+			had an odd or even number of characters
 		:raise AmbiguousObjectName: 
 		:raise BadObject: """
 		candidate = None
 		for item in self._entities:
-			item_index = item[1].index().partial_sha_to_index(partial_binsha)
+			item_index = item[1].index().partial_sha_to_index(partial_binsha, canonical_length)
 			if item_index is not None:
 				sha = item[1].index().sha(item_index)
 				if candidate and candidate != sha:

fun.py

@@ -40,7 +40,8 @@
 chunk_size = 1000*mmap.PAGESIZE
 
 __all__ = ('is_loose_object', 'loose_object_header_info', 'msb_size', 'pack_object_header_info', 
-			'write_object', 'loose_object_header', 'stream_copy', 'apply_delta_data' )
+			'write_object', 'loose_object_header', 'stream_copy', 'apply_delta_data', 
+			'is_equal_canonical_sha' )
 
 #{ Routines
 
@@ -223,5 +224,23 @@ def apply_delta_data(src_buf, src_buf_size, delta_buf, delta_buf_size, target_fi
 	# yes, lets use the exact same error message that git uses :)
 	assert i == delta_buf_size, "delta replay has gone wild"
 
+	
+def is_equal_canonical_sha(canonical_length, match, sha1):
+	"""
+	:return: True if the given lhs and rhs 20 byte binary shas
+		The comparison will take the canonical_length of the match sha into account, 
+		hence the comparison will only use the last 4 bytes for uneven canonical representations
+	:param match: less than 20 byte sha
+	:param sha1: 20 byte sha"""
+	binary_length = canonical_length/2
+	if match[:binary_length] != sha1[:binary_length]:
+		return False
+		
+	if canonical_length - binary_length and \
+		(ord(match[-1]) ^ ord(sha1[len(match)-1])) & 0xf0:
+		return False
+	# END handle uneven canonnical length
+	return True
+	
 #} END routines
 

pack.py

@@ -12,6 +12,7 @@
 
 from fun import (
 					pack_object_header_info,
+					is_equal_canonical_sha,
 					type_id_to_type_map,
 					write_object,
 					stream_copy, 
@@ -303,24 +304,26 @@ def sha_to_index(self, sha):
 		# END bisect
 		return None
 
-	def partial_sha_to_index(self, partial_sha):
-		""":return: index as in `sha_to_index` or None if the sha was not found in this
-		index file
-		:param partial_sha: an at least two bytes of a partial sha
+	def partial_sha_to_index(self, partial_bin_sha, canonical_length):
+		"""
+		:return: index as in `sha_to_index` or None if the sha was not found in this
+			index file
+		:param partial_bin_sha: an at least two bytes of a partial binary sha
+		:param canonical_length: lenght of the original hexadecimal representation of the 
+			given partial binary sha
 		:raise AmbiguousObjectName:"""
-		if len(partial_sha) < 2:
+		if len(partial_bin_sha) < 2:
 			raise ValueError("Require at least 2 bytes of partial sha")
 
-		first_byte = ord(partial_sha[0])
+		first_byte = ord(partial_bin_sha[0])
 		get_sha = self.sha
 		lo = 0					# lower index, the left bound of the bisection
 		if first_byte != 0:
 			lo = self._fanout_table[first_byte-1]
 		hi = self._fanout_table[first_byte]		# the upper, right bound of the bisection
 
-		len_partial = len(partial_sha)
 		# fill the partial to full 20 bytes
-		filled_sha = partial_sha + '\0'*(20 - len_partial)
+		filled_sha = partial_bin_sha + '\0'*(20 - len(partial_bin_sha))
 
 		# find lowest 
 		while lo < hi:
@@ -336,14 +339,15 @@ def partial_sha_to_index(self, partial_sha):
 				lo = mid + 1
 			# END handle midpoint
 		# END bisect
-		if lo < self.size:
+		
+		if lo < self.size():
 			cur_sha = get_sha(lo)
-			if cur_sha[:len_partial] == partial_sha:
+			if is_equal_canonical_sha(canonical_length, partial_bin_sha, cur_sha):
 				next_sha = None
-				if lo+1 < self.size:
+				if lo+1 < self.size():
 					next_sha = get_sha(lo+1)
 				if next_sha and next_sha == cur_sha:
-					raise AmbiguousObjectName(partial_sha)
+					raise AmbiguousObjectName(partial_bin_sha)
 				return lo
 			# END if we have a match
 		# END if we found something

test/db/test_pack.py

@@ -55,7 +55,7 @@ def test_writing(self, path):
 		for i, sha in enumerate(sha_list):
 			short_sha = sha[:max((i % max_bytes), min_bytes)]
 			try:
-				assert pdb.partial_to_complete_sha(short_sha) == sha
+				assert pdb.partial_to_complete_sha(short_sha, len(short_sha)*2) == sha
 			except AmbiguousObjectName:
 				num_ambiguous += 1
 				pass # valid, we can have short objects
@@ -67,4 +67,4 @@ def test_writing(self, path):
 		# assert num_ambiguous
 
 		# non-existing
-		self.failUnlessRaises(BadObject, pdb.partial_to_complete_sha, "\0\0")
+		self.failUnlessRaises(BadObject, pdb.partial_to_complete_sha, "\0\0", 4)

test/test_pack.py

@@ -60,10 +60,10 @@ def _assert_index_file(self, index, version, size):
 
 			# verify partial sha
 			for l in (4,8,11,17,20):
-				assert index.partial_sha_to_index(sha[:l]) == oidx
+				assert index.partial_sha_to_index(sha[:l], l*2) == oidx
 
 		# END for each object index in indexfile
-		self.failUnlessRaises(ValueError, index.partial_sha_to_index, "\0")
+		self.failUnlessRaises(ValueError, index.partial_sha_to_index, "\0", 2)
 
 
 	def _assert_pack_file(self, pack, version, size):