mrdevel2002-ui opened a new pull request, #3453:
URL: https://github.com/apache/tika/pull/3453

   `HttpFetcher.buildRequestConfig` (used by both `fetch` overloads) and 
`AtlassianJwtFetcher.fetch` put a `RequestConfig` built from 
`RequestConfig.custom()` on every GET so they can apply `maxRedirects`. 
HttpClient 4.5 uses a request's own config in place of the client's default 
instead of merging the two, so the `connectTimeoutMillis`, 
`socketTimeoutMillis` and `requestTimeoutMillis` that 
`HttpClientFactory.build()` installs as the default never reach a fetch. All 
three come out as -1: no connect timeout, no socket read timeout, and an 
unbounded wait for a pooled connection. A server that accepts the connection 
and then stalls holds the fetch until `overallTimeoutMillis` fires, and with 
`overallTimeoutMillis` set to -1 nothing bounds it. An operator who relies on 
the documented per-socket timeouts for large downloads would plausibly use that 
setting. The security model treats a limit that is accepted and then silently 
not enforced as a security issue, which is why I'm raising it h
 ere rather than as a plain bug. The same replacement also drops the factory's 
preferred auth schemes (BASIC, NTLM).
   
   `HttpClientFactory.getDefaultRequestConfig()` now returns the config 
`build()` installs, and both fetchers start their per-request config from 
`RequestConfig.copy(...)` of it, so the redirect settings are the only thing a 
request changes. I put the derivation in the factory rather than repeating the 
three timeout settings in each fetcher, so the fetchers cannot drift from what 
the client was built with. The cost is one new public getter in 
tika-httpclient-commons.
   
   `testSocketTimeoutAppliesToRequests` in `HttpFetcherTest` (whole-document 
and range fetch) and in `AtlassianJwtFetcherTest` points the fetcher at a 
loopback `ServerSocket` that completes the handshake and never answers, with 
`socketTimeoutMillis=500` and `overallTimeoutMillis=10000`. On main both fail 
after about 10s with `TikaTimeoutException: Overall timeout after 10000ms` 
instead of `SocketTimeoutException`. With the patch they get the 
`SocketTimeoutException` at 500ms. The mocked factory in `HttpFetcherTest` 
stubs the new getter. `./mvnw clean apache-rat:check install -Pci` of 
tika-httpclient-commons, tika-pipes-http and tika-pipes-atlassian-jwt (tests, 
checkstyle, spotless, forbidden-apis) is green, and tika-pipes-es, 
tika-pipes-opensearch and tika-pipes-solr still compile against the factory 
change.
   
   - [ ] open issue on the Tika issue tracker: not filed yet, happy to open one 
or have it linked
   - [ ] issue ID in the PR title and commit message: pending the issue above
   - [x] commits are squashed into a single one
   - [x] Tika builds and unit tests pass with `./mvnw clean install` (affected 
modules)
   - [x] generative AI tool used: the commit carries `Generated-by:` per the 
ASF Generative Tooling Guidance.
   - [x] no conflicts with the recent `main` branch
   - [ ] new downstream module added to `tika-bom/pom.xml`: not applicable
   


-- 
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]

Reply via email to