-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-22863 : Cleanup transitive Jackson1 vulnerable dependencies(forward-port HBASE-22728) #505
Conversation
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
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.
Is it safe to just exclude these transitive dependencies? At least hadoop adds them as dependencies...
hbase-zookeeper/pom.xml
Outdated
@@ -274,6 +284,16 @@ | |||
<dependency> | |||
<groupId>org.apache.hadoop</groupId> | |||
<artifactId>hadoop-common</artifactId> | |||
<exclusions> |
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.
Can we add these exclusions in the parent pom? No?
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.
Unfortunately, that doesn't work
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.
Oh you mean just exclude from parent pom and we don't need to exclude from individual modules right? Sure let me do that for jackson-jaxrs and jackson-xc since they are common across all modules
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, that's what I mean, excluding in the parent pom so the sub modules just inherit the exclusions automatically. But I'm not a maven expert so please confirm whether it actually works...
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.
@Apache9 this doesn't work as expected. For instance, if downstream app has hbase-client dependency and if we have just excluded from parent pom, downstream would still get vulnerable Jackson1 dependencies from hbase-client. In fact, the same is done for branch-1: 4b34d24 (removal in individual modules so that downstreamer won't pull in from HBase)
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.
OK for now but this is a bit strange. At least the downstreamer need to find the version from parent pom, then why it just ignores the exclusions?
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.
And what I mean is to exclude them in the dependencyManagement section in the parent pom, not in dependency section(and I wonder whether we have this section in the parent pom...)
Could you please try again?
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.
You are correct @Apache9
It works...Thanks. Just committed the changes
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.
I have committed all the necessary and suggested changes.
@Apache9 Since we have moved to Jackson2, we can safely exclude these dependencies. At some places, we require jackson-mapper-asl:1.9.13(CVE exposed) at test scope to run tests like HBaseTestUtility.startMiniCluster(). But definitely not required at compile scope as we would expose these to downstreamers otherwise. This is partly forwardport from HBASE-22728 to master and branch-2. |
So we need to add the jackson dependency explicitly as a test dependency in our own pom as we exclude them from the hadoop-common? |
Correct. Just did it here: https://github.com/apache/hbase/pull/505/files#diff-659d55cb388d174df96d2c372cfc982fR138 |
So what about our own UTs? How do we include the jackson1 dependencies for hbase-server test module for example? |
That is already included from hadoop-minicluster only, but at test scop:
|
Let me provide the full dependency tree for Jackson1 with this patch:
@Apache9 Please let me know how this looks. |
🎊 +1 overall
This message was automatically generated. |
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.
+1. Let's try it.
1e8f7d0
to
3251c54
Compare
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
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.
+1
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Thanks for the review @Apache9 @Reidddddd For branch-2, I have attached patch(HBASE-22863.branch-2.000.patch) on the JIRA. With that patch, dependency tree for branch-2 comes exactly similar to what I updated on this PR for master. |
💔 -1 overall
This message was automatically generated. |
@Apache9 @Reidddddd |
💔 -1 overall
This message was automatically generated. |
…rd-port HBASE-22728) (#505) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Reid Chan <reidchan@apache.org>
…rd-port HBASE-22728) (#505) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Reid Chan <reidchan@apache.org>
…rd-port HBASE-22728) (#505) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Reid Chan <reidchan@apache.org>
💔 -1 overall
This message was automatically generated. |
…rd-port HBASE-22728) (apache#505) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Reid Chan <reidchan@apache.org>
…rd-port HBASE-22728) (apache#505) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Reid Chan <reidchan@apache.org> (cherry picked from commit 84d092c) Change-Id: Ib0da7f02001acead63190b80c27413cb2e6a93c5
Partly forwardport from HBASE-22728 to master and branch-2