jnioche opened a new issue, #2211:
URL: https://github.com/apache/stormcrawler/issues/2211
### Context
`stormcrawler-core` depends on
`org.apache.httpcomponents:httpclient:4.5.14`, which brings in `httpcore`
4.4.16 and `commons-logging` 1.2. Core no longer uses HttpClient to fetch
anything. The remaining usages are header/status constants, cookie handling and
query-string parsing. HttpComponents 4.x is in maintenance mode, and
`external/opensearch-java` already uses `httpclient5`/`httpcore5`.
This issue proposes moving core to
`org.apache.httpcomponents.client5:httpclient5` (5.6).
### Current usages
| Class | From | Where |
|---|---|---|
| `org.apache.http.HttpHeaders` | httpcore | 13 core files (FetcherBolt,
SimpleFetcherBolt, JSoupParserBolt, FeedParserBolt, SiteMapParserBolt,
HttpRobotRulesParser, AdaptiveScheduler, CharsetIdentification, FileResponse,
okhttp `HttpProtocol`, …), plus `external/tika` ParserBolt and `external/warc`
WARCRecordFormat (via core, transitively) |
| `org.apache.http.HttpStatus` | httpcore | `FileResponse` |
| `org.apache.http.NameValuePair` | httpcore | `BasicURLNormalizer` |
| `org.apache.http.client.utils.URLEncodedUtils` | httpclient |
`BasicURLNormalizer` (query param sorting) |
| `org.apache.http.cookie.Cookie`, `impl.cookie.BasicClientCookie` |
httpclient | `CookieConverter`, okhttp `HttpProtocol`, `CookieConverterTest` |
| `org.apache.http.client.utils.DateUtils` (fully qualified, not imported) |
httpclient | `CookieConverter` (cookie expiry parsing) |
### Changes required
#### 1. Import-only changes (no behaviour change)
- `org.apache.http.{HttpHeaders, HttpStatus, NameValuePair}` →
`org.apache.hc.core5.http.*`
- Checked against httpcore5 5.4.3: every constant we use exists in 5.x with
the same name and value. That includes `SC_METHOD_FAILURE`, `ETAG`,
`IF_NONE_MATCH`, `IF_MODIFIED_SINCE`, `PROXY_AUTHORIZATION` and `LAST_MODIFIED`.
- `external/tika` and `external/warc` must update their `HttpHeaders`
imports.
#### 2. `BasicURLNormalizer`: ⚠️ query parsing behaviour differs
`URLEncodedUtils` is deprecated in 5.x. Its suggested replacement,
`WWWFormCodec`, does **not** treat `;` as a parameter separator, while 4.x
`URLEncodedUtils.parse(String, Charset)` does:
```
4.x URLEncodedUtils: b=1;a=2&c=x+y%20z&d -> b=1&a=2&c=x+y+z&d
5.x WWWFormCodec: b=1;a=2&c=x+y%20z&d -> b=1%3Ba%3D2&c=x+y+z&d
```
Switching naively would change normalised URLs, and so the keys of
already-crawled URLs, which leads to duplicates. Options:
- use `org.apache.hc.core5.net.URLEncodedUtils.parse(query, UTF_8, '&',
';')` (deprecated but keeps current behaviour), or
- write a small in-house parser/formatter.
Either way, add a test covering `;`-separated parameters.
#### 3. `CookieConverter`: ⚠️ public API change
- `Cookie` / `BasicClientCookie` →
`org.apache.hc.client5.http.cookie.Cookie` /
`org.apache.hc.client5.http.impl.cookie.BasicClientCookie`
- `DateUtils.parseDate(String)` →
`org.apache.hc.client5.http.utils.DateUtils.parseStandardDate(String)` (returns
`Instant`)
- `setExpiryDate(Date)` / `isExpired(Date)` → their `Instant` versions
- `CookieConverter.getCookies(...)` returns `List<Cookie>`, so **its
signature changes**. This is a breaking change for anyone calling it.
#### 4. okhttp `HttpProtocol`, `CookieConverterTest`
Import changes only. They only use `getName()` / `getValue()`.
### Not affected / side effects
- **`external/opensearch`:** its `org.apache.http.*` usages (`HttpHost`,
`AuthScope`, `CredentialsProvider`, SSL classes) come from
`opensearch-rest-client`, which keeps bringing in HttpClient 4.x.
- **`external/opensearch-java`:** already on 5.x. Align
`httpclient5`/`httpcore5` versions in `dependencyManagement` to avoid two 5.x
versions on the classpath.
- **Logging:** HttpClient 5 logs through SLF4J, so `commons-logging` drops
out of core's dependencies.
- **Archetype:** the flux-core shade filter excluding `org/apache/http/**`
becomes irrelevant but harmless.
- **Housekeeping:** regenerate `THIRD-PARTY.txt` / LICENSE.
### Compatibility
Breaking for:
- callers of `CookieConverter.getCookies(...)`
- user code that relied on core bringing in `org.apache.http.*` classes
transitively
Suggest targeting **4.0.0** and adding a note to the release notes.
### Related (separate issue?)
`MD5SignatureParseFilter`, `AbstractIndexerBolt` and
`AbstractStatusUpdaterBolt` use `commons-codec`, but core only gets it
transitively via `commons-compress`. It should be declared explicitly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
