Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Default to UNKNOWN in java version checker #1637

Merged
merged 3 commits into from Apr 14, 2022

Conversation

terriko
Copy link
Contributor

@terriko terriko commented Apr 13, 2022

Static analysis says we can still sometimes wind up with version set to None in the java version parser. Set it explicitly to "UNKNOWN" to match the other checkers.

cc @anthonyharrison who might want to review this

@@ -236,6 +236,11 @@ def run_java_checker(self, filename: str) -> Iterator[ScanInfo]:
version = root.find(schema + "version").text
if version is None and parent is not None:
version = parent.find(schema + "version").text

# If all else fails, set version to UNKNOWN
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alienmaster pointed out that saying "else" here can make it confusing when there is no actual code else involved. English is confusing as always. I'm going to go ahead and try to change the comment to something clearer.

@@ -236,6 +236,11 @@ def run_java_checker(self, filename: str) -> Iterator[ScanInfo]:
version = root.find(schema + "version").text
if version is None and parent is not None:
version = parent.find(schema + "version").text

# If all else fails, set version to UNKNOWN
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alienmaster pointed out that saying "else" here can make it confusing when there is no actual code else involved. English is confusing as always. I'm going to go ahead and try to change the comment to something clearer.

@codecov-commenter
Copy link

Codecov Report

Merging #1637 (6f57051) into main (9e32cd7) will decrease coverage by 0.16%.
The diff coverage is 0.00%.

❗ Current head 6f57051 differs from pull request most recent head d34af62. Consider uploading reports for the commit d34af62 to get more accurate results

@@            Coverage Diff             @@
##             main    #1637      +/-   ##
==========================================
- Coverage   78.49%   78.33%   -0.17%     
==========================================
  Files         291      291              
  Lines        5975     5995      +20     
  Branches      980      983       +3     
==========================================
+ Hits         4690     4696       +6     
- Misses       1072     1082      +10     
- Partials      213      217       +4     
Flag Coverage Δ
longtests 78.33% <0.00%> (-0.17%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cve_bin_tool/version_scanner.py 75.58% <0.00%> (-0.60%) ⬇️
cve_bin_tool/nvd_api.py 75.00% <0.00%> (-5.18%) ⬇️
test/test_cli.py 81.01% <0.00%> (-1.64%) ⬇️
cve_bin_tool/cli.py 70.43% <0.00%> (+0.43%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@terriko terriko merged commit 2d22c68 into intel:main Apr 14, 2022
anthonyharrison pushed a commit to anthonyharrison/cve-bin-tool that referenced this pull request May 2, 2022
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.

None yet

3 participants