Skip to content

fix(config): reject a solr.url SolrJ cannot connect to at bind time - #104

Merged
epugh merged 1 commit into
apache:mainfrom
adityamparikh:fix/solr-config-properties-validation
Sep 16, 2026
Merged

epugh merged 1 commit into
apache:mainfrom
adityamparikh:fix/solr-config-properties-validation

Conversation

@adityamparikh

@adityamparikh adityamparikh commented Apr 24, 2026 •

Copy link
Copy Markdown
Contributor

Problem

solr.url is bound into a record and handed to SolrJ. Nothing checks that it is a URL SolrJ can use, so a misconfigured deployment starts cleanly and fails at first request with an opaque client error. The easy mistake is omitting the scheme:

solr.url=localhost:8983
  → URI.create(...) parses scheme="localhost", host=null
  → SolrConfig normalizes the *path* by string concatenation, never noticing
  → SolrJ: "localhost:8983/solr/" is not a URL it can connect to

Change

SolrConfigurationProperties gets a compact constructor that rejects anything but an absolute http/https URL with a host, with a message that names the property, shows an example and echoes the offending value:

Accepted Rejected
http://localhost:8983 localhost:8983 (scheme localhost, no host)
http://localhost:8983/solr/ solr.example.com, /solr (not absolute)
https://solr.internal:8983/custom/solr/ ftp://…, file://… (SolrJ cannot speak them)
http://solr:8983/solr/ not a url, http://, empty, blank, absent

Boot surfaces the IllegalArgumentException through its bind failure analyzer, so startup prints Failed to bind properties under 'solr' with the reason.

Why the constructor and not Bean Validation. Hibernate Validator is not on the classpath; adding it for one field means a new runtime dependency in the native image plus two classes for a custom constraint, and @NotBlank/@URL alone cannot express "http(s) with a host" (Hibernate's @URL accepts ftp:// and file://). A record's compact constructor is the plain-Java place for an invariant, and the binder invokes it reflectively with null when the property is absent, so the one hasText check is the guard for that boundary. No network I/O; native-safe as-is.

Path normalization (appending /solr/) is unchanged and still lives in SolrConfig.

Verification

SolrConfigurationPropertiesTest: 19 cases over accepted URLs, rejected values, the absent value, and two ApplicationContextRunner bind tests. Those two are @DisabledInNativeImage because that runner builds its context through a JDK dynamic proxy; the constructor itself runs natively. ./gradlew build on JDK 25 against main @ b4ffe18: green.

🤖 Generated with Claude Code

@adityamparikh
adityamparikh force-pushed the fix/solr-config-properties-validation branch from a3f6b1a to b2c8af9 Compare May 2, 2026 17:04
@adityamparikh
adityamparikh force-pushed the fix/solr-config-properties-validation branch from b2c8af9 to 184bdd3 Compare June 15, 2026 16:42
@adityamparikh
adityamparikh force-pushed the fix/solr-config-properties-validation branch 2 times, most recently from b1c961d to 0f3e043 Compare August 19, 2026 11:58
@adityamparikh
adityamparikh force-pushed the fix/solr-config-properties-validation branch from 777168e to b4ffe18 Compare September 12, 2026 16:03
@adityamparikh adityamparikh changed the title fix: add fail-fast validation to SolrConfigurationProperties fix(config): reject a solr.url SolrJ cannot connect to at bind time Sep 12, 2026
A solr.url without a scheme (localhost:8983) binds cleanly: java.net.URI
parses it as scheme "localhost" with no host, SolrConfig normalizes the path
by concatenation without noticing, and the deployment fails at first request
with an opaque SolrJ error instead of at startup.

The record's compact constructor now requires an absolute http or https URL
with a host and reports the property name, an example and the offending
value; Boot's bind failure analyzer surfaces that at startup. No Bean
Validation: Hibernate Validator is not on the classpath and a custom
constraint would cost a dependency and two classes for one field. The
binder invokes the constructor reflectively and passes null when the
property is absent, so the hasText check here is the guard for that
boundary.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wh7SJkZhL1uuK7pYc3SLk8
Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
@adityamparikh adityamparikh reopened this Sep 12, 2026
@epugh
epugh merged commit 7d920af into apache:main Sep 16, 2026
4 checks passed
adityamparikh referenced this pull request in adityamparikh/solr-mcp Sep 16, 2026
…103 #104 apache#205) into sb4

Resolves the conflicts against main and ports what merged cleanly but
would not compile on the Boot 4 / Jackson 3 line:

- gradle/libs.versions.toml: keep the Boot 4 test starters and the
  mcp-sdk pin, add spring-security-test (apache#187), drop commons-csv (apache#205).
- JsonDocumentCreator: take apache#202's flatten()/toDocument() split with
  Jackson 3 imports and JacksonException.
- CollectionServiceIntegrationTest / IndexingServiceIntegrationTest:
  take main's typed-documents and autowired-creator setup.
- TestDocuments, JsonDocumentCreatorTest (new on main): com.fasterxml ->
  tools.jackson, JsonMapper.builder().build(), JacksonException.
- IndexingServiceIntegrationTest: add the assertThrows static import
  that sb4's explicit-import expansion left out for apache#205's new XML test.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
@adityamparikh
adityamparikh deleted the fix/solr-config-properties-validation branch October 7, 2026 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants