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