Skip to content

Commit 46c6819

Browse files
authored
fix: improve version_compare to drop hashes (#3566)
Currently version_compare raises an error when it gets a hash, since we can't really compare those without more knowledge of the product. This changes the code to silently drop hashes and similar distro markers from the end of versions, which should make our code more robust to real-life cases. We may also want to put in an error handler to log the versions we can't handle without halting the program, but this doesn't do that yet. * Adds tests based on #3556 rust versions * fixes #3556 Signed-off-by: Terri Oda <[email protected]>
1 parent cb3826d commit 46c6819

File tree

2 files changed

+23
-6
lines changed

2 files changed

+23
-6
lines changed

cve_bin_tool/version_compare.py

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ def parse_version(version_string: str):
4949
# future, but we'd like to look at those cases before adding them in case the version
5050
# logic is very different.
5151

52-
# Attempt a split
52+
# remove any trailing . then split
53+
versionString = versionString.strip(".")
5354
split_version = versionString.split(".")
5455

5556
# if the whole string was numeric then we're done and you can move on
@@ -58,10 +59,10 @@ def parse_version(version_string: str):
5859
return versionArray
5960

6061
# Go through and split up anything like 6a in to 6 and a
61-
number_letter = re.compile("([0-9]+)([a-zA-Z]+)")
62-
letter_number = re.compile("([a-zA-Z]+)([0-9]+)")
62+
number_letter = re.compile("^([0-9]+)([a-zA-Z]+)$")
63+
letter_number = re.compile("^([a-zA-Z]+)([0-9]+)$")
6364
for section in split_version:
64-
# if it's all letters or all nubmers, just add it to the array
65+
# if it's all letters or all numbers, just add it to the array
6566
if section.isnumeric() or section.isalpha():
6667
versionArray.append(section)
6768

@@ -84,9 +85,19 @@ def parse_version(version_string: str):
8485
elif re.match(letter_number, section):
8586
versionArray.append(section)
8687

87-
# If all else fails, complain
88+
# It's not a "pure" alpha or number string, it's not something like rc12 or 44g
89+
90+
# It could be a hash, which we can't string compare without knowledge of the product.
91+
# It could also be a distro release string like deb8u5, which we could compare
92+
# but the data may not be useful or usable in context.
8893
else:
89-
if versionString != ".":
94+
# If it's the last part of the version just drop it silently
95+
# we could log these but I suspect it would be very noisy
96+
if section == split_version[len(split_version) - 1]:
97+
pass
98+
99+
# if it's not, raise an exception because we should probably examine it
100+
elif versionString != ".":
90101
raise CannotParseVersionException(f"version string = {versionString}")
91102

92103
return versionArray

test/test_version_compare.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ def test_lt(self):
3131
assert Version("9.10") < Version("9.10.post")
3232
assert Version("5.3.9") < Version("5.4")
3333
assert Version("2.0.0") < Version("2.0.0-1+deb9u1")
34+
assert Version("0.0.0.20190813141303.74dc4d7220e7") < Version(
35+
"0.0.0.20200813141303"
36+
)
3437

3538
def test_gt(self):
3639
"""Make sure > works between versions, including some with unusual version schemes"""
@@ -44,6 +47,9 @@ def test_gt(self):
4447
assert Version("9.10.post") > Version("9.10")
4548
assert Version("5.5") > Version("5.4.1")
4649
assert Version("2.0.0-1+deb9u1") > Version("2.0.0")
50+
assert Version("0.0.0.20200813141303") > Version(
51+
"0.0.0.20190813141303.74dc4d7220e7"
52+
)
4753

4854
def test_error(self):
4955
"""Make sure 'unknown' and blank strings raise appropriate errors"""

0 commit comments

Comments
 (0)