SOLR-18044 EssentialSolrRequestFilter to perform non-optional request wrapping operations

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

solr/core/src/java/org/apache/solr/servlet/EssentialSolrRequestFilter.java

@@ -0,0 +1,97 @@
+/*
+ * 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.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+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 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 {
+
+  // Best to put constant here because solr is not supposed to be functional (or compile)
+  // without this filter.
+  public static final String CORE_CONTAINER_REQUEST_ATTRIBUTE = "org.apache.solr.CoreContainer";
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @Override
+  @SuppressForbidden(
+      reason =
+          "Set the thread contextClassLoader for all 3rd party dependencies that we cannot control")
+  protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain)
+      throws IOException, ServletException {
+    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 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(CORE_CONTAINER_REQUEST_ATTRIBUTE, 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);
+      }
+    }
+  }
+}

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() {

solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java

@@ -16,6 +16,8 @@
  */
 package org.apache.solr.servlet;
 
+import static org.apache.solr.servlet.EssentialSolrRequestFilter.CORE_CONTAINER_REQUEST_ATTRIBUTE;
+
 import com.google.common.net.HttpHeaders;
 import jakarta.servlet.http.HttpServlet;
 import jakarta.servlet.http.HttpServletRequest;
@@ -62,7 +64,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");

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(

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,16 +412,16 @@ 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);
       dispatchFilter.setHeldClass(SolrDispatchFilter.class);
 
       // 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

solr/webapp/web/WEB-INF/web.xml

@@ -46,7 +46,7 @@
 
   <filter>
     <filter-name>MDCLoggingFilter</filter-name>
-    <filter-class>org.apache.solr.servlet.MdcLoggingFilter</filter-class>
+    <filter-class>org.apache.solr.servlet.EssentialSolrRequestFilter</filter-class>
   </filter>
 
   <filter-mapping>