-
-
Notifications
You must be signed in to change notification settings - Fork 607
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: Exact match between CPE with NA (-) update part #2240
Fix: Exact match between CPE with NA (-) update part #2240
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
ProtectedMembersInFinalClass: Make members of final classes package-private: getInstance
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
InvalidParam: Parameter name
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
EmptyBlockTag: A block tag (@param, @return, @throws, @deprecated) has an empty description. Block tags without descriptions don't add much value for future readers of the code; consider removing the tag entirely or adding a description.
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
JdkObsolete: It is very rare for LinkedList to out-perform ArrayList or ArrayDeque. Avoid it unless you're willing to invest a lot of time into benchmarking. Caveat: LinkedList supports null elements, but ArrayDeque does not.
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
ProtectedMembersInFinalClass: Make members of final classes package-private: getInstance
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
DefaultCharset: Implicit use of the platform default charset, which can result in differing behaviour between JVM executions or incorrect behavior if the encoding of the data source doesn't match expectations.
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
ProtectedMembersInFinalClass: Make members of final classes package-private: getInstance
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
ProtectedMembersInFinalClass: Make members of final classes package-private: getInstance
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
NonCanonicalType: The type
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
DefaultCharset: Implicit use of the platform default charset, which can result in differing behaviour between JVM executions or incorrect behavior if the encoding of the data source doesn't match expectations.
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
ProtectedMembersInFinalClass: Make members of final classes package-private: getInstance
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
InvalidParam: Parameter name
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
EmptyBlockTag: A block tag (@param, @return, @throws, @deprecated) has an empty description. Block tags without descriptions don't add much value for future readers of the code; consider removing the tag entirely or adding a description.
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
JdkObsolete: It is very rare for LinkedList to out-perform ArrayList or ArrayDeque. Avoid it unless you're willing to invest a lot of time into benchmarking. Caveat: LinkedList supports null elements, but ArrayDeque does not.
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
ProtectedMembersInFinalClass: Make members of final classes package-private: getInstance
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
DefaultCharset: Implicit use of the platform default charset, which can result in differing behaviour between JVM executions or incorrect behavior if the encoding of the data source doesn't match expectations.
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
ProtectedMembersInFinalClass: Make members of final classes package-private: getInstance
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
ProtectedMembersInFinalClass: Make members of final classes package-private: getInstance
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
NonCanonicalType: The type
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
ProtectedMembersInFinalClass: Make members of final classes package-private: getInstance
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
DefaultCharset: Implicit use of the platform default charset, which can result in differing behaviour between JVM executions or incorrect behavior if the encoding of the data source doesn't match expectations.
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
EmptyBlockTag: A block tag (@param, @return, @throws, @deprecated) has an empty description. Block tags without descriptions don't add much value for future readers of the code; consider removing the tag entirely or adding a description.
ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
MissingSummary: A summary line is required on public/protected Javadocs. ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
UnusedMethod: Method 'generateExcludeSuppressed' is never used. ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
MutablePublicArray: Non-empty arrays are mutable, so this ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
💬 3 similar findings have been found in this PR InlineFormatString: Prefer to create format strings inline, instead of extracting them to a single-use constant 🔎 Expand here to view all instances of this finding
Visit the Lift Web Console to find more details in your report. ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
💬 2 similar findings have been found in this PR JavaUtilDate: Date has a bad API that leads to bugs; prefer java.time.Instant or LocalDate. 🔎 Expand here to view all instances of this finding
Visit the Lift Web Console to find more details in your report. ℹ️ Learn about @sonatype-lift commandsYou can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Note: When talking to LiftBot, you need to refresh the page to see its response. Was this a good recommendation? |
@syalioune Please rebase with The many comments from Lift appear to be a bug in Lift, we reported that to Sonatype already and it appears to be fixed for newer PRs. |
… and vulnerable_software should be possible See DependencyTrack#1929 (comment) Signed-off-by: Alioune SY <[email protected]>
750ebbf
to
bc2e844
Compare
LGTM, but I'm a bit nervous to merge this one as I can't estimate the impact this will have on existing behavior. Mainly due to the lack of tests we have for this functionality (I raised #2243 for that). |
I looked at the linked issues/PRs an I have a feeling about what this PR does. But if I really wanted to know I would have to study the code more. Can I suggest to add a little explanation here so that it "instantly" clear for readers what this PR does? |
Done, ty for the suggestion.
Would it make it less risky if I perform the update higher up instead of deep down in the comparison methods like in the snippet below. protected void analyzeVersionRange(final QueryManager qm, final List<VulnerableSoftware> vsList,
final String targetVersion, final String targetUpdate, final Component component,
final VulnerabilityAnalysisLevel vulnerabilityAnalysisLevel) {
for (final VulnerableSoftware vs: vsList) {
boolean equalCpe = (vs.getCpe23() != null && vs.getCpe23().equals(component.getCpe())) ||
(vs.getCpe22() != null && vs.getCpe22().equals(component.getCpe()));
if (equalCpe || (compareVersions(vs, targetVersion) && compareUpdate(vs, targetUpdate))) {
if (vs.getVulnerabilities() != null) {
for (final Vulnerability vulnerability : vs.getVulnerabilities()) {
NotificationUtil.analyzeNotificationCriteria(qm, vulnerability, component, vulnerabilityAnalysisLevel);
qm.addVulnerability(vulnerability, component, this.getAnalyzerIdentity());
}
}
}
}
} Worst case being to link more vuln that people are used to but it seems legit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@syalioune Sorry, I think I was not fully grasping the proposed solution when writing my previous comment.
You're right, checking for equality first makes sense as it's the simplest and most obvious "match" case. Thanks for the PR!
Description
#2188
Addressed Issue
See #1929 (comment)
This PR fix a bug making
cpe:2.3:a:xiph:speex:1.2:-:*:*:*:*:*:*
not equal tocpe:2.3:a:xiph:speex:1.2:-:*:*:*:*:*:*
when performing internal analysis. Special cases (NA
andANY
on each part) are handled before making the simplest of comparison.Right now, the relation 6 in the picture below from NIST CPE matching spec is not correctly handled for CPE's update part.
Additional Details
N/A
Checklist
- [] This PR implements an enhancement, and I have provided tests to verify that it works as intended- [ ] This PR introduces changes to the database model, and I have added corresponding update logic- [ ] This PR introduces new or alters existing behavior, and I have updated the documentation accordingly