diff --git a/changelog/unreleased/SOLR-18042.yml b/changelog/unreleased/SOLR-18042.yml
deleted file mode 100644
index 73ea60967df..00000000000
--- a/changelog/unreleased/SOLR-18042.yml
+++ /dev/null
@@ -1,8 +0,0 @@
-# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
-title: SOLR 18042 - MDC Logging context lifecycle is now managed by a servlet filter
-type: other # added, changed, fixed, deprecated, removed, dependency_update, security, other
-authors:
- - name: Gus Heck
-links:
- - name: SOLR-18042
- url: https://issues.apache.org/jira/browse/SOLR-18042
diff --git a/changelog/unreleased/SOLR-18044.yml b/changelog/unreleased/SOLR-18044.yml
new file mode 100644
index 00000000000..8f1c723b9bf
--- /dev/null
+++ b/changelog/unreleased/SOLR-18044.yml
@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: EssentialSolrRequestFilter has been added to perform non-optional request wrapping operations required for proper functioning of Solr
+type: other # added, changed, fixed, deprecated, removed, dependency_update, security, other
+authors:
+ - name: Gus Heck
+links:
+ - name: SOLR-18044
+ url: https://issues.apache.org/jira/browse/SOLR-18044
diff --git a/solr/core/src/java/org/apache/solr/servlet/MdcLoggingFilter.java b/solr/core/src/java/org/apache/solr/servlet/EssentialSolrRequestFilter.java
similarity index 52%
rename from solr/core/src/java/org/apache/solr/servlet/MdcLoggingFilter.java
rename to solr/core/src/java/org/apache/solr/servlet/EssentialSolrRequestFilter.java
index 30dda327295..d82fb656b06 100644
--- a/solr/core/src/java/org/apache/solr/servlet/MdcLoggingFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/EssentialSolrRequestFilter.java
@@ -25,11 +25,20 @@
import org.apache.solr.common.util.SuppressForbidden;
import org.apache.solr.logging.MDCLoggingContext;
import org.apache.solr.logging.MDCSnapshot;
+import org.apache.solr.request.SolrRequestInfo;
+import org.apache.solr.util.RTimerTree;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-/** Servlet Filter to set up and tear down MDC Logging context for each reqeust. */
-public class MdcLoggingFilter extends CoreContainerAwareHttpFilter {
+/**
+ * Servlet Filter to set up and tear down a various things that downstream code in solr relies on.
+ * It is expected that solr will fail to function as intended without the conditions initialized in
+ * this filter. Before adding anything to this filter ask yourself if a user could run without the
+ * feature you are adding (either with an alternative implementation or without it at all to reduce
+ * cpu/memory/dependencies). If it is not essential, it should have its own separate filter. Also,
+ * please include a comment indicating why the thing added is essential.
+ */
+public class EssentialSolrRequestFilter extends CoreContainerAwareHttpFilter {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
@@ -42,16 +51,44 @@ protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterC
ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
// this autocloseable is here to invoke MDCSnapshot.close() which restores captured state
try (var mdcSnapshot = MDCSnapshot.create()) {
+
+ // MDC logging *shouldn't* be essential but currently is, see SOLR-18050.
+ // Our use of SLF4J indicates that we intend logging to be pluggable, and
+ // some implementations won't have an MDC, so having it as essential limits
+ // logging implementations.
log.trace("MDC snapshot recorded {}", mdcSnapshot); // avoid both compiler and ide warning.
MDCLoggingContext.reset();
MDCLoggingContext.setNode(getCores());
- // This doesn't belong here, but for the moment it is here to preserve it's relative
- // timing of execution for now. Probably I will break this out in a subsequent change
+
+ // This is essential to accommodate libraries that (annoyingly) use
+ // Thread.currentThread().getContextClassLoader()
Thread.currentThread().setContextClassLoader(getCores().getResourceLoader().getClassLoader());
+
+ // set a request timer which can be reused by requests if needed
+ // Request Timer is essential for QueryLimits functionality as well as
+ // timing our requests.
+ req.setAttribute(SolrRequestParsers.REQUEST_TIMER_SERVLET_ATTRIBUTE, new RTimerTree());
+
+ // put the core container in request attribute
+ // This is essential for the LoadAdminUiServlet class. Removing it will cause 404
+ req.setAttribute("org.apache.solr.CoreContainer", getCores());
chain.doFilter(req, res);
} finally {
+ // cleanups for above stuff
MDCLoggingContext.reset();
Thread.currentThread().setContextClassLoader(contextClassLoader);
+
+ // This is an essential safety valve to ensure we don't accidentally bleed information
+ // between requests.
+ SolrRequestInfo.reset();
+ if (!req.isAsyncStarted()) { // jetty's proxy uses this
+
+ // essential to avoid SOLR-8453 and SOLR-8683
+ ServletUtils.consumeInputFully(req, res);
+
+ // essential to remove temporary files created during multipart requests.
+ SolrRequestParsers.cleanupMultipartFiles(req);
+ }
}
}
}
diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
index 97a9677b69b..774519b2577 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -99,7 +99,6 @@
import org.apache.solr.servlet.cache.HttpCacheHeaderUtil;
import org.apache.solr.servlet.cache.Method;
import org.apache.solr.update.processor.DistributingUpdateProcessorFactory;
-import org.apache.solr.util.RTimerTree;
import org.apache.solr.util.tracing.TraceUtils;
import org.apache.zookeeper.KeeperException;
import org.eclipse.jetty.client.HttpClient;
@@ -157,10 +156,6 @@ public HttpSolrCall(
this.path = ServletUtils.getPathAfterContext(req);
req.setAttribute(HttpSolrCall.class.getName(), this);
- // set a request timer which can be reused by requests if needed
- req.setAttribute(SolrRequestParsers.REQUEST_TIMER_SERVLET_ATTRIBUTE, new RTimerTree());
- // put the core container in request attribute
- req.setAttribute("org.apache.solr.CoreContainer", cores);
}
public String getPath() {
diff --git a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
index bbbd9166c51..c33e7dd0aff 100644
--- a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
+++ b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
@@ -44,6 +44,7 @@ public final class LoadAdminUiServlet extends HttpServlet {
Boolean.parseBoolean(System.getProperty("solr.ui.enabled", "true"));
// list of comma separated URLs to inject into the CSP connect-src directive
public static final String SYSPROP_CSP_CONNECT_SRC_URLS = "solr.ui.headers.csp.connect-src.urls";
+ public static final String CORE_CONTAINER_REQUEST_ATTRIBUTE = "org.apache.solr.CoreContainer";
@Override
public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException {
@@ -62,7 +63,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro
// This attribute is set by the SolrDispatchFilter
String admin = request.getRequestURI().substring(request.getContextPath().length());
- CoreContainer cores = (CoreContainer) request.getAttribute("org.apache.solr.CoreContainer");
+ CoreContainer cores = (CoreContainer) request.getAttribute(CORE_CONTAINER_REQUEST_ATTRIBUTE);
try (InputStream in = getServletContext().getResourceAsStream(admin)) {
if (in != null && cores != null) {
response.setCharacterEncoding("UTF-8");
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 6d80be11c70..43de6e2bd07 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -37,7 +37,6 @@
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.NodeRoles;
import org.apache.solr.handler.api.V2ApiUtils;
-import org.apache.solr.request.SolrRequestInfo;
import org.apache.solr.security.AuditEvent;
import org.apache.solr.security.AuditEvent.EventType;
import org.apache.solr.security.AuthenticationPlugin;
@@ -146,25 +145,17 @@ private void doFilterRetry(
throws IOException, ServletException {
setTracer(request, getCores().getTracer());
RateLimitManager rateLimitManager = getRateLimitManager();
- try {
- ServletUtils.rateLimitRequest(
- rateLimitManager,
- request,
- response,
- () -> {
- try {
- dispatch(chain, request, response, retry);
- } catch (IOException | ServletException | SolrAuthenticationException e) {
- throw new ExceptionWhileTracing(e);
- }
- });
- } finally {
- SolrRequestInfo.reset();
- if (!request.isAsyncStarted()) { // jetty's proxy uses this
- ServletUtils.consumeInputFully(request, response);
- SolrRequestParsers.cleanupMultipartFiles(request);
- }
- }
+ ServletUtils.rateLimitRequest(
+ rateLimitManager,
+ request,
+ response,
+ () -> {
+ try {
+ dispatch(chain, request, response, retry);
+ } catch (IOException | ServletException | SolrAuthenticationException e) {
+ throw new ExceptionWhileTracing(e);
+ }
+ });
}
private void dispatch(
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 2825b1f4a72..99ebe39fe67 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,7 +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.MdcLoggingFilter;
+import org.apache.solr.servlet.EssentialSolrRequestFilter;
import org.apache.solr.servlet.PathExclusionFilter;
import org.apache.solr.servlet.SolrDispatchFilter;
import org.apache.solr.util.SocketProxy;
@@ -113,7 +113,7 @@ public class JettySolrRunner {
volatile FilterHolder debugFilter;
volatile FilterHolder pathExcludeFilter;
- volatile FilterHolder mdcLoggingFilter;
+ volatile FilterHolder essentialFilter;
volatile FilterHolder dispatchFilter;
private int jettyPort = -1;
@@ -412,8 +412,8 @@ public void contextInitialized(ServletContextEvent event) {
pathExcludeFilter.setInitParameter("excludePatterns", excludePatterns);
// logging context setup
- mdcLoggingFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED);
- mdcLoggingFilter.setHeldClass(MdcLoggingFilter.class);
+ essentialFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED);
+ essentialFilter.setHeldClass(EssentialSolrRequestFilter.class);
// This is our main workhorse
dispatchFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED);
@@ -421,7 +421,7 @@ public void contextInitialized(ServletContextEvent event) {
// Map dispatchFilter in same path as in web.xml
root.addFilter(pathExcludeFilter, "/*", EnumSet.of(DispatcherType.REQUEST));
- root.addFilter(mdcLoggingFilter, "/*", EnumSet.of(DispatcherType.REQUEST));
+ root.addFilter(essentialFilter, "/*", EnumSet.of(DispatcherType.REQUEST));
root.addFilter(dispatchFilter, "/*", EnumSet.of(DispatcherType.REQUEST));
// Default servlet as a fall-through
diff --git a/solr/webapp/web/WEB-INF/web.xml b/solr/webapp/web/WEB-INF/web.xml
index a7df6a32113..af5e733526c 100644
--- a/solr/webapp/web/WEB-INF/web.xml
+++ b/solr/webapp/web/WEB-INF/web.xml
@@ -46,7 +46,7 @@
MDCLoggingFilter
- org.apache.solr.servlet.MdcLoggingFilter
+ org.apache.solr.servlet.EssentialSolrRequestFilter