Performance degradation in Java from duplicate warnings check

[tkruse]

What version of protobuf and what language are you using?
Version: main/v3.25.8
Language: Java

What operating system (Linux, Windows, ...) and version?
Linux

What runtime / compiler are you using (e.g., python version or gcc version)
Java17

What did you do?
On a Java webServer producing a lot of protoc2 messages, upgrade from 3.25.3 to 3.25.8

What did you expect to see
No functional change

What did you see instead?
Degraded performance, pinpointed to thread contention for method call Set.add in this code

private static final Set<String> loggedPre22TypeNames
      = Collections.synchronizedSet(new HashSet<String>()); // <-- synchronizedSet causing locking

static void warnPre22Gencode(Class<?> messageClass) {
    // ...
    if (!loggedPre22TypeNames.add(messageName)) {   //   <---- thread contention here
       return;
    }
    logger.warning(vulnerabilityMessage);
}

Related changes:

• https://github.com/protocolbuffers/protobuf/pull/20084/files Add a ExtensionsImmutable check poison pill
• Weaken Failure to warning in 320eafa
• Prevent multiple logging in https://github.com/protocolbuffers/protobuf/pull/21754/files

CC @esrauchg @zhangskz

[mmorrisontx]

Might be more reasonable to use a CopyOnWriteArraySet in this spot.

[esrauchg]

Thanks for the report. If someone wants to send a PR to switch it to CopyOnWriteArraySet here I will accept it.

Though a couple flags here:

1. The reason for the warning on that codepath is that it is a path that only executes if your code is covered by this CVE (as noted in the message): GHSA-h4h5-3hr4-j3g2

That CVE is a "some adversarial inputs will take an arbitrarily long time to parse / consume server resources continuing to try to parse" issue.

The codepath isn't executed at all when the runtime is used alongside gencode is not covered by that CVE, so there's only any runtime performance cost if you are executing affected gencode. If you are performance or security sensitive and this is showing up as a topic, it is a strong indication you should prioritizing regenerating the corresponding affected gencode.

2. We are separately restoring compatibility with ancient gencode on the 4.x line in upcoming minor releases; you can already test this on 4.33-rc2 if you are interested in compatibility. As we are removing all known blockers from 4.x compatibility, it increasingly puts 3.x into its intended "urgent/CVE bugfix only" state.

Optimistically that means we're not going to need to do another 3.25 patch release, so a switch of this code to CopyOnWriteArraySet likely will only be released in the 4.x lineage (but, would be released and so if you intentionally want to use insecure gencode on trusted inputs without this performance concern, you would be able to get it that way).

[mmorrisontx]

it is a strong indication you should prioritizing regenerating the corresponding affected gencode

This issue actually appeared during a big internal bump of protobuf-java, in preparation for that big internal regeneration of gencode. Chicken and egg problem, we need the new runtime first in order to get the new gencode :)

Understood though, if the v3 branch is on the way out, we'll probably workaround this by setting the ignore system property everywhere during the interim time.

[esrauchg]

That makes a lot of sense; apologies for any difficulty caused by this. The notice here is only to try to help people exactly like you, its sad when it induces negative tradeoffs instead.

I think setting the disable define makes sense for your case. The 'bad' codepath is only <= 3.21 gencode so if thats not clean for some reason it may also be possible that you could go onto eg 3.25.5 patch release (before this warning was added) and regen to that point, and then proceed forward.

v3 branch is on the way out

v3 will still be supported for critical fixes through Q1-2027. The broader context is that we had intended for 4.x to only support a narrower set of old gencode which caused a lot of reasons for people to stay on 3.x for an extended period. But as I mentioned, starting with the upcoming 4.33 the ancient gencode compat window will be extended back again, which I'm optimistic will make it smoother for people to move up and reduce the need for further 3.x patches.

[tkruse]

@esrauchg feel free to close as wontfix if "setting the disable" is the recommended mitigation for users like us.

[esrauchg]

Ok, I think we can resolve that the official recommendation is to set the env variable to disable here for your need; after you believe you have upgraded gencode you can restore it to the default and confirm that you've flushed them out.

I've committed a corresponding fix to main which will mitigate the same problem from happening to future people when they go onto 4.x (once its released).

Thank you for the clear report, feel free to open similar issues for any other problems you hit when upgrading Protobuf versions going forward.