From 52ff44252109246476fd1df06d758abe807534b7 Mon Sep 17 00:00:00 2001 From: Aditya Parikh Date: Sat, 12 Sep 2026 12:03:14 -0400 Subject: [PATCH] fix(config): reject a solr.url SolrJ cannot connect to at bind time 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 Claude-Session: https://claude.ai/code/session_01Wh7SJkZhL1uuK7pYc3SLk8 Signed-off-by: Aditya Parikh --- .../config/SolrConfigurationProperties.java | 47 +++++++++- .../SolrConfigurationPropertiesTest.java | 93 +++++++++++++++++++ 2 files changed, 135 insertions(+), 5 deletions(-) create mode 100644 src/test/java/org/apache/solr/mcp/server/config/SolrConfigurationPropertiesTest.java 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 @@ * that requires Solr connection information. * *

- * Validation Considerations: + * Validation: * *

- * 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}. * *

* Optional Basic Authentication: @@ -120,7 +127,8 @@ * 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 @@ */ @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. + * + *

+ * {@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 { + } +}