Skip to content

fix(version): do not cache a comparator that failed to build#3567

Open
arpitjain099 wants to merge 1 commit into
anchore:mainfrom
arpitjain099:fix/version-comparator-cache
Open

fix(version): do not cache a comparator that failed to build#3567
arpitjain099 wants to merge 1 commit into
anchore:mainfrom
arpitjain099:fix/version-comparator-cache

Conversation

@arpitjain099

Copy link
Copy Markdown

getComparator caches the constructed comparator in v.comparators[format] unconditionally, including when construction returned an error:

comparator, err = newSemanticVersion(v.Raw, false) // may return a broken comparator + err
...
v.comparators[format] = comparator
return comparator, err

On a parse failure the constructor returns a zero-value comparator (for Semantic/JVM/Bitnami its internal *hashiVer.Version is nil) together with an error. Caching it means the next lookup for that format hits the cache and returns it with a nil error, so the caller proceeds to Compare, which dereferences the nil version and panics. For formats without a pointer it silently compares against an empty version, i.e. a false negative in matching.

The map persists across calls once a pointer-receiver entry point (Is / Validate) populates it, so a repeated comparison on the same *Version triggers it. In-tree this is reachable from matcher/rpm/rhel_eus.go neededFixes, which calls v.Is(LT, ...) in a loop on the same package version.

Repro:

v := New("3.e", SemanticFormat)
other := New("1.0", SemanticFormat)
v.Is(LT, other) // caches the broken comparator; error is logged/handled
v.Is(LT, other) // cache hit: nil error, then panic in Compare

The fix returns early on a build error so the broken comparator is never cached, and the error surfaces consistently instead. Added a regression test. go test ./grype/version/ passes.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant