You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "adityamparikh (via GitHub)" <gi...@apache.org> on 2026/04/24 21:02:38 UTC
[PR] fix: add consistent collection name validation to all MCP tool methods [solr-mcp]
adityamparikh opened a new pull request, #108:
URL: https://github.com/apache/solr-mcp/pull/108
## Summary
- Add collection name blank/null validation to `checkHealth()`, `getCollectionStats()`, `search()`, `indexJsonDocuments()`, `indexCsvDocuments()`, `indexXmlDocuments()`, `getSchema()`, and `getSchemaResource()`
- Consistent `IllegalArgumentException` with clear message for invalid collection names
- Matches existing pattern in `createCollection()`
## Test plan
- [x] `./gradlew build` passes
- [x] `./gradlew nativeTest -Pnative` passes (119/119 tests)
- [x] No regressions
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] fix: validate collection name consistently across all MCP tool methods [solr-mcp]
Posted by "adityamparikh (via GitHub)" <gi...@apache.org>.
adityamparikh commented on PR #108:
URL: https://github.com/apache/solr-mcp/pull/108#issuecomment-5699600823
Closing this. Re-checked against `main` @ `7d920af`, and most of the rationale in the description doesn't hold up:
- **"a null collection silently targets the client's *default* collection"** — not true here. `SolrConfig.buildSolrClient` never calls `withDefaultCollection`, so there is no default to fall back to.
- **whitespace needs a guard** — it doesn't. `" "` encodes to `%20`, reaches Solr and 404s, and `SearchService.withRemediationHint` already turns that into the `list-collections` hint. Same for any unknown collection name.
What's actually left is much narrower: only the *empty* string degrades. `/solr//select` is rejected by Jetty during URI parsing, so it is a **400 `Ambiguous URI empty segment`** rather than a 404 — it misses the `NOT_FOUND` branch, carries none of the message tokens the other branches match on, and reaches the client raw.
| collection argument | HTTP | what the client sees |
|---|---|---|
| `""` | 400 | `Ambiguous URI empty segment` |
| `" "` | 404 | + `list-collections` hint |
| unknown name | 404 | + `list-collections` hint |
That is a real dead end, but a rare one, and not worth the shape this PR has taken: eleven call sites plus ~140 lines of Mockito tests that only exercise the shared helper through four service constructors, and add ten `@DisabledInNativeImage` skips to `nativeTest`.
If it proves worth fixing later, the cheaper place is `SolrClient.request(SolrRequest<?>, String)` — the single abstract funnel every operation goes through, and where `null` legitimately means "no collection targeted" while blank never is. One check there would cover every existing and future tool.
The one piece worth keeping is the de-duplication: the same message exists twice, as `CollectionService.BLANK_COLLECTION_NAME_ERROR` and as a private `SchemaService.requireCollection`. I'll fold that into the next PR that touches `SchemaService`.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] fix: validate collection name consistently across all MCP tool methods [solr-mcp]
Posted by "adityamparikh (via GitHub)" <gi...@apache.org>.
adityamparikh closed pull request #108: fix: validate collection name consistently across all MCP tool methods
URL: https://github.com/apache/solr-mcp/pull/108
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org