fix: validate collection name consistently across all MCP tool methods - #108
Open
adityamparikh wants to merge 3 commits into
Open
adityamparikh wants to merge 3 commits into
adityamparikh wants to merge 3 commits into
Conversation
adityamparikh
force-pushed
the
fix/unified-collection-validation
branch
from
May 2, 2026 17:04
14c39e5 to
d8ddc16
Compare
adityamparikh
force-pushed
the
fix/unified-collection-validation
branch
from
June 14, 2026 03:39
d8ddc16 to
2c77c92
Compare
adityamparikh
force-pushed
the
fix/unified-collection-validation
branch
from
August 18, 2026 21:25
2c77c92 to
d918d29
Compare
adityamparikh
changed the title
adityamparikh
force-pushed
the
fix/unified-collection-validation
branch
from
August 18, 2026 21:31
d918d29 to
5f0eb18
Compare
Only createCollection and the two schema-modification tools validated the
collection name. checkHealth, getCollectionStats, search, the three
index*Documents tools, getSchema and getSchemaResource accepted null or blank
and failed downstream — a null collection reaches SolrJ and silently targets
the client's default collection rather than reporting a bad argument.
The message was already duplicated on main: a BLANK_COLLECTION_NAME_ERROR
constant in CollectionService and a copied string literal in SchemaService's
private requireCollection. Extract one shared
ToolArguments.requireCollection(String) in the util package and route all
eleven call sites through it, so the four services cannot drift apart.
Message wording is aligned with the document-creator family
("<Subject> cannot be null or empty"), which also covers SchemaService's
sibling requireNonEmpty helper.
The null half of the check is deliberate rather than redundant defensive
coding: the package is @NullMarked with NullAway as a build error, but that
analysis only binds callers the compiler can see. MCP tool methods are invoked
reflectively by the Spring AI annotation runtime, which resolves each parameter
with a plain lookup against the request's argument map and passes the result
straight through — a missing or null JSON value therefore arrives as null no
matter what the annotations declare. @McpToolParam(required = true) only marks
the parameter required in the advertised JSON schema; the server does not
validate incoming arguments against it.
CollectionNameValidationTest asserts all nine entry points against the shared
constant rather than a copied literal.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
adityamparikh
force-pushed
the
fix/unified-collection-validation
branch
from
August 19, 2026 11:58
5f0eb18 to
98c8ece
Compare
main gained indexMarkdownDocuments after this branch's merge base; it is now guarded like the other tools and covered by CollectionNameValidationTest. Reverts an unrelated rewording of SchemaService.requireNonEmpty. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiUHyyXLTo9ATdgg8eRFZJ Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Only
createCollectionand the two schema-modification tools validated the collection name.checkHealth,getCollectionStats,search, the fourindex*Documentstools,getSchemaandgetSchemaResourceaccepted null or blank and failed downstream, and a null collection reaching SolrJ silently targets the client's default collection rather than reporting a bad argument. The message was also duplicated: a constant inCollectionServiceand a copied literal inSchemaService.Changes
ToolArguments.requireCollection(String)in theutilpackage, called by all twelve entry points, so the four services cannot drift apart.SchemaService's private duplicate is removed."Collection name cannot be null or empty".CollectionServicecreateCollection*,getCollectionStats,checkHealthIndexingServiceindexJsonDocuments,indexCsvDocuments,indexXmlDocuments,indexMarkdownDocumentsSchemaServicegetSchema,getSchemaResource,addFields,addFieldTypesSearchServicesearch* already validated on
main; switched to the shared helper.Why a runtime null check in
@NullMarkedcodeNullAway is a compile-time analysis and only binds callers the compiler can see. MCP tool methods are invoked reflectively by the Spring AI annotation runtime, which looks each parameter up in the request's argument map and passes
nullthrough for a missing value.@McpToolParam(required = true)only marks the parameter required in the advertised JSON schema; the MCP SDK onmain(0.18) does not validate incoming arguments against it. The rationale is recorded onToolArguments.Verification
CollectionNameValidationTest(10 tests) asserts every entry point against the shared constant rather than a copied literal../gradlew buildon Java 25: 413 tests, 0 failures, 7 skipped (the OTLP suite, skipped onmainuntil test(observability): re-enable OtlpExportIntegrationTest #198).Touches the same
SchemaService.getSchemalines as #98; whichever merges second needs a one-line rebase.🤖 Generated with Claude Code
https://claude.ai/code/session_01CiUHyyXLTo9ATdgg8eRFZJ