This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-mcp.git
The following commit(s) were added to refs/heads/main by this push:
new 7d920af8 fix(config): reject a solr.url SolrJ cannot connect to at
bind time (#104)
7d920af8 is described below
commit 7d920af801f991c0d4577fe4c43906dfb6d5d4a3
Author: Aditya Parikh <[email protected]>
AuthorDate: Wed Sep 16 10:47:48 2026 -0400
fix(config): reject a solr.url SolrJ cannot connect to at bind time (#104)
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.
Claude-Session: https://claude.ai/code/session_01Wh7SJkZhL1uuK7pYc3SLk8
Signed-off-by: Aditya Parikh <[email protected]>
Co-authored-by: Claude Fable 5.1 <[email protected]>
---
.../server/config/SolrConfigurationProperties.java | 47 +++++++++--
.../config/SolrConfigurationPropertiesTest.java | 93 ++++++++++++++++++++++
2 files changed, 135 insertions(+), 5 deletions(-)
diff --git
a/src/main/java/org/apache/solr/mcp/server/config/SolrConfigurationProperties.java
b/src/main/java/org/apache/solr/mcp/server/config/SolrConfigurationProperties.java
index a0e15357..bb484fe1 100644
---
a/src/main/java/org/apache/solr/mcp/server/config/SolrConfigurationProperties.java
+++
b/src/main/java/org/apache/solr/mcp/server/config/SolrConfigurationProperties.java
@@ -16,8 +16,10 @@
*/
package org.apache.solr.mcp.server.config;
+import java.net.URI;
import org.jspecify.annotations.Nullable;
import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.util.StringUtils;
/**
* Spring Boot Configuration Properties record for Apache Solr connection
@@ -101,12 +103,17 @@ import
org.springframework.boot.context.properties.ConfigurationProperties;
* that requires Solr connection information.
*
* <p>
- * <strong>Validation Considerations:</strong>
+ * <strong>Validation:</strong>
*
* <p>
- * While basic validation is handled by the configuration system, additional
URL
- * validation and normalization occurs in the {@link SolrConfig} class during
- * SolrClient bean creation.
+ * {@code url} must be an absolute {@code http} or {@code https} URL with a
+ * host. The compact constructor enforces this at bind time, so a misconfigured
+ * deployment fails at startup with an actionable message instead of at first
+ * request with an opaque SolrJ error. {@code localhost:8983} (scheme omitted)
+ * is the easy mistake: {@code java.net.URI} parses it as scheme
+ * {@code localhost} with no host, and path normalization in {@link SolrConfig}
+ * would otherwise concatenate it without noticing. Path normalization (adding
+ * {@code /solr/}) still happens in {@link SolrConfig}.
*
* <p>
* <strong>Optional Basic Authentication:</strong>
@@ -120,7 +127,8 @@ import
org.springframework.boot.context.properties.ConfigurationProperties;
* header is attached and the client behaves as before.
*
* @param url
- * the base URL of the Apache Solr server (required, non-null)
+ * the base URL of the Apache Solr server; an absolute http(s) URL
+ * with a host
* @param username
* the HTTP Basic Authentication username (optional; required
* together with {@code password} to enable auth)
@@ -133,4 +141,33 @@ import
org.springframework.boot.context.properties.ConfigurationProperties;
*/
@ConfigurationProperties(prefix = "solr")
public record SolrConfigurationProperties(String url, @Nullable String
username, @Nullable String password) {
+
+ private static final String HTTP = "http";
+
+ private static final String HTTPS = "https";
+
+ public SolrConfigurationProperties {
+ if (!isAbsoluteHttpUrlWithHost(url)) {
+ throw new IllegalArgumentException("solr.url must be an
absolute http or https URL including a host, "
+ + "for example
http://localhost:8983/solr/ (was: '" + url + "')");
+ }
+ }
+
+ /**
+ * The binder hands over whatever was configured, including nothing at
all, so
+ * this is the one place a null or blank value can arrive.
+ */
+ private static boolean isAbsoluteHttpUrlWithHost(@Nullable String url) {
+ if (!StringUtils.hasText(url)) {
+ return false;
+ }
+ URI uri;
+ try {
+ uri = URI.create(url);
+ } catch (IllegalArgumentException ex) {
+ return false;
+ }
+ String scheme = uri.getScheme();
+ return (HTTP.equals(scheme) || HTTPS.equals(scheme)) &&
StringUtils.hasText(uri.getHost());
+ }
}
diff --git
a/src/test/java/org/apache/solr/mcp/server/config/SolrConfigurationPropertiesTest.java
b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigurationPropertiesTest.java
new file mode 100644
index 00000000..d4d4e176
--- /dev/null
+++
b/src/test/java/org/apache/solr/mcp/server/config/SolrConfigurationPropertiesTest.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.mcp.server.config;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledInNativeImage;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import
org.springframework.boot.context.properties.EnableConfigurationProperties;
+import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+
+/**
+ * {@code solr.url} is validated where the binder hands it over, in the
record's
+ * compact constructor, so a misconfigured deployment dies at startup with an
+ * actionable message instead of at first request with an opaque SolrJ error.
+ *
+ * <p>
+ * {@code URI.create("localhost:8983")} happily parses as scheme
+ * {@code localhost} with no host, and {@link SolrConfig} would normalize it by
+ * string concatenation without noticing; that is the case that motivates the
+ * check.
+ */
+class SolrConfigurationPropertiesTest {
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {"http://localhost:8983";,
"http://localhost:8983/";, "http://localhost:8983/solr";,
+ "http://localhost:8983/solr/";,
"https://solr.internal:8983/custom/solr/";,
+ "https://solr.example.com";,
"http://solr:8983/solr/"})
+ void acceptsAbsoluteHttpUrlsWithAHost(String url) {
+ assertThat(new SolrConfigurationProperties(url, null,
null).url()).isEqualTo(url);
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {"localhost:8983", "solr.example.com",
"/solr", "ftp://solr.example.com/solr";, "file:///var/solr",
+ "not a url", "http://";, "", " "})
+ void rejectsUrlsSolrJCannotConnectTo(String url) {
+ assertThatThrownBy(() -> new SolrConfigurationProperties(url,
null, null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("solr.url must be an
absolute http or https URL including a host")
+
.hasMessageContaining("http://localhost:8983/solr/";);
+ }
+
+ /** The binder passes null when the property is absent altogether. */
+ @Test
+ void rejectsAMissingUrl() {
+ assertThatThrownBy(() -> new SolrConfigurationProperties(null,
null, null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("solr.url must be an
absolute http or https URL including a host");
+ }
+
+ @Test
+ @DisabledInNativeImage
+ void bindingFailsAtStartupWhenTheSchemeIsMissing() {
+
contextRunner().withPropertyValues("solr.url=localhost:8983").run(context -> {
+ assertThat(context).hasFailed();
+
assertThat(context).getFailure().rootCause().hasMessageContaining("solr.url
must be an absolute");
+ });
+ }
+
+ @Test
+ @DisabledInNativeImage
+ void bindingSucceedsForAnAbsoluteUrl() {
+
contextRunner().withPropertyValues("solr.url=http://localhost:8983/solr/";)
+ .run(context ->
assertThat(context).hasNotFailed());
+ }
+
+ private static ApplicationContextRunner contextRunner() {
+ return new
ApplicationContextRunner().withUserConfiguration(PropertiesOnly.class);
+ }
+
+ @EnableConfigurationProperties(SolrConfigurationProperties.class)
+ static class PropertiesOnly {
+ }
+}
