From e7dda8687fbfbaac7f7ada6cec2ff28301af41b5 Mon Sep 17 00:00:00 2001 From: Gus Heck <46900717+gus-asf@users.noreply.github.com> Date: Sat, 27 Dec 2025 12:39:18 -0500 Subject: [PATCH] SOLR-18041 PathExclusionFilter extracted from SolrDispatchFilter (#3981) Collects and simplify all path exclusion logic into a single class configured in web.xml and makes our reliance on jetty's DefaultServlet (or whatever is configured with the name 'default') for serving static content explicit rather than an implicit feature one has to figure out (or already know about). (cherry picked from commit 58e11d8457f51a52e31267cc7b942cdd0a10c5f5) --- changelog/unreleased/SOLR-18041.yml | 8 ++ .../org/apache/solr/servlet/PathExcluder.java | 28 ------- .../solr/servlet/PathExclusionFilter.java | 80 +++++++++++++++++++ .../org/apache/solr/servlet/ServletUtils.java | 44 ---------- .../solr/servlet/SolrDispatchFilter.java | 13 +-- .../apache/solr/embedded/JettySolrRunner.java | 29 ++++++- solr/webapp/web/WEB-INF/web.xml | 18 ++++- 7 files changed, 129 insertions(+), 91 deletions(-) create mode 100644 changelog/unreleased/SOLR-18041.yml delete mode 100644 solr/core/src/java/org/apache/solr/servlet/PathExcluder.java create mode 100644 solr/core/src/java/org/apache/solr/servlet/PathExclusionFilter.java diff --git a/changelog/unreleased/SOLR-18041.yml b/changelog/unreleased/SOLR-18041.yml new file mode 100644 index 00000000000..d36eaad3ee5 --- /dev/null +++ b/changelog/unreleased/SOLR-18041.yml @@ -0,0 +1,8 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: SOLR 18041 - Path exclusions for the admin UI are now defined in a separate servlet filter +type: other # added, changed, fixed, deprecated, removed, dependency_update, security, other +authors: + - name: Gus Heck +links: + - name: SOLR-18041 + url: https://issues.apache.org/jira/browse/SOLR-18041 diff --git a/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java b/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java deleted file mode 100644 index 47d5b5c9578..00000000000 --- a/solr/core/src/java/org/apache/solr/servlet/PathExcluder.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * 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.servlet; - -import java.util.List; -import java.util.regex.Pattern; - -/** - * Denotes an object, usually a servlet that denies access to some paths based on the supplied - * patterns. Typically, this would be implemented via compiled regular expressions. - */ -public interface PathExcluder { - void setExcludePatterns(List excludePatterns); -} diff --git a/solr/core/src/java/org/apache/solr/servlet/PathExclusionFilter.java b/solr/core/src/java/org/apache/solr/servlet/PathExclusionFilter.java new file mode 100644 index 00000000000..87e1e195a21 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/servlet/PathExclusionFilter.java @@ -0,0 +1,80 @@ +/* + * 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.servlet; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.FilterConfig; +import jakarta.servlet.RequestDispatcher; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpFilter; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Filter to identify paths that should be processed by Jetty's DefaultServlet. Typically, these + * paths contain static resources that need to be returned verbatim, with appropriate content type, + * which Jetty will determine via + * + *

{@code org.eclipse.jetty.http.MimeTypes#getMimeByExtension(java.lang.String)} + */ +public class PathExclusionFilter extends HttpFilter { + + private List excludePatterns; + + boolean shouldBeExcluded(HttpServletRequest request) { + String requestPath = ServletUtils.getPathAfterContext(request); + if (excludePatterns != null) { + return excludePatterns.stream().map(p -> p.matcher(requestPath)).anyMatch(Matcher::lookingAt); + } + return false; + } + + @Override + public void init(FilterConfig config) throws ServletException { + String patternConfig = config.getInitParameter("excludePatterns"); + if (patternConfig != null) { + String[] excludeArray = patternConfig.split(","); + this.excludePatterns = Arrays.stream(excludeArray).map(Pattern::compile).toList(); + } + super.init(config); + } + + @Override + protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain) + throws IOException, ServletException { + if (shouldBeExcluded(req)) { + // N.B. "default" is the name for org.eclipse.jetty.ee10.servlet.DefaultServlet + // configured in solr/server/etc/webdefault.xml if it doesn't exist something is + // very wrong. + RequestDispatcher defaultServlet = req.getServletContext().getNamedDispatcher("default"); + if (defaultServlet == null) { + res.sendError( + 500, + "Server Misconfiguration: cannot find default servlet (normally defined as org.eclipse.jetty.ee10.servlet.DefaultServlet in webdefault.xml)"); + } else { + defaultServlet.forward(req, res); + } + } else { + chain.doFilter(req, res); + } + } +} diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java index e02f6802f93..f63a16187d2 100644 --- a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java +++ b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java @@ -19,7 +19,6 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import jakarta.servlet.FilterChain; import jakarta.servlet.ReadListener; import jakarta.servlet.ServletException; import jakarta.servlet.ServletInputStream; @@ -33,10 +32,6 @@ import java.io.InputStream; import java.io.OutputStream; import java.lang.invoke.MethodHandles; -import java.util.ArrayList; -import java.util.List; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.util.Utils; @@ -131,45 +126,6 @@ public void close() { }; } - static boolean excludedPath( - List excludePatterns, - HttpServletRequest request, - HttpServletResponse response, - FilterChain chain) - throws IOException, ServletException { - String requestPath = getPathAfterContext(request); - // No need to even create the HttpSolrCall object if this path is excluded. - if (excludePatterns != null) { - for (Pattern p : excludePatterns) { - Matcher matcher = p.matcher(requestPath); - if (matcher.lookingAt()) { - if (chain != null) { - chain.doFilter(request, response); - } - return true; - } - } - } - return false; - } - - static boolean excludedPath( - List excludePatterns, HttpServletRequest request, HttpServletResponse response) - throws IOException, ServletException { - return excludedPath(excludePatterns, request, response, null); - } - - static void configExcludes(PathExcluder excluder, String patternConfig) { - if (patternConfig != null) { - String[] excludeArray = patternConfig.split(","); - List patterns = new ArrayList<>(); - excluder.setExcludePatterns(patterns); - for (String element : excludeArray) { - patterns.add(Pattern.compile(element)); - } - } - } - /** * Enforces rate limiting for a request. Should be converted to a servlet filter at some point. * Currently, this is tightly coupled with request tracing which is not ideal either. diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index 632c8aef504..aad2d353052 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -17,8 +17,6 @@ package org.apache.solr.servlet; import static org.apache.solr.servlet.ServletUtils.closeShield; -import static org.apache.solr.servlet.ServletUtils.configExcludes; -import static org.apache.solr.servlet.ServletUtils.excludedPath; import static org.apache.solr.util.tracing.TraceUtils.getSpan; import static org.apache.solr.util.tracing.TraceUtils.setTracer; @@ -67,7 +65,7 @@ // servlets that are more focused in scope. This should become possible now that we have a // ServletContextListener for startup/shutdown of CoreContainer that sets up a service from which // things like CoreContainer can be requested. (or better yet injected) -public class SolrDispatchFilter extends HttpFilter implements PathExcluder { +public class SolrDispatchFilter extends HttpFilter { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private CoreContainerProvider containerProvider; @@ -78,11 +76,6 @@ public class SolrDispatchFilter extends HttpFilter implements PathExcluder { private HttpSolrCallFactory solrCallFactory; - @Override - public void setExcludePatterns(List excludePatterns) { - this.excludePatterns = excludePatterns; - } - private List excludePatterns; public final boolean isV2Enabled = V2ApiUtils.isEnabled(); @@ -133,7 +126,6 @@ public void init(FilterConfig config) throws ServletException { log.trace("SolrDispatchFilter.init(): {}", this.getClass().getClassLoader()); } - configExcludes(this, config.getInitParameter("excludePatterns")); } catch (Throwable t) { // catch this so our filter still works log.error("Could not start Dispatch Filter.", t); @@ -157,9 +149,6 @@ public CoreContainer getCores() throws UnavailableException { "Set the thread contextClassLoader for all 3rd party dependencies that we cannot control") public void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { - if (excludedPath(excludePatterns, request, response, chain)) { - return; - } try (var mdcSnapshot = MDCSnapshot.create()) { assert null != mdcSnapshot; // prevent compiler warning diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java index 522ec25491e..982fc98c135 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java @@ -61,6 +61,7 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.servlet.CoreContainerProvider; +import org.apache.solr.servlet.PathExclusionFilter; import org.apache.solr.servlet.SolrDispatchFilter; import org.apache.solr.util.SocketProxy; import org.apache.solr.util.TimeOut; @@ -109,8 +110,9 @@ public class JettySolrRunner { private Server server; - volatile FilterHolder dispatchFilter; volatile FilterHolder debugFilter; + volatile FilterHolder pathExcludeFilter; + volatile FilterHolder dispatchFilter; private int jettyPort = -1; @@ -398,14 +400,35 @@ public void contextInitialized(ServletContextEvent event) { for (Map.Entry entry : config.extraServlets.entrySet()) { root.addServlet(entry.getKey(), entry.getValue()); } + // TODO: This needs to be driven by a parsing of web.xml eventually + // though we still want to avoid classpath scanning. + + // this path excludes filter isn't actually necessary for any tests, but it's being + // added for parity with the live application. + pathExcludeFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); + pathExcludeFilter.setHeldClass(PathExclusionFilter.class); + pathExcludeFilter.setInitParameter("excludePatterns", excludePatterns); + + // This is our main workhorse dispatchFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); dispatchFilter.setHeldClass(SolrDispatchFilter.class); - dispatchFilter.setInitParameter("excludePatterns", excludePatterns); + // Map dispatchFilter in same path as in web.xml + root.addFilter(pathExcludeFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); root.addFilter(dispatchFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); // Default servlet as a fall-through - root.addServlet(Servlet404.class, "/"); + ServletHolder defaultHolder = root.getServletHandler().newServletHolder(Source.EMBEDDED); + + // considered adding DefaultServlet.class here but perhaps that might grant our unit tests + // the power to serve static resources on the build machines? Not sure, so I'll just give a + // name to our existing hack. The tests passed without this, but it will ensure that if anyone + // ever hits the PathExcludeFilter in the unit test they get a 404 as before not a 500 + defaultHolder.setHeldClass(Servlet404.class); + defaultHolder.setName("default"); + root.addServlet(defaultHolder, "/"); + + // TODO: end area that should be driven by web.xml and webdefault.xml chain = root; } diff --git a/solr/webapp/web/WEB-INF/web.xml b/solr/webapp/web/WEB-INF/web.xml index 37a68b4692b..aa3a192862d 100644 --- a/solr/webapp/web/WEB-INF/web.xml +++ b/solr/webapp/web/WEB-INF/web.xml @@ -26,11 +26,11 @@ - SolrRequestFilter - org.apache.solr.servlet.SolrDispatchFilter + PathExclusionsFilter + org.apache.solr.servlet.PathExclusionFilter @@ -39,6 +39,16 @@ + + PathExclusionsFilter + /* + + + + SolrRequestFilter + org.apache.solr.servlet.SolrDispatchFilter + + SolrRequestFilter /*