KAFKA-13518: Update gson dependency by dongjinleekr · Pull Request #11579 · apache/kafka

dongjinleekr · 2021-12-08T11:52:25Z

Here is the fix. Since spotbugs 4.5.1 was released just 12 hours ago, it would take a little bit to be synched with maven central.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

showuon

@dongjinleekr , thanks for the PR. But there's spotBugs 4.5.2. Could you upgrade to the latest one? Also, the PR title/description can't tell what's the relationship between spotBugs and gson, could you add explanation there? Thanks.

showuon

Also, there are build errors. Please fix them. Thanks.

dongjinleekr · 2021-12-25T07:31:41Z

Hi @showuon,

Here it is. I just updated the issue title more clearly and updated the spotbugs dependency into 4.5.2.

showuon

LGTM!

showuon

Please also help fix the build failure. Thanks.

dongjinleekr · 2021-12-25T09:11:37Z

@showuon Sorry for bothering you. Here is the update. There were some updates on spotbugs between 4.2.2 and 4.5.2 and some previously-unfound problems are now detected:

In 4.3.0, spotbugs improved their detection logic for MS_EXPOSE_REP:

MS_EXPOSE_REP and EI_EXPOSE_REP are now reported for code returning a reference to a mutable object indirectly (e.g. via a local variable)
In 4.4.2, spotbugs fixed some false positives for DMI_RANDOM_USED_ONLY_ONCE and started to detect some unfound problems:

DMI_RANDOM_USED_ONLY_ONCE false positive

After the update, it works like a charm:

showuon

@dongjinleekr , thanks for the update, but it looks like there are other failed spotBugs.

dongjinleekr · 2021-12-27T14:18:29Z

@showuon My bad. I found several other false-positives from other modules with spotbugs; They are now fixed. (Please see the comments.) 🙇

ijuma · 2022-02-05T05:30:32Z

Thanks for the PR. Seems like the new version has more false positives. Do you know if they intend to fix those?

dongjinleekr · 2022-02-14T10:28:40Z

@ijuma

Do you know if they intend to fix those?

Oh yes, as you can see in the updated PR, I updated spotbugs to 4.5.3 following the gradle plugin 5.0.5, and rebased onto the latest trunk. It seems like there are a bunch of false positives in the recent version of spotbugs (below) but, I verified that none of them are affecting.

+1. They also have not fixed the issues I commented on in spotbugs-exclude.xml yet. I will follow up and apply them as soon as they fix them.

ijuma · 2022-02-14T14:10:04Z

Should we wait until they fix these issues in spotBugs? It doesn't look like the cost/benefit in upgrading here isn't favorable.

dongjinleekr · 2022-02-15T00:46:23Z

@ijuma If you don't mind CVE WS-2021-0419 introduced by gson 2.8.6. This PR is to fix it.

dongjinleekr · 2022-03-17T03:18:23Z

Rebased onto the latest trunk. cc/ @ijuma

Boojapho · 2022-05-18T16:58:46Z

Gson library has another recent vulnerability: https://nvd.nist.gov/vuln/detail/CVE-2022-25647. Gson library 2.8.9 fixes this, which is included in Spotbugs 4.5.0 and higher.

dongjinleekr · 2022-06-06T01:41:38Z

@Boojapho Thanks for reporting. Here is the fix - rebased onto the latest trunk and upgraded spotbugs into 4.7.0, which also fixes the gson vulnerability.

ijuma · 2022-06-06T13:10:52Z

Looks like spotbugs/spotbugs@720af6c would help, but it's not available in a released version yet.

ijuma · 2022-06-06T13:11:16Z

Disabled by default here spotbugs/spotbugs@7ba0e74 (not released yet).

ijuma · 2022-06-06T13:12:38Z

Why do we have to disable this one?

Public static org.apache.kafka.server.metrics.KafkaYammerMetrics.defaultRegistry() may expose internal representation by returning KafkaYammerMetrics.metricsRegistry.

Public static org.apache.kafka.common.security.auth.SecurityProtocol.names() may expose internal representation by returning SecurityProtocol.NAMES

ijuma · 2022-06-06T13:12:53Z

Are we disabling this one due to false positives?

Yes.

Random object created and used only once in new kafka.tools.ConsoleConsumer$ConsumerConfig(String[])

Random object created and used only once in new kafka.tools.ConsumerPerformance$ConsumerPerfConfig(String[])

…D_HAVE_BEEN_A_NPE, EI_EXPOSE_STATIC_REP2

…4.7.0

… to leave the workarounds later. 3. Reduce the scope of DMI_RANDOM_USED_ONLY_ONCE and MS_EXPOSE_REP into scala-only and some specific classes only, respectively.

dongjinleekr · 2022-06-11T13:43:56Z

@ijuma Here is the update:

Rebased onto the latest trunk.
Gather the false positives together and add some TODO comments not to leave the workarounds later.
Reduce the scope of DMI_RANDOM_USED_ONLY_ONCE and MS_EXPOSE_REP into scala-only and some specific classes only, respectively.

ijuma · 2022-08-08T14:36:16Z

Can you please update to spotbugs 4.7.1? It seems like it fixes the false positives.

omkreddy · 2022-10-19T14:14:39Z

@dongjinleekr I have not seen this PR. I have raised draft PR #12768 to upgrade spotbugs. Can we update to spotbugs 4.7.3?

omkreddy · 2022-10-24T17:42:23Z

Closing this PR in favour of #12768

dongjinleekr force-pushed the feature/KAFKA-13518 branch from 21bff61 to 955e08a Compare December 25, 2021 02:14

showuon reviewed Dec 25, 2021

View reviewed changes

dongjinleekr changed the title ~~KAFKA-13518: Update gson and netty-codec in 3.0.0~~ KAFKA-13518: Update gson dependency Dec 25, 2021

showuon approved these changes Dec 25, 2021

View reviewed changes

showuon reviewed Dec 25, 2021

View reviewed changes

showuon reviewed Dec 27, 2021

View reviewed changes

dongjinleekr force-pushed the feature/KAFKA-13518 branch from 62fb35b to 2cd8ca1 Compare February 14, 2022 10:28

andrewchoi5 approved these changes Feb 21, 2022

View reviewed changes

dongjinleekr force-pushed the feature/KAFKA-13518 branch from 2cd8ca1 to 7bd7b86 Compare March 17, 2022 03:17

dongjinleekr force-pushed the feature/KAFKA-13518 branch from 7bd7b86 to d4e9f36 Compare June 6, 2022 01:38

ijuma reviewed Jun 6, 2022

View reviewed changes

Comment thread gradle/spotbugs-exclude.xml Outdated

ijuma Jun 6, 2022

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

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

Disabled by default here spotbugs/spotbugs@7ba0e74 (not released yet).

ijuma reviewed Jun 6, 2022

View reviewed changes

dongjinleekr added 5 commits June 11, 2022 22:40

Upgrade spotbugs dependency: 4.2.2 → 4.7.0

542a52e

Add spotbugs-exclude: MS_EXPOSE_REP, DMI_RANDOM_USED_ONLY_ONCE

2511e7b

Suppress false-positive spotbugs checks: RCN_REDUNDANT_NULLCHECK_WOUL…

1624eb9

…D_HAVE_BEEN_A_NPE, EI_EXPOSE_STATIC_REP2

Disable bug types introduced by the ThrowingExceptions detector with …

bcc3f1d

…4.7.0

1. Gather the false positives together. 2. Add some TODO comments not…

d193b71

… to leave the workarounds later. 3. Reduce the scope of DMI_RANDOM_USED_ONLY_ONCE and MS_EXPOSE_REP into scala-only and some specific classes only, respectively.

dongjinleekr force-pushed the feature/KAFKA-13518 branch from d4e9f36 to d193b71 Compare June 11, 2022 13:43

ijuma mentioned this pull request Oct 19, 2022

MINOR: Upgrade spotbugs dependency #12768

Merged

3 tasks

omkreddy closed this Oct 24, 2022

Conversation

dongjinleekr commented Dec 8, 2021

Committer Checklist (excluded from commit message)

Uh oh!

showuon left a comment

Choose a reason for hiding this comment

Uh oh!

showuon left a comment

Choose a reason for hiding this comment

Uh oh!

dongjinleekr commented Dec 25, 2021

Uh oh!

showuon left a comment

Choose a reason for hiding this comment

Uh oh!

showuon left a comment

Choose a reason for hiding this comment

Uh oh!

dongjinleekr commented Dec 25, 2021

Uh oh!

showuon left a comment

Choose a reason for hiding this comment

Uh oh!

dongjinleekr commented Dec 27, 2021

Uh oh!

ijuma commented Feb 5, 2022

Uh oh!

dongjinleekr commented Feb 14, 2022

Uh oh!

ijuma commented Feb 14, 2022

Uh oh!

dongjinleekr commented Feb 15, 2022

Uh oh!

dongjinleekr commented Mar 17, 2022

Uh oh!

Boojapho commented May 18, 2022

Uh oh!

dongjinleekr commented Jun 6, 2022

Uh oh!

ijuma Jun 6, 2022

Choose a reason for hiding this comment

Uh oh!

ijuma Jun 6, 2022

Choose a reason for hiding this comment

Uh oh!

ijuma Jun 6, 2022

Choose a reason for hiding this comment

Uh oh!

dongjinleekr Jun 11, 2022

Choose a reason for hiding this comment

Uh oh!

ijuma Jun 6, 2022

Choose a reason for hiding this comment

Uh oh!

dongjinleekr Jun 11, 2022

Choose a reason for hiding this comment

Uh oh!

dongjinleekr commented Jun 11, 2022

Uh oh!

ijuma commented Aug 8, 2022

Uh oh!

omkreddy commented Oct 19, 2022

Uh oh!

omkreddy commented Oct 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants