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
Jackson: upgrade for CVE-2020-36518 #8201
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8201 +/- ##
============================================
- Coverage 74.48% 74.47% -0.01%
- Complexity 43494 43496 +2
============================================
Files 3387 3387
Lines 168687 168718 +31
Branches 20168 20178 +10
============================================
+ Hits 125653 125660 +7
- Misses 33490 33510 +20
- Partials 9544 9548 +4
|
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.
Reviewable status: 0 of 18 files reviewed, all discussions resolved (waiting on @arifogel)
projects/coordinator/BUILD, line 15 at r1 (raw file):
runtime_deps = [ "//projects/question", "@maven//:javax_activation_activation",
FYI this is not needed because it's already a direct dependency of existing libs.
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.
Reviewed 18 of 18 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dhalperi)
projects/pom.xml, line 74 at r1 (raw file):
<jackson.version>2.13.2.20220328</jackson.version> <jakarta-activation.version>1.2.2</jakarta-activation.version> <javax-activation.version>1.1</javax-activation.version>
Do we need explicit javax-activation version or dep in maven anymore?
projects/batfish-common-protocol/src/main/java/org/batfish/common/util/BatfishObjectMapper.java, line 166 at r1 (raw file):
} /** Configures all the default options for a Batfish {@link ObjectMapper}. */
Is javadoc still accurate?
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.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dhalperi)
projects/batfish-common-protocol/src/main/java/org/batfish/common/util/BatfishObjectMapper.java, line 166 at r1 (raw file):
Previously, arifogel (Ari Fogel) wrote…
Is javadoc still accurate?
I think it's fine. Did you want a specific change?
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.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dhalperi)
projects/batfish-common-protocol/src/main/java/org/batfish/common/util/BatfishObjectMapper.java, line 166 at r1 (raw file):
Previously, dhalperi (Dan Halperin) wrote…
I think it's fine. Did you want a specific change?
Just making sure ObjectMapper
is still accurate. I guess so, given the lack of other changes.
Includes fixups: - new builder pattern - Jackson now correctly implements Include.NON_EMPTY for JsonValue, so things like IntegerSpace are not serialized instead of "". Fix a few deserializers as a result. - switch to jakarta activation API - it's the new thing.
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.
Reviewed 15 of 18 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dhalperi)
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.
Reviewed 6 of 6 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dhalperi)
Includes fixups:
so things like IntegerSpace are not serialized instead of "".
Fix a few deserializers as a result.