[CookieStore] Only set Cookies if they are not already set

[pickypg]

This changes the behavior of the automatic usage of the CookieStore to avoid overwriting already-set Cookies and, instead only sets them if they do not exist yet.

Closes #1964

[hyperxpro]

I'll take a look this weekend.

[hyperxpro]

Sadly this is a breaking change. RequestBuilderBase is an abstract class and adding anything will break compatibility. Perhaps, you might want to take a different approach.

[pickypg]

I'm happy to if you have a suggestion? The current functionality is buggy given that it overrides explicitly provided cookies.

[hyperxpro]

Let me try something

[hyperxpro]

I'd say change this directly in RequestBuilder. People extending RequestBuilderBase will be safe from this breaking change.

Copilot reviewed 4 out of 4 changed files in this pull request and generated no suggestions.

[pickypg]

That's doable, but this breaking change is clearly necessary.

[hyperxpro]

Yup, I do feel this but as a library maintainer, I cannot. I am at fault, I should have made that as interface. But doing with directly changing should be best because people extending RequestBuilderBase should always take extra care of their custom implementations.

[hyperxpro]

Fire this up so I can merge and cut a release.

[lhotari]

Is there any way to backport this to 2.12.x to maintain Java 8 support?

We use AsyncHttpClient 2.12.x in the Apache Pulsar Admin client. Our challenge in Apache Pulsar is that the minimum Java requirement for clients is Java 8. Since AsyncHttpClient 3.0.x requires Java 11, we don't have a way to eliminate CVE-2024-53990 from our dependencies. While it's possible to set the CookieStore to null in AHC 2.12.x (as we're doing in apache/pulsar#23725), the main issue for Apache Pulsar is that many users have security policies requiring the elimination of dependencies with high or critical vulnerabilities. Therefore, configuring the CookieStore isn't a sufficient mitigation for many of our users.

Any suggestions and recommendations for solving our use case in Apache Pulsar would be much appreciated.

[hyperxpro]

@lhotari Sure, yes. I will push the emergency release for 2.12.x.

[lhotari]

@lhotari Sure, yes. I will push the emergency release for 2.12.x.

Thank you, @hyperxpro. After the release, it would be great to have CVE-2024-53990 updated so that it would enlist the new release clear of the vulnerability so that vulnerability scanners would have the information too.

[hyperxpro]

Released 2.12.4; it will sync to Maven Central in some time :)

@pickypg Can you do PR in the advisory database to exclude 2.12.4?

[pickypg]

Can do!

[xuwei-k]

missing 2.12.4 tag on GitHub? 👀

[aidandempsey]

@hyperxpro Thanks for the fix, is there any chance this will be back ported to 2.12.3? Thanks!

[hyperxpro]

@hyperxpro Thanks for the fix, is there any chance this will be back ported to 2.12.3? Thanks!

2.12.4 is out

[hyperxpro]

missing 2.12.4 tag on GitHub? 👀

Added async-http-client-project-2.12.4

[xuwei-k]

thanks!

[pickypg]

Updated the GitHub Advisory: GHSA-mfj5-cf8g-g2fv