semver: Bug: Pre-release versions not considered matching

This test fails:

diff --git a/tests/regression.rs b/tests/regression.rs
index a47a777..041bd7c 100644
--- a/tests/regression.rs
+++ b/tests/regression.rs
@@ -28,3 +28,22 @@ fn test_regressions() {
         }
     }
 }
+
+// See https://github.com/RustSec/cargo-audit/issues/17
+#[test]
+fn test_suffix() {
+    let req = semver::VersionReq::parse(">= 0.10.2").unwrap();
+    let version = semver::Version::parse("0.12.0-pre.0").unwrap();
+
+    // Test parsing
+    assert_eq!(version.major, 0);
+    assert_eq!(version.minor, 12);
+    assert_eq!(version.patch, 0);
+    assert_eq!(version.pre, vec![
+        semver::Identifier::AlphaNumeric("pre".into()),
+        semver::Identifier::Numeric(0),
+    ]);
+
+    // Test matching
+    assert!(req.matches(&version));
+}

It matches the req if I remove the “-pre.0” suffix though.

This causes cargo-audit to report wrong vulnerabilities.

I assume this is a bug, and does not follow the spec, right?

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 2
Comments: 35 (9 by maintainers)

Most upvoted comments

For reasons unrelated to this issue, we’ll be migrating away from VersionReqs (to one implemented using Version comparators, which work correctly for our purposes), so this no longer concerns me, but honestly this seems like a “bug” in the spec. I agree that this behavior does appear to match the spec as written, however.

The rationale for the criteria in the spec all center around something which does make sense: making sure that prereleases don’t match as a sort of boundary condition in a “less than” expression. As it were, I opened a separate issue related to that: #236.

But this leads to this rather confusing behavior for anything other than that particular edge case…

For the match expression >= 2.0.0, here’s the output of VersionReq::matches for the following versions:

2.0.0-rc.0: false
2.0.0: true
3.0.0-rc.0: false
3.0.0: true
4.0.0-rc.0: false
4.0.0: true

Of the prereleases returning false, I’d argue only the first one makes sense. I don’t think it makes sense for prereleases to cease to match after a major version bump (but for the corresponding release version to start matching again, only for the next major version’s prereleases to cease to match, and so on for each major version), but again, I’d agree this is the “correct” behavior per the spec as written.

tarcieri on May 25, 2021

@tarcieri I think rustsec should not be using VersionReq for this, it should simply store the Version when a fix was applied: the existing PartialOrd implementation on Version behaves the way you want.

Maybe you need something more complicated to handle regressions, eg. Vec<(Version, Option<Version>)>, but this is already more flexible than using VersionReq.

Diggsey on Sep 12, 2018