Fix version parsing, validate debian package version

This patch adds validation to the version pasing logic (parseVersion) to
not allow packages to be registered which apt wouldn't accept anyway.

It then propogates validation errors through CompareVersions, the way
in which parseVersion gets called, so that appropriate decisions can be
made.

New packages with invalid versions are not accepted.

For any pre-exisiting packages with invalid versions, as much care is
taken as possible to log warnings, and not use those packages.

This PR is loosely bnased on one by by Aaron Foster
<afoster@cloudflare.com> though it has largely been written at this
point.

Signed off with internal and external emails for clarity, but commit
is under personal email where we typically do open source contributions
from.

Signed-off-by: Phil Dibowitz <pdibowitz@cloudflare.com>
Signed-off-by: Phil Dibowitz <pdibowitz@ipom.com>

AUTHORS

@@ -85,3 +85,4 @@ List of contributors, in chronological order:
 * Tom Nguyen (https://github.com/lecafard)
 * Philip Cramer (https://github.com/PhilipCramer)
 * Kerem Gur (https://github.com/KereMath)
+* Phil Dibowitz (https://github.com/jaymzh)

deb/import.go

@@ -124,6 +124,12 @@ func ImportPackageFiles(list *PackageList, packageFiles []string, forceReplace b
 			continue
 		}
 
+		if !isValidVersion(p.Version) {
+			reporter.Warning("Version number ('%s') for the '%s' package is invalid", p.Version, p.Name)
+			failedFiles = append(failedFiles, file)
+			continue
+		}
+
 		if p.Architecture == "" {
 			reporter.Warning("Empty architecture on %s", file)
 			failedFiles = append(failedFiles, file)

deb/import_test.go

@@ -0,0 +1,47 @@
+package deb
+
+import (
+	"github.com/aptly-dev/aptly/aptly"
+	"github.com/aptly-dev/aptly/database"
+	"github.com/aptly-dev/aptly/database/goleveldb"
+
+	. "gopkg.in/check.v1"
+)
+
+type ImportSuite struct {
+	db                database.Storage
+	packageCollection *PackageCollection
+	reporter          *aptly.RecordingResultReporter
+}
+
+var _ = Suite(&ImportSuite{})
+
+func (s *ImportSuite) SetUpTest(c *C) {
+	s.db, _ = goleveldb.NewOpenDB(c.MkDir())
+	s.packageCollection = NewPackageCollection(s.db)
+	s.reporter = &aptly.RecordingResultReporter{
+		Warnings:     []string{},
+		AddedLines:   []string{},
+		RemovedLines: []string{},
+	}
+}
+
+func (s *ImportSuite) TearDownTest(c *C) {
+	_ = s.db.Close()
+}
+
+func (s *ImportSuite) TestImportPackageFilesRejectsInvalidVersion(c *C) {
+	list := NewPackageList()
+
+	processedFiles, failedFiles, err := ImportPackageFiles(
+		list, []string{"testdata/import/invalid-version.dsc"}, false, &NullVerifier{},
+		nil, s.packageCollection, s.reporter, nil,
+		func(database.ReaderWriter) aptly.ChecksumStorage { return nil })
+
+	c.Assert(err, IsNil)
+	c.Check(processedFiles, HasLen, 0)
+	c.Check(failedFiles, DeepEquals, []string{"testdata/import/invalid-version.dsc"})
+	c.Check(s.reporter.Warnings, DeepEquals, []string{
+		"Version number ('1.2.3-') for the 'aptly-test-invalid-version' package is invalid",
+	})
+}

deb/list.go

@@ -184,8 +184,21 @@ func (l *PackageList) FilterLatest() (*PackageList, error) {
 	err := l.ForEach(func(p *Package) error {
 		key := p.Architecture + "|" + p.Name
 
-		if existing, found := filtered[key]; !found || CompareVersions(p.Version, existing.Version) > 0 {
+		existing, found := filtered[key]
+		if !found {
 			filtered[key] = p
+			return nil
+		}
+
+		cmp, compareErr := CompareVersions(p.Version, existing.Version)
+		switch compareErr {
+		case nil:
+			if cmp > 0 {
+				filtered[key] = p
+			}
+		case RightInvalid:
+			filtered[key] = p
+		case LeftInvalid, BothInvalid:
 		}
 
 		return nil
@@ -427,7 +440,13 @@ func (l *PackageList) Swap(i, j int) {
 
 func (l *PackageList) lessPackages(iPkg, jPkg *Package) bool {
 	if iPkg.Name == jPkg.Name {
-		cmp := CompareVersions(iPkg.Version, jPkg.Version)
+		cmp, err := CompareVersions(iPkg.Version, jPkg.Version)
+		switch err {
+		case LeftInvalid, BothInvalid:
+			return false
+		case RightInvalid:
+			return true
+		}
 		if cmp == 0 {
 			return iPkg.Architecture < jPkg.Architecture
 		}

deb/list_test.go

@@ -531,6 +531,36 @@ func (s *PackageListSuite) TestFilterLatest(c *C) {
 	c.Check(filtered.Has(sharedPkg), Equals, true)
 }
 
+func (s *PackageListSuite) TestFilterLatestInvalidVersions(c *C) {
+	list := NewPackageList()
+	valid := &Package{Name: "pkg", Version: "1.0", Architecture: "amd64"}
+	invalid := &Package{Name: "pkg", Version: "2.0-", Architecture: "amd64"}
+	_ = list.Add(valid)
+	_ = list.Add(invalid)
+
+	filtered, err := list.FilterLatest()
+	c.Assert(err, IsNil)
+	c.Check(filtered.Len(), Equals, 1)
+	c.Check(filtered.Has(valid), Equals, true)
+
+	invalidOnly := NewPackageList()
+	_ = invalidOnly.Add(&Package{Name: "pkg", Version: "1.0-", Architecture: "amd64"})
+	_ = invalidOnly.Add(&Package{Name: "pkg", Version: "2.0-", Architecture: "amd64"})
+	filtered, err = invalidOnly.FilterLatest()
+	c.Assert(err, IsNil)
+	c.Check(filtered.Len(), Equals, 1)
+}
+
+func (s *PackageListSuite) TestLessPackagesInvalidVersions(c *C) {
+	list := NewPackageList()
+	valid := &Package{Name: "pkg", Version: "1.0", Architecture: "amd64"}
+	invalid := &Package{Name: "pkg", Version: "2.0-", Architecture: "amd64"}
+
+	c.Check(list.lessPackages(valid, invalid), Equals, true)
+	c.Check(list.lessPackages(invalid, valid), Equals, false)
+	c.Check(list.lessPackages(invalid, invalid), Equals, false)
+}
+
 func (s *PackageListSuite) TestFilterLatestPreservesDuplicatesFlag(c *C) {
 	list := NewPackageListWithDuplicates(true, 2)
 

deb/package.go

@@ -405,19 +405,19 @@ func (p *Package) MatchesDependency(dep Dependency) bool {
 }
 
 func versionSatisfiesDependency(version string, dep Dependency) bool {
-	r := CompareVersions(version, dep.Version)
+	r, err := CompareVersions(version, dep.Version)
 
 	switch dep.Relation {
 	case VersionEqual:
-		return r == 0
+		return err == nil && r == 0
 	case VersionLess:
-		return r < 0
+		return err == nil && r < 0
 	case VersionGreater:
-		return r > 0
+		return err == nil && r > 0
 	case VersionLessOrEqual:
-		return r <= 0
+		return err == nil && r <= 0
 	case VersionGreaterOrEqual:
-		return r >= 0
+		return err == nil && r >= 0
 	case VersionPatternMatch:
 		matched, err := filepath.Match(dep.Version, version)
 		return err == nil && matched

deb/query.go

@@ -168,15 +168,20 @@ func (q *FieldQuery) Matches(pkg PackageLike) bool {
 	case VersionDontCare:
 		return field != ""
 	case VersionEqual:
-		return CompareVersions(field, q.Value) == 0
+		result, err := CompareVersions(field, q.Value)
+		return err == nil && result == 0
 	case VersionGreater:
-		return CompareVersions(field, q.Value) > 0
+		result, err := CompareVersions(field, q.Value)
+		return err == nil && result > 0
 	case VersionGreaterOrEqual:
-		return CompareVersions(field, q.Value) >= 0
+		result, err := CompareVersions(field, q.Value)
+		return err == nil && result >= 0
 	case VersionLess:
-		return CompareVersions(field, q.Value) < 0
+		result, err := CompareVersions(field, q.Value)
+		return err == nil && result < 0
 	case VersionLessOrEqual:
-		return CompareVersions(field, q.Value) <= 0
+		result, err := CompareVersions(field, q.Value)
+		return err == nil && result <= 0
 	case VersionPatternMatch:
 		matched, err := filepath.Match(q.Value, field)
 		return err == nil && matched

deb/query_test.go

@@ -21,3 +21,43 @@ func (s *QuerySuite) TestVersionCompare(c *C) {
 	c.Check(q.Matches(&p100), Equals, false)
 	c.Check(q.Matches(&p1), Equals, true)
 }
+
+func (s *QuerySuite) TestVersionCompareGreater(c *C) {
+	q := FieldQuery{"Version", VersionGreater, "5.0.0.2", nil}
+
+	p100 := Package{}
+	p100.Version = "5.0.0.100"
+
+	p1 := Package{}
+	p1.Version = "5.0.0.1"
+
+	c.Check(q.Matches(&p100), Equals, true)
+	c.Check(q.Matches(&p1), Equals, false)
+
+	// invalid version on either side of the comparison must not match
+	pInvalid := Package{}
+	pInvalid.Version = "1.2.3-"
+	c.Check(q.Matches(&pInvalid), Equals, false)
+}
+
+func (s *QuerySuite) TestVersionCompareGreaterOrEqual(c *C) {
+	q := FieldQuery{"Version", VersionGreaterOrEqual, "5.0.0.2", nil}
+
+	p100 := Package{}
+	p100.Version = "5.0.0.100"
+
+	pEqual := Package{}
+	pEqual.Version = "5.0.0.2"
+
+	p1 := Package{}
+	p1.Version = "5.0.0.1"
+
+	c.Check(q.Matches(&p100), Equals, true)
+	c.Check(q.Matches(&pEqual), Equals, true)
+	c.Check(q.Matches(&p1), Equals, false)
+
+	// invalid version on either side of the comparison must not match
+	pInvalid := Package{}
+	pInvalid.Version = "1.2.3-"
+	c.Check(q.Matches(&pInvalid), Equals, false)
+}

deb/reflist.go

@@ -365,32 +365,36 @@ func (l *PackageRefList) Merge(r *PackageRefList, overrideMatching, ignoreConfli
 func (l *PackageRefList) FilterLatestRefs() {
 	var (
 		lastArch, lastName, lastVer []byte
-		arch, name, ver             []byte
-		parts                       [][]byte
 	)
 
-	for i := 0; i < len(l.Refs); i++ {
-		parts = bytes.Split(l.Refs[i][1:], []byte(" "))
-		arch, name, ver = parts[0], parts[1], parts[2]
+	filtered := l.Refs[:0]
+	for _, ref := range l.Refs {
+		parts := bytes.Split(ref[1:], []byte(" "))
+		arch, name, ver := parts[0], parts[1], parts[2]
 
 		if bytes.Equal(arch, lastArch) && bytes.Equal(name, lastName) {
-			// Two packages are identical, check version and only one wins
-			vres := CompareVersions(string(ver), string(lastVer))
-
-			// Remove the older refs from the result
-			if vres > 0 {
-				// ver[i] > ver[i-1], remove element i-1
-				l.Refs = append(l.Refs[:i-1], l.Refs[i:]...)
-			} else {
-				// ver[i] < ver[i-1], remove element i
-				l.Refs = append(l.Refs[:i], l.Refs[i+1:]...)
-				arch, name, ver = lastArch, lastName, lastVer
+			// The previous ref is valid, so this also validates the current ref.
+			vres, err := CompareVersions(string(ver), string(lastVer))
+			if err != nil {
+				continue
 			}
 
-			// Compensate for the reduced set
-			i--
+			// Replace the older ref in the result.
+			if vres > 0 {
+				filtered[len(filtered)-1] = ref
+				lastArch, lastName, lastVer = arch, name, ver
+			}
+			continue
 		}
 
+		// Comparing a version with itself validates singleton refs.
+		if _, err := CompareVersions(string(ver), string(ver)); err != nil {
+			continue
+		}
+
+		filtered = append(filtered, ref)
 		lastArch, lastName, lastVer = arch, name, ver
 	}
+
+	l.Refs = filtered
 }

deb/reflist_test.go

@@ -398,3 +398,17 @@ func (s *PackageRefListSuite) TestFilterLatestRefs(c *C) {
 	c.Check(toStrSlice(result), DeepEquals,
 		[]string{"Pi386 dpkg 1.6", "Pi386 lib 1.2"})
 }
+
+func (s *PackageRefListSuite) TestFilterLatestRefsRemovesInvalidVersions(c *C) {
+	result := &PackageRefList{Refs: [][]byte{
+		[]byte("Pi386 app 1.0-"),
+		[]byte("Pi386 dpkg 1.0"),
+		[]byte("Pi386 dpkg 2.0-"),
+		[]byte("Pi386 lib 1.0-"),
+		[]byte("Pi386 lib 2.0-"),
+	}}
+
+	result.FilterLatestRefs()
+
+	c.Check(toStrSlice(result), DeepEquals, []string{"Pi386 dpkg 1.0"})
+}

deb/testdata/import/invalid-version.dsc

@@ -0,0 +1,7 @@
+Format: 1.0
+Source: aptly-test-invalid-version
+Binary: aptly-test-invalid-version
+Architecture: any
+Version: 1.2.3-
+Maintainer: Aptly Test <test@example.com>
+Standards-Version: 3.9.3

deb/version.go

@@ -6,38 +6,86 @@ import (
 	"strconv"
 	"strings"
 	"unicode"
+
+	"github.com/rs/zerolog/log"
+)
+
+var (
+	upstreamVersionRegex = regexp.MustCompile(`^[0-9][A-Za-z0-9.+~\-]*$`)
+	debianRevisionRegex  = regexp.MustCompile(`^[A-Za-z0-9.+~]*$`)
+)
+
+// VersionCompareError identifies an invalid operand passed to CompareVersions.
+type VersionCompareError string
+
+// Error implements the error interface.
+func (e VersionCompareError) Error() string {
+	return string(e)
+}
+
+// CompareVersions errors.
+const (
+	LeftInvalid  VersionCompareError = "left version is invalid"
+	RightInvalid VersionCompareError = "right version is invalid"
+	BothInvalid  VersionCompareError = "both versions are invalid"
 )
 
 // Using documentation from: http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Version
 
 // CompareVersions compares two package versions
-func CompareVersions(ver1, ver2 string) int {
-	e1, u1, d1 := parseVersion(ver1)
-	e2, u2, d2 := parseVersion(ver2)
+func CompareVersions(ver1, ver2 string) (int, error) {
+	e1, u1, d1, err1 := parseVersion(ver1)
+	e2, u2, d2, err2 := parseVersion(ver2)
+
+	if err1 != nil {
+		log.Warn().Err(err1).Str("version", ver1).Msg("Invalid package version")
+	}
+	if err2 != nil && (err1 == nil || ver1 != ver2) {
+		log.Warn().Err(err2).Str("version", ver2).Msg("Invalid package version")
+	}
+
+	if err1 != nil && err2 != nil {
+		return 0, BothInvalid
+	}
+	if err1 != nil {
+		return 0, LeftInvalid
+	}
+	if err2 != nil {
+		return 0, RightInvalid
+	}
 
 	r := compareVersionPart(e1, e2)
 	if r != 0 {
-		return r
+		return r, nil
 	}
 
 	r = compareVersionPart(u1, u2)
 	if r != 0 {
-		return r
+		return r, nil
 	}
 
-	return compareVersionPart(d1, d2)
+	return compareVersionPart(d1, d2), nil
 }
 
 // parseVersions breaks down full version to components (possibly empty)
-func parseVersion(ver string) (epoch, upstream, debian string) {
+func parseVersion(ver string) (epoch, upstream, debian string, err error) {
 	i := strings.Index(ver, ":")
 	if i != -1 {
 		epoch, ver = ver[:i], ver[i+1:]
 	}
 
-	i = strings.Index(ver, "-")
+	// Debian policy specifies that the upstream_version and debian_revision
+	// are separated by the LAST hyphen in the string, since upstream_version
+	// itself may legitimately contain hyphens.
+	i = strings.LastIndex(ver, "-")
 	if i != -1 {
 		debian, ver = ver[i+1:], ver[:i]
+		if debian == "" {
+			// if a hyphen is detected in the upstream version
+			// string without a debian revision following it, the
+			// version is invalid
+			return "", "", "", fmt.Errorf("could not parse version: version string ('%s-') includes hyphen without Debian revision", ver)
+		}
 	}
 
 	upstream = ver
@@ -45,6 +93,42 @@ func parseVersion(ver string) (epoch, upstream, debian string) {
 	return
 }
 
+// isValidVersion checks whether package version is compliant with control field spec
+// source: https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-version
+func isValidVersion(ver string) bool {
+	epoch, upstream, deb, err := parseVersion(ver)
+	if err != nil {
+		return false
+	}
+
+	// validate epoch component
+	if epoch != "" {
+		// uint64 for unexpectedly high epoch values
+		_, err := strconv.ParseUint(epoch, 10, 64)
+		if err != nil {
+			return false
+		}
+	}
+
+	if upstream == "" || !isValidUpstreamVersion(upstream) {
+		return false
+	}
+
+	if deb != "" && !isValidDebianRevision(deb) {
+		return false
+	}
+
+	return true
+}
+
+func isValidUpstreamVersion(upstream string) bool {
+	return upstreamVersionRegex.MatchString(upstream)
+}
+
+func isValidDebianRevision(debian string) bool {
+	return debianRevisionRegex.MatchString(debian)
+}
+
 // compareLexicographic compares in "Debian lexicographic" way, see below compareVersionPart for details
 func compareLexicographic(s1, s2 string) int {
 	i := 0

deb/version_test.go

@@ -10,20 +10,74 @@ type VersionSuite struct {
 var _ = Suite(&VersionSuite{})
 
 func (s *VersionSuite) TestParseVersion(c *C) {
-	e, u, d := parseVersion("1.3.4")
+	e, u, d, err := parseVersion("1.3.4")
 	c.Check([]string{e, u, d}, DeepEquals, []string{"", "1.3.4", ""})
+	c.Check(err, Equals, nil)
 
-	e, u, d = parseVersion("4:1.3:4")
+	e, u, d, err = parseVersion("4:1.3:4")
 	c.Check([]string{e, u, d}, DeepEquals, []string{"4", "1.3:4", ""})
+	c.Check(err, Equals, nil)
 
-	e, u, d = parseVersion("1.3.4-1")
+	e, u, d, err = parseVersion("1.3.4-1")
 	c.Check([]string{e, u, d}, DeepEquals, []string{"", "1.3.4", "1"})
+	c.Check(err, Equals, nil)
 
-	e, u, d = parseVersion("1.3-pre4-1")
-	c.Check([]string{e, u, d}, DeepEquals, []string{"", "1.3", "pre4-1"})
+	// upstream_version and debian_revision are separated by the LAST
+	// hyphen, since upstream_version may itself contain hyphens.
+	e, u, d, err = parseVersion("1.3-pre4-1")
+	c.Check([]string{e, u, d}, DeepEquals, []string{"", "1.3-pre4", "1"})
+	c.Check(err, Equals, nil)
 
-	e, u, d = parseVersion("4:1.3-pre4-1")
-	c.Check([]string{e, u, d}, DeepEquals, []string{"4", "1.3", "pre4-1"})
+	e, u, d, err = parseVersion("4:1.3-pre4-1")
+	c.Check([]string{e, u, d}, DeepEquals, []string{"4", "1.3-pre4", "1"})
+	c.Check(err, Equals, nil)
+
+	e, u, d, err = parseVersion("1:2026.07.07-0325-54-a773b756")
+	c.Check([]string{e, u, d}, DeepEquals, []string{"1", "2026.07.07-0325-54", "a773b756"})
+	c.Check(err, Equals, nil)
+
+	e, u, d, err = parseVersion("1:1.2024-")
+	c.Check([]string{e, u, d}, DeepEquals, []string{"", "", ""})
+	c.Check(err.Error(), Equals, "could not parse version: version string ('1.2024-') includes hyphen without Debian revision")
+}
+
+func (s *VersionSuite) TestIsValidVersion(c *C) {
+	// valid cases
+	valid := isValidVersion("1.2.3-abc")
+	c.Check(valid, Equals, true)
+
+	valid = isValidVersion("1.0.1337~rc2-3")
+	c.Check(valid, Equals, true)
+
+	valid = isValidVersion("1.2.3+fdsfgs")
+	c.Check(valid, Equals, true)
+
+	valid = isValidVersion("1:2.3~4-5six")
+	c.Check(valid, Equals, true)
+
+	// upstream_version containing multiple hyphens (e.g. date/build-number/
+	// git-sha style versioning) is valid as long as it's split on the LAST
+	// hyphen for the debian_revision.
+	valid = isValidVersion("1:2026.07.07-0325-54-a773b756")
+	c.Check(valid, Equals, true)
+
+	// invalid cases
+	valid = isValidVersion("1:1.2.3-")
+	c.Check(valid, Equals, false)
+
+	valid = isValidVersion("42:")
+	c.Check(valid, Equals, false)
+
+	valid = isValidVersion("1:a.1.2.3~-4")
+	c.Check(valid, Equals, false)
+
+	// non-numeric epoch
+	valid = isValidVersion("abc:1.2.3")
+	c.Check(valid, Equals, false)
+
+	// valid upstream_version, but debian_revision contains a disallowed character
+	valid = isValidVersion("1.2.3-abc!")
+	c.Check(valid, Equals, false)
 }
 
 func (s *VersionSuite) TestCompareLexicographic(c *C) {
@@ -86,21 +140,35 @@ func (s *VersionSuite) TestCompareVersionPart(c *C) {
 }
 
 func (s *VersionSuite) TestCompareVersions(c *C) {
-	c.Check(CompareVersions("3:1.0~beta1~svn1245-1", "3:1.0~beta1~svn1245-1"), Equals, 0)
+	check := func(left, right string, expectedCmp int, expectedErr error) {
+		cmp, err := CompareVersions(left, right)
+		c.Check(cmp, Equals, expectedCmp)
+		c.Check(err, Equals, expectedErr)
+	}
 
-	c.Check(CompareVersions("1:1.0~beta1~svn1245-1", "3:1.0~beta1~svn1245-1"), Equals, -1)
-	c.Check(CompareVersions("1:1.0~beta1~svn1245-1", "1.0~beta1~svn1245-1"), Equals, 1)
-	c.Check(CompareVersions("1.0~beta1~svn1245-1", "1.0~beta1~svn1245-2"), Equals, -1)
-	c.Check(CompareVersions("3:1.0~beta1~svn1245-1", "3:1.0~beta1-1"), Equals, -1)
+	check("3:1.0~beta1~svn1245-1", "3:1.0~beta1~svn1245-1", 0, nil)
 
-	c.Check(CompareVersions("1.0~beta1~svn1245", "1.0~beta1"), Equals, -1)
-	c.Check(CompareVersions("1.0~beta1", "1.0"), Equals, -1)
+	check("1:1.0~beta1~svn1245-1", "3:1.0~beta1~svn1245-1", -1, nil)
+	check("1:1.0~beta1~svn1245-1", "1.0~beta1~svn1245-1", 1, nil)
+	check("1.0~beta1~svn1245-1", "1.0~beta1~svn1245-2", -1, nil)
+	check("3:1.0~beta1~svn1245-1", "3:1.0~beta1-1", -1, nil)
 
-	c.Check(CompareVersions("1.0-133-avc", "1.1"), Equals, -1)
-	c.Check(CompareVersions("1.0-133-avc", "1.0"), Equals, 1)
+	check("1.0~beta1~svn1245", "1.0~beta1", -1, nil)
+	check("1.0~beta1", "1.0", -1, nil)
 
-	c.Check(CompareVersions("5.2.0.3", "5.2.0.283"), Equals, -1)
-	c.Check(CompareVersions("4.3.5a", "4.3.5-rc3-1"), Equals, 1)
+	check("1.0-133-avc", "1.1", -1, nil)
+	check("1.0-133-avc", "1.0", 1, nil)
+
+	check("5.2.0.3", "5.2.0.283", -1, nil)
+	// upstream_version/debian_revision split on the LAST hyphen: "4.3.5-rc3-1"
+	// is upstream="4.3.5-rc3", debian="1", so it's actually greater than "4.3.5a"
+	// (confirmed against `dpkg --compare-versions`).
+	check("4.3.5a", "4.3.5-rc3-1", -1, nil)
+
+	// Version validation happens independently, so these exercise parse failures.
+	check("1:abc~1.2.3-", "1:1.2.3~abc-good", 0, LeftInvalid)
+	check("1:1.2.3~abc-good", "1:abc~1.2.3-", 0, RightInvalid)
+	check("1:abc~1.2.3-", "2:def~4.5.6-", 0, BothInvalid)
 }
 
 func (s *VersionSuite) TestParseDependency(c *C) {