epugh commented on PR #4665:
URL: https://github.com/apache/solr/pull/4665#issuecomment-5132425766
I asked Claude to trace it, and it was a bit deeper, but yeah.. It's the
tightening up.... I don't know if what Claude suggested about making some
chagnes elsewhere....
I also keep wondering if this has an intersection with `qt` and our attempt
to remove it...
## Root cause
PR #4665 added a guard to `SolrRequest.setPath()` that rejects `null`. The
reindex step in the test (`BasicAuthIntegrationTest.java:322`) drives a
server-side streaming expression, and `SolrStream` calls `setPath` with a
`null` path:
```
Caused by: java.lang.IllegalArgumentException: Must start with a '/': null
at org.apache.solr.client.solrj.SolrRequest.setPath(SolrRequest.java:186)
at ...SolrStream.constructParser(SolrStream.java:289) ←
query.setPath(p)
at ...SolrStream.open(SolrStream.java:126)
at ...CloudSolrStream$StreamOpener.call(CloudSolrStream.java:527)
```
**The chain:**
1. `SolrStream.constructParser()` line 273: `String p =
requestParams.get("qt")` → `null` (no `qt` param).
2. `core == null`, so line 289 runs `query.setPath(p)` = `setPath(null)`.
3. Old behavior: harmless (path stayed `null`, defaulted downstream). New
behavior: throws `IllegalArgumentException`.
4. The stream open fails on every shard → reindex daemon never completes →
the client's reindex request **times out** (the `TimeoutException` you see at
the surface at line 322).
So the visible symptom (client `TimeoutException`) is a red herring; the
real failure is the `setPath(null)` guard firing inside server-side streaming.
## Fix options
**Recommended — fix the caller in `SolrStream.java:289`** so it never passes
`null`:
```java
} else {
query.setPath(p != null ? p : "/select");
}
```
This mirrors the `core != null` branch (line 287), which already defaults to
`/select`, and restores the prior effective behavior.
**Alternative — soften the PR's guard** to permit `null` (only reject a
non-null slash-less path). But since David added path enforcement deliberately,
tightening the callers is the cleaner direction — and there's a sibling call at
`JSONTupleStream.java:63` (`query.setPath(p)`) worth auditing for the same
`null` risk.
Want me to apply the `SolrStream` fix (and check `JSONTupleStream`)? I'd
re-run `testBasicAuth` to confirm green — though note the run takes ~2 min and
there's a `-Ptests.jvmargs` / security-manager config in the reproduce line I'd
want to match.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
