[commons-text-upgrade] Upgrade commons text transitive dependency

[abrackx]

Impact

• Bug fix (non-breaking change which fixes expected existing functionality)
• Enhancement/New feature (adds functionality without impacting existing logic)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

Excludes vulnerable transitive dependency, adds upgraded dependency and includes upgraded dependency in lib.
https://security.snyk.io/vuln/SNYK-JAVA-ORGAPACHECOMMONS-3043138

Things to be aware of

Adds upgraded library to internal/lib directory when shipping.

Things to worry about

Not sure if necessary to add this dependency to the dependencyset, I don't know if excluding the dependency will break the opencsv jar that we ship out. I'm assuming it does.

Additional Context

N/A

[github-actions]

Unit Test Results

  4 668 files  ±0    4 668 suites  ±0   35m 21s ⏱️ + 2m 2s
  4 642 tests ±0    4 416 ✔️ ±0     226 💤 ±0  0 ❌ ±0 
54 708 runs  ±0  49 645 ✔️ ±0  5 063 💤 ±0  0 ❌ ±0 

Results for commit 41198f7. ± Comparison against base commit 856ed0e.

♻️ This comment has been updated with latest results.

[abrackx]

I tested this using the generated zip and validated that the loadData changeset still works as expected. Let me know if there's a better test.

[StevenMassaro]

Here's the discussion in opencsv about upgrading commons-text on their end

https://sourceforge.net/p/opencsv/patches/66/

It's not clear how long it will be before this is fixed in the upstream. Generally, I would prefer to wait for them to fix it than us fix it.

[abrackx]

Here's the discussion in opencsv about upgrading commons-text on their end

https://sourceforge.net/p/opencsv/patches/66/

It's not clear how long it will be before this is fixed in the upstream. Generally, I would prefer to wait for them to fix it than us fix it.

I agree. @suryaaki asked me to provide this patch and get @nvoxland's take.

[suryaaki2]

We need to merge this PR without waiting for opencsv fix as few customers have already reached out to us about this issue..
@kristyldatical

[kristyldatical]

I can't say whether they are paying customers, but in the span of about 24 hours, we were notified by 3 unique sources that this vulnerability was detected and they asked when it would be addressed. Given that, I think it would behoove us to upgrade commons-text independently now, and when opencsv provides the additive upgrade, we can switch to that.

[nvoxland]

I've seen some new articles about it even, so some interest in it going along.

In general, I tend to not like forcing upgrading libraries not used by our code but only by libraries we use. It introduces risk since the code we use has explicitly tested themselves against one version and we're replacing that with something else. Instead, I like just waiting for our direct dependency to come up with a new version that uses the new sub-dependency. If the sub-dependency is a big enough deal, the owning library tends to fix it pretty quickly.

In this case, while they are going to address the problem (https://sourceforge.net/p/opencsv/source/merge-requests/34/) there seems to be enough asks of us to do a temporary work-around of overriding the dependency. Especially since the commons-text 1.9 to 1.10 change seems pretty straighforward. That is a very stable library.

I did ask @abrackx to update this PR with comments on when to remove it in the code, and also try to take out the portion to make less for us to have to remove the next week or whenver opencsv gets upgraded.