Merge pull request #119 from sueun-dev/fix-digestset-ambiguous-crossalg

digestset: detect ambiguous short codes across algorithms

Author: thaJeztah

digestset/set.go

@@ -109,17 +109,30 @@ func (dst *Set) Lookup(d string) (digest.Digest, error) {
 		}
 	}
 	idx := sort.Search(len(dst.entries), searchFunc)
-	if idx == len(dst.entries) || !checkShortMatch(dst.entries[idx].alg, dst.entries[idx].val, string(alg), hex) {
-		return "", ErrDigestNotFound
-	}
-	if dst.entries[idx].alg == alg && dst.entries[idx].val == hex {
-		return dst.entries[idx].digest, nil
+
+	// Entries whose value has hex as a prefix form a contiguous run starting
+	// at idx. Digests of a different algorithm may sort within that run, so a
+	// second matching entry is not necessarily adjacent to the first; scan the
+	// whole run instead of only inspecting idx and idx+1.
+	var match *digestEntry
+	for i := idx; i < len(dst.entries) && strings.HasPrefix(dst.entries[i].val, hex); i++ {
+		if !checkShortMatch(dst.entries[i].alg, dst.entries[i].val, string(alg), hex) {
+			continue
+		}
+		if dst.entries[i].alg == alg && dst.entries[i].val == hex {
+			// An exact whole-value match is unambiguous.
+			return dst.entries[i].digest, nil
+		}
+		if match != nil {
+			return "", ErrDigestAmbiguous
+		}
+		match = dst.entries[i]
 	}
-	if idx+1 < len(dst.entries) && checkShortMatch(dst.entries[idx+1].alg, dst.entries[idx+1].val, string(alg), hex) {
-		return "", ErrDigestAmbiguous
+	if match == nil {
+		return "", ErrDigestNotFound
 	}
 
-	return dst.entries[idx].digest, nil
+	return match.digest, nil
 }
 
 // Add adds the given digest to the set. An error will be returned

digestset/set_test.go

@@ -20,6 +20,7 @@ import (
 	"crypto/sha256"
 	"encoding/binary"
 	"math/rand"
+	"strings"
 	"testing"
 
 	"github.com/opencontainers/go-digest"
@@ -391,3 +392,31 @@ func BenchmarkShortCode100(b *testing.B) {
 func BenchmarkShortCode1000(b *testing.B) {
 	benchShortCodeNTable(b, 1000, 12)
 }
+
+// TestLookupAmbiguousAcrossAlgorithms verifies that an algorithm-scoped short
+// code is reported as ambiguous even when a digest of a different algorithm
+// sorts between the two matching entries. The two sha256 digests below share
+// the prefix "1234", so "sha256:1234" is ambiguous (see TestLookup). The sha512
+// digest's value also starts with "1234" and sorts lexically between them.
+func TestLookupAmbiguousAcrossAlgorithms(t *testing.T) {
+	sha256A := digest.Digest("sha256:1234" + strings.Repeat("0", 60))
+	sha256C := digest.Digest("sha256:1234" + strings.Repeat("1", 60))
+	sha512B := digest.Digest("sha512:12340" + strings.Repeat("5", 123))
+
+	dset := NewSet()
+	for _, d := range []digest.Digest{sha256A, sha512B, sha256C} {
+		if err := dset.Add(d); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	if _, err := dset.Lookup("sha256:1234"); err != ErrDigestAmbiguous {
+		t.Fatalf("Lookup(%q) = %v; want ErrDigestAmbiguous", "sha256:1234", err)
+	}
+
+	// A different-algorithm entry that sorts first in the prefix run must not
+	// hide the matching entries either.
+	if _, err := dset.Lookup("sha512:12340"); err != nil {
+		t.Fatalf("Lookup(%q) = %v; want the sha512 digest", "sha512:12340", err)
+	}
+}