[SCORP711]

Fixes #2211.

Migrates core to HttpClient 5.6.4 and aligns HttpCore dependencies at
5.4.3. Updates the affected imports in core, Tika and WARC.

Preserves query parsing with both & and ; separators. Cookie expiry
handling now uses Instant, with a fallback for legacy date formats.
Adds regression tests and documents the public Cookie type change.

Validation: all 18 modules passed clean verify on Ubuntu/WSL with
Java 25, Maven 3.9.16 and -DCI_ENV=true.
1,050 tests passed, 15 were skipped by existing environment conditions,
and there were no failures or errors.

Thank you for contributing to Apache StormCrawler.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes

• Is there a issue associated with this PR? Is it referenced in the commit message?

• Does your PR title start with #XXXX where XXXX is the issue number you are trying to resolve?

• Has your PR been rebased against the latest commit within the target branch (typically main)?

• Is your initial contribution a single, squashed commit?

• Is the code properly formatted with mvn git-code-format:format-code -Dgcf.globPattern="**/*" -Dskip.format.code=false?

For code changes

• Have you ensured that the full suite of tests is executed via mvn clean verify?
• Have you written or updated unit tests to verify your changes?
• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
• If applicable, have you updated the LICENSE file, including the main LICENSE file?
• If applicable, have you updated the NOTICE file, including the main NOTICE file?

Note

Please ensure that once the PR is submitted, you check GitHub Actions for build issues and submit an update to your PR as soon as possible.

[jnioche]

Thanks for this, the migration looks solid. Here are some AI assisted findings

I checked HttpClient 4 and HttpClient 5 against each other with the actual jars:

• Query parsing and formatting with '&', ';' give identical output on a set of tricky queries, so normalised URLs don't change.
• All 20 HttpHeaders/HttpStatus constants used in core, tika and warc have the same string values.
• CookieConverterTest (61) and BasicURLNormalizerTest (21) pass.
• Dependency trees: compared with the base commit, the only changes are the expected ones.

A few comments, most important first:

1. BasicURLNormalizer: pin the format encoding with a test
The whole org.apache.hc.core5.net.URLEncodedUtils class is deprecated in httpcore5 5.4. The code depends on it for parse and also for format. The comment only mentions that WWWFormCodec doesn't treat ; as a separator, but WWWFormCodec.format also encodes differently: it leaves ~ as is and encodes * as %2A. For a=~!'()*, URLEncodedUtils.format gives a=%7E%21%27%28%29* and WWWFormCodec gives a=~%21%27%28%29%2A. If someone later "fixes" the deprecation warning, normalised URLs change silently and we get duplicates in the status index. Could you add a test with ?a=~!'()* and extend the comment to say that format has to stay as well?

2. CookieConverter.getCookies: public API still exposes an HttpClient type
We're breaking the API anyway, but the change swaps one HttpClient Cookie for another. The only caller (okhttp HttpProtocol) uses just getName()/getValue(). Returning a StormCrawler-owned type, or java.net.HttpCookie, would decouple us from HttpComponents. Core could then drop httpclient5 and keep only httpcore5 for the constants. Otherwise the next HttpComponents major upgrade breaks this API again. This could be a follow-up, but 4.0.0 is the natural time to do it.

3. Root dependencyManagement pins override module versions
Pinning httpclient5/httpcore5/httpcore5-h2 at the root forces those versions on every module, including opensearch-java (via opensearch-java 3.10.0) and aws (via awssdk apache5-client). The versions happen to match today. When Dependabot bumps one of those clients to a release that needs a newer HC5, the pin will silently downgrade it, which risks a NoSuchMethodError at runtime.

4. CookieConverter.parseCookieExpiry: trying HttpClient 5 first changes some results
The legacy fallback alone reproduces HttpClient 4's DateUtils.parseDate exactly: same patterns, quote trimming, 2000 pivot and leniency. Because parseStandardDate runs first, a few inputs now give different results:

• Wed, 09 Jun 2021 10:18:14 PST gives 17:18:14Z (HttpClient 5 applies daylight saving time) instead of 18:18:14Z.
• Wed, 09 Jun 21 10:18:14 GMT gives 0021-06-09 instead of 0021-06-07.

parseStandardDate also never parses the asctime format (it has no time zone, so it returns null), and every miss throws and catches three exceptions. I'd suggest using only the legacy path, or documenting the difference.

5. parseCookieExpiry: per-call allocations
Every fallback call creates the patterns array and three SimpleDateFormat instances, and it re-parses Instant.parse("2000-01-01T00:00:00Z") on each loop iteration. With http.use.cookies=true this runs for every request. HttpClient 4 cached its formatters per thread. Moving the patterns and the pivot Date into static final fields would help.

6. pom.xml: use version properties
Every other managed dependency uses a ${...version} property. httpcore5 and httpcore5-h2 must stay in lockstep, but with two literal 5.4.3 values Dependabot will open separate PRs, which can leave them mismatched. A shared httpcore5.version property, plus httpclient5.version, would keep them together.

7. core/pom.xml: unused httpcore5-h2
httpclient5 brings in httpcore5-h2, the HTTP/2 implementation, which core never uses, so it ends up in every topology uber-jar. Consider excluding it, or see point 2.

8. README
The "Migrating to 4.0.0" section sits in the build instructions of the top-level README. Its second paragraph describes things that did not change, so users have nothing to do about them. These notes probably belong in the release notes or docs instead.

Minor: the PR checklist says the issue is referenced in the commit message, but the commit message doesn't mention #2211.

[rzo1]

@jnioche Would it make to just fork the classes we need into SC itself? We could drop the dependency than and are not dependend on it just for the sake of a few utility classes. wdyt?

[jnioche]

@jnioche Would it make to just fork the classes we need into SC itself? We could drop the dependency than and are not dependend on it just for the sake of a few utility classes. wdyt?

yes, they are likely to be very stable as well. At least rely on http-core only, which in practice would be pulled from some of the external modules anyway. Or go the whole way and have our own utility classes for it all

[rzo1]

yep - would opt for just forking the classes we need...

[SCORP711]

@jnioche Thanks for the detailed review. I’ve addressed the eight points and pushed the updated commit, which now references #2211. Core now depends only on HttpCore 5.

[dpol1]

httpcore5-only looks fine to me for 4.0.0. After this change no HttpComponents type is left in core's public API, so copying the few classes can come in a follow-up without another API break. @jnioche @rzo1, ok with that?

[rzo1]

httpcore5-only looks fine to me for 4.0.0. After this change no HttpComponents type is left in core's public API, so copying the few classes can come in a follow-up without another API break. @jnioche @rzo1, ok with that?

I would avoid having the API dependency at all. 4.0.0 is a possiblity to break things, which is hard to argue for any subsequent dot release actually. We dropped the Apache HTTP Protocol Implementation for 4.0.0, in 4d49466, so I don't see any value in having any dependency to Apache HTTP components as left over in the project ;-)

That being said, I am -0 here ;-) - so if ppl want to move forward with this PR, feel free.

[jnioche]

httpcore5-only looks fine to me for 4.0.0. After this change no HttpComponents type is left in core's public API, so copying the few classes can come in a follow-up without another API break. @jnioche @rzo1, ok with that?

I would avoid having the API dependency at all. 4.0.0 is a possiblity to break things, which is hard to argue for any subsequent dot release actually. We dropped the Apache HTTP Protocol Implementation for 4.0.0, in 4d49466, so I don't see any value in having any dependency to Apache HTTP components as left over in the project ;-)

That being said, I am -0 here ;-) - so if ppl want to move forward with this PR, feel free.

We can't change the class for HTTP Headers later within 4.x without potentially breaking user code so now would be a good time to bite the bullet and invest time in copying what we need to SC.

The HttpHeaders class is a gloriously simple list of public static Strings, HttpStatus is similar.

@SCORP711 any chance this could be done within this PR?

[SCORP711]

Yep, I’ll look into it.