Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions changelog/unreleased/SOLR-18041.yml
Original file line number Diff line number Diff line change
@@ -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
28 changes: 0 additions & 28 deletions solr/core/src/java/org/apache/solr/servlet/PathExcluder.java

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* 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.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;

public class PathExclusionFilter extends HttpFilter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have a bit of documentaton on what it does. Like "if the path matches the configured criteria, we process with the 'default' servlet, that which serves files."


private List<Pattern> 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)) {
chain.doFilter(req, res);
} else {
// N.B. "default" is the name for org.eclipse.jetty.ee10.servlet.DefaultServlet
// configured in solr/server/etc/webdefault.xml
req.getServletContext().getNamedDispatcher("default").forward(req, res);
}
}
}
44 changes: 0 additions & 44 deletions solr/core/src/java/org/apache/solr/servlet/ServletUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -131,45 +126,6 @@ public void close() {
};
}

static boolean excludedPath(
List<Pattern> 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow that was a sneaky side-effect of what looked like a simple boolean method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup took me a moment to figure out why my first pass attempt was somehow still passing control to SolrDispatchFilter (which becomes next in line)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah actually looking there, there is a change that probably should be made, since patterns are being set (but now ignored).

}
return true;
}
}
}
return false;
}

static boolean excludedPath(
List<Pattern> 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<Pattern> 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you can also remove PathExcluder

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and pull methods out of ServletUtils too! this is fun :-)

public class SolrDispatchFilter extends HttpFilter {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

private CoreContainerProvider containerProvider;
Expand All @@ -78,11 +76,6 @@ public class SolrDispatchFilter extends HttpFilter implements PathExcluder {

private HttpSolrCallFactory solrCallFactory;

@Override
public void setExcludePatterns(List<Pattern> excludePatterns) {
this.excludePatterns = excludePatterns;
}

private List<Pattern> excludePatterns;

public final boolean isV2Enabled = V2ApiUtils.isEnabled();
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down
18 changes: 14 additions & 4 deletions solr/webapp/web/WEB-INF/web.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@
</listener>
<!-- Any path (name) registered in solrconfig.xml will be sent to that filter -->
<filter>
<filter-name>SolrRequestFilter</filter-name>
<filter-class>org.apache.solr.servlet.SolrDispatchFilter</filter-class>
<filter-name>PathExclusionsFilter</filter-name>
<filter-class>org.apache.solr.servlet.PathExclusionFilter</filter-class>
<!--
Exclude patterns is a list of directories that would be short circuited by the
SolrDispatchFilter. It includes all Admin UI related static content.
Exclude patterns is a list of directories that would be short-circuited by this
Filter. It includes all Admin UI related static content.
NOTE: It is NOT a pattern but only matches the start of the HTTP ServletPath.
-->
<init-param>
Expand All @@ -39,6 +39,16 @@
</init-param>
</filter>

<filter-mapping>
<filter-name>PathExclusionsFilter</filter-name>
<url-pattern>/*</url-pattern>
</filter-mapping>

<filter>
<filter-name>SolrRequestFilter</filter-name>
<filter-class>org.apache.solr.servlet.SolrDispatchFilter</filter-class>
</filter>

<filter-mapping>
<filter-name>SolrRequestFilter</filter-name>
<url-pattern>/*</url-pattern>
Expand Down