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

Dependency bumps #1144

Merged
merged 6 commits into from
Oct 6, 2022
Merged

Dependency bumps #1144

merged 6 commits into from
Oct 6, 2022

Conversation

eedrummer
Copy link
Collaborator

@eedrummer eedrummer commented Sep 13, 2022

This PR bumps dependencies to eliminate some transitive dependencies on libraries with known vulnerabilities. It also eliminates the dependency on the Spring Framework (!) which was being used solely for its HTTP client. That has now been replaced by OkHttp, which was already a transitive dependency.

An update to the library that is used for URL validation started flagging URLs being constructed for value set expansions. This was fixed by URI encoding some of the characters in the URL, but this should be checked more closely.

This PR bumps the minimum version of Java required to run Synthea to Java 11. This is required to use hapi 6.x.

There are still known issues that will require deeper code changes to fix:

  • Physiology - cql-engine and cql-to-elm both have vulnerabilities, but the underlying use of CQL will need to be changed to deal with changes in the environment. Perhaps more difficult is that this functionality relies on jsmbl, which has a critical vulnerability in its most recently released version.
  • graphviz - graphviz-java needs to be updates, but API changes break existing code

@codecov
Copy link

codecov bot commented Sep 15, 2022

Codecov Report

Merging #1144 (dd459fe) into master (0afa939) will increase coverage by 0%.
The diff coverage is 76%.

@@           Coverage Diff            @@
##             master   #1144   +/-   ##
========================================
  Coverage        80%     80%           
- Complexity     3410    3429   +19     
========================================
  Files           135     135           
  Lines         22769   22786   +17     
  Branches       3090    3091    +1     
========================================
+ Hits          18225   18245   +20     
- Misses         3578    3588   +10     
+ Partials        966     953   -13     
Impacted Files Coverage Δ
src/main/java/org/mitre/synthea/export/FhirR4.java 84% <ø> (+<1%) ⬆️
...org/mitre/synthea/helpers/RandomCodeGenerator.java 88% <71%> (-8%) ⬇️
...va/org/mitre/synthea/export/FhirR4PatientHome.java 94% <100%> (+1%) ⬆️
...ain/java/org/mitre/synthea/export/CSVExporter.java 82% <0%> (-2%) ⬇️
...world/concepts/healthinsurance/CoverageRecord.java 88% <0%> (-2%) ⬇️
.../main/java/org/mitre/synthea/export/FhirDstu2.java 85% <0%> (-2%) ⬇️
...c/main/java/org/mitre/synthea/export/FhirStu3.java 83% <0%> (-2%) ⬇️
...e/synthea/modules/CardiovascularDiseaseModule.java 94% <0%> (-1%) ⬇️
src/main/java/Graphviz.java 0% <0%> (ø)
... and 17 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Some tests are broken, but this commit does a few things:
* Bumps most dependencies
* Removes Spring as it was only used for RestClient
* Uses OkHttp since it was already a transitive dependency

Remaining issues are related to URL validation for value set expansion.
Previous versions of the library were ok with things like `http://snomed.info/sct?fhir_vs=ecl/<<2491000087104`.
Not so much anymore.
Fixed some of the cases where invalid characters for URLs are being
used in value set expansion URLs
ObjectMapper objectMapper = new ObjectMapper();
valueSet = objectMapper.readValue(response.getBody(),
valueSet = objectMapper.readValue(response.body().byteStream(),
Copy link
Contributor

Choose a reason for hiding this comment

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

NULL_DEREFERENCE: object returned by response.body() could be null and is dereferenced at line 79.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

implementation 'org.yaml:snakeyaml:1.27'
implementation 'org.apache.commons:commons-csv:1.9.0'
implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-csv:2.13.4'
implementation 'org.yaml:snakeyaml:1.32'
Copy link
Contributor

Choose a reason for hiding this comment

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

Medium Vulnerability:

pkg:maven/org.yaml/[email protected]

0 Critical, 0 High, 1 Medium, 0 Low, 0 None vulnerabilities have been found across 1 dependencies

To see more details about this component, go to the Sonatype Lift console


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]


// google guava for some data structures
implementation 'com.google.guava:guava:30.0-jre'
implementation 'com.google.guava:guava:31.1-jre'
Copy link
Contributor

Choose a reason for hiding this comment

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

Medium Vulnerability:

pkg:maven/com.google.guava/[email protected]

0 Critical, 0 High, 1 Medium, 0 Low, 0 None vulnerabilities have been found across 1 dependencies

To see more details about this component, go to the Sonatype Lift console


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@eedrummer eedrummer marked this pull request as ready for review September 20, 2022 21:02
@eedrummer eedrummer mentioned this pull request Sep 23, 2022
@jawalonoski
Copy link
Member

⚠️ This PR bumps the minimum version of Java required to run Synthea to Java 11. ⚠️

@eedrummer eedrummer mentioned this pull request Sep 28, 2022
@jawalonoski jawalonoski merged commit 261450b into master Oct 6, 2022
@jawalonoski jawalonoski deleted the dependency-bumps branch October 6, 2022 20:53
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.

2 participants