From 4877f19b2794d97471dd4d95ac607a4c589d0a02 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Thu, 1 Jan 2026 21:10:34 -0500 Subject: [PATCH 1/3] SOLR-18044 EssentialSolrRequestFilter to perform non-optional request wrapping operations required for proper functioning of Solr --- ...r.java => EssentialSolrRequestFilter.java} | 45 +++++++++++++++++-- .../org/apache/solr/servlet/HttpSolrCall.java | 5 --- .../solr/servlet/LoadAdminUiServlet.java | 3 +- .../solr/servlet/SolrDispatchFilter.java | 31 +++++-------- .../apache/solr/embedded/JettySolrRunner.java | 10 ++--- solr/webapp/web/WEB-INF/web.xml | 2 +- 6 files changed, 60 insertions(+), 36 deletions(-) rename solr/core/src/java/org/apache/solr/servlet/{MdcLoggingFilter.java => EssentialSolrRequestFilter.java} (52%) 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 30dda3272959..d82fb656b06b 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 97a9677b69bc..774519b25770 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 bbbd9166c510..c33e7dd0affa 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 6d80be11c707..43de6e2bd074 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 2825b1f4a720..99ebe39fe678 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 a7df6a321131..af5e733526ce 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 From a951b6ab0808fd18e036030f6606c1db415e9ba0 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Thu, 1 Jan 2026 12:15:32 -0500 Subject: [PATCH 2/3] SOLR-18044 changelog --- changelog/unreleased/SOLR-18042.yml | 8 -------- changelog/unreleased/SOLR-18044.yml | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-) delete mode 100644 changelog/unreleased/SOLR-18042.yml create mode 100644 changelog/unreleased/SOLR-18044.yml diff --git a/changelog/unreleased/SOLR-18042.yml b/changelog/unreleased/SOLR-18042.yml deleted file mode 100644 index 73ea60967dff..000000000000 --- 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 000000000000..f6c3cacb205a --- /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: SOLR 18044 - 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 From d9097e5b0d50d3839ff7453c3ada89d067e0bc84 Mon Sep 17 00:00:00 2001 From: Gus Heck Date: Fri, 2 Jan 2026 08:17:27 -0500 Subject: [PATCH 3/3] SOLR-18044 Remove issue number from title --- changelog/unreleased/SOLR-18044.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/SOLR-18044.yml b/changelog/unreleased/SOLR-18044.yml index f6c3cacb205a..8f1c723b9bf1 100644 --- a/changelog/unreleased/SOLR-18044.yml +++ b/changelog/unreleased/SOLR-18044.yml @@ -1,5 +1,5 @@ # See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc -title: SOLR 18044 - EssentialSolrRequestFilter has been added to perform non-optional request wrapping operations required for proper functioning of Solr +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