-
-
Notifications
You must be signed in to change notification settings - Fork 592
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
Replace usages of org.json
with Jackson
#2520
base: master
Are you sure you want to change the base?
Conversation
@nscuro Let's get this out of the way :-) nscuro#187 |
Thanks @walterdeboer! I did put this on hold because there are (were?) still pending PRs that utilize Greatly appreciate the work, that'll certainly speed things up! |
I'm one of the pending PR's, that's why I wanted to give it a push :-) |
src/main/java/org/dependencytrack/parser/ossindex/OssIndexParser.java
Outdated
Show resolved
Hide resolved
src/main/java/org/dependencytrack/parser/vulndb/VulnDbParser.java
Outdated
Show resolved
Hide resolved
src/main/java/org/dependencytrack/parser/vulndb/VulnDbParser.java
Outdated
Show resolved
Hide resolved
src/main/java/org/dependencytrack/integrations/defectdojo/DefectDojoUploader.java
Outdated
Show resolved
Hide resolved
src/main/java/org/dependencytrack/integrations/fortifyssc/FortifySscUploader.java
Outdated
Show resolved
Hide resolved
🛠 Lift Auto-fixSome of the Lift findings in this PR can be automatically fixed. You can download and apply these changes in your local project directory of your branch to review the suggestions before committing.1 # Download the patch
curl https://lift.sonatype.com/api/patch/github.com/DependencyTrack/dependency-track/2520.diff -o lift-autofixes.diff
# Apply the patch with git
git apply lift-autofixes.diff
# Review the changes
git diff Want it all in a single command? Open a terminal in your project's directory and copy and paste the following command: curl https://lift.sonatype.com/api/patch/github.com/DependencyTrack/dependency-track/2520.diff | git apply Once you're satisfied, commit and push your changes in your project. Footnotes |
@nscuro And some sonatype-lift fixes here: nscuro#188 (rebased) |
src/main/java/org/dependencytrack/tasks/scanners/SnykAnalysisTask.java
Outdated
Show resolved
Hide resolved
src/main/java/org/dependencytrack/tasks/scanners/SnykAnalysisTask.java
Outdated
Show resolved
Hide resolved
src/main/java/org/dependencytrack/integrations/kenna/KennaSecurityUploader.java
Outdated
Show resolved
Hide resolved
src/main/java/org/dependencytrack/tasks/repositories/CpanMetaAnalyzer.java
Outdated
Show resolved
Hide resolved
src/main/java/org/dependencytrack/integrations/defectdojo/DefectDojoClient.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Walter de Boer <[email protected]> Signed-off-by: nscuro <[email protected]>
@@ -93,21 +94,21 @@ public void testParseSeveritiesNvd() throws IOException { | |||
|
|||
// By default NVD is first priority for CVSS, no need to set config property. |
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.
@nscuro curious question. Lately i've seen NVD "being re-evaluated" or not evaluated at all. Ive noticed some wonkiness with some tools (not sure i've seen it in DT), where it doesn't seem to like that status. Do we defer to another alias (if they are on) for the scoring if NVD has not evaluated it??
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.
Are you asking in general, or specific for Snyk?
Generally we do not re-use scoring of aliasing vulnerabilities, if a given vuln does not have a scoring.
For Snyk specifically, they provide at least one score for each vulnerability. Beside Snyk's own scoring, and the "offical" NVD one, there is also vendor-specific scoring from Red Hat, SUSE, etc.; In that case DT should follow a priority list of sorts, where it would fall back to the next preferred source if necessary.
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.
yes thats what i was thinking. following a sequence of logic if there happens to be a vuln that does not have a score (for some reason) to default to the next best source.
Moved to 4.10 as we want this to be thoroughly exercised in users' test environments. Merging shortly before 4.9 release is too risky. |
Description
In cases where we don't directly map JSON to and from POJOs, we currently predominantly use
org.json:json
.That library once shipped with Unirest, but Unirest switched to another serialization library, and we have moved away from Unirest recently, too.
This PR replaces usages of
org.json
with Jackson.Addressed Issue
Closes #1113
Additional Details
Another option would've been to use JSON-P, but the spec and its implementations lack capabilities of object mapping. That is, it's not possible to
put
a POJO into a JSON object using the JSON-P API. Jackson supports this.A new utility class,
org.dependencytrack.common.Json
, has been added which includes various methods (e.g.optString
,optArray
etc.) that behave just like their counterparts inorg.json
. This was done to reduce the amount of necessary refactoring to a minimum.Tests for various existing JSON transformations (e.g. the FPF) have been added along the way, to ensure that this change does not introduce regressions.
Usage of the
org.json.*
package has been blacklisted with Checkstyle.Checklist
This PR fixes a defect, and I have provided tests to verify that the fix is effectiveThis PR introduces changes to the database model, and I have added corresponding update logicThis PR introduces new or alters existing behavior, and I have updated the documentation accordingly