zipkin-reporter-java
zipkin-reporter-java copied to clipboard
Upgrade libthrift to 0.14.1
Feature
upgarde thrift to 0.14.x which contais CVE-2020-13949 libthrift: potential DoS when processing untrusted payloads
Rationale
it also help camel-zipkin which depends on io.zipkin.reporter2:zipkin-sender-libthrift
Example Scenario
Prior Art
- it should be related to https://github.com/openzipkin/zipkin/pull/3368
- camel upgarde thrift PR https://github.com/apache/camel/pull/5976
I will raise a PR when https://github.com/openzipkin/zipkin/pull/3368 get merged and waiting for the next release of zipkin.
zipkin server can upgrade its thrift independently as the important thing is that the serialization is compatible.
so the versions don't need to be interlocked as long as newer versions of libthrift don't change the serialization format. However, versions after 0.13.0 break classpath compatibility, which is an issue.
If we change the version here above 0.13.0, it will break people who cannot change code that depends on libthrift.
So, the main way out we usually use is when there's a hard break, we make another reporter that is forwards compatible. We could just break and upgrade (on next minor) considering scribe is a deprecated transport anyway.
@shakuzen @jeqo @anuraaga any feelings on this one?
We could just break and upgrade (on next minor) considering scribe is a deprecated transport anyway.
Would bumping major be an option?
Would bumping major be an option?
yep sure
@zhfeng @anuraaga another option is to remove the thrift/scribe transport from camel. Zipkin has deprecated this for ages and the only main consumer is finagle, which doesn't use this code in Twitter's repo, nor the finagle integration https://github.com/openzipkin/zipkin-finagle/blob/master/scribe/src/main/java/zipkin2/finagle/scribe/ScribeSender.java
so add 👍 to just remove this transport on the next major release (stop publishing it)
It seems reasonable to not get too deep in tricky compatibility for this old transport. Either remove or bump on a major preferably or otherwise even minor all seem fine to me.
Yeah ,camel-zipkin
has been removed in Camel 4.0
after sleeping on it, I think in the special case of libthrift which I think the community is used to revlock forcing.. bump on next minor and cite in RATIONALE.md is how I'll handle it.
@zhfeng sorry this led to axing of camel-zipkin, but understand as this issue was not looked at in over two years. One goal I have is to find sites who are active and can help with maintenance to avoid this happening again. In any case, I apologize on behalf of the maintainers group for the time you lost.