This is an automated email from the ASF dual-hosted git repository.
gus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 0538d17e5dd SOLR-18044 EssentialSolrRequestFilter to perform
non-optional request wrapping operations (#3997)
0538d17e5dd is described below
commit 0538d17e5ddb2a0f01bfe16884d1255c5fa45fb7
Author: Gus Heck <[email protected]>
AuthorDate: Sat Jan 3 18:02:16 2026 -0500
SOLR-18044 EssentialSolrRequestFilter to perform non-optional request
wrapping operations (#3997)
---
changelog/unreleased/SOLR-18042.yml | 8 --
changelog/unreleased/SOLR-18044.yml | 8 ++
.../solr/servlet/EssentialSolrRequestFilter.java | 97 ++++++++++++++++++++++
.../java/org/apache/solr/servlet/HttpSolrCall.java | 5 --
.../apache/solr/servlet/LoadAdminUiServlet.java | 4 +-
.../org/apache/solr/servlet/MdcLoggingFilter.java | 57 -------------
.../apache/solr/servlet/SolrDispatchFilter.java | 31 +++----
.../org/apache/solr/embedded/JettySolrRunner.java | 10 +--
solr/webapp/web/WEB-INF/web.xml | 2 +-
9 files changed, 125 insertions(+), 97 deletions(-)
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/EssentialSolrRequestFilter.java
b/solr/core/src/java/org/apache/solr/servlet/EssentialSolrRequestFilter.java
new file mode 100644
index 00000000000..076d7caae6c
--- /dev/null
+++ b/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);
+ }
+ }
+ }
+}
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.SolrDispatchFilter.Action;
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 class 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..24b0d6e1063 100644
--- a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
+++ b/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 final class LoadAdminUiServlet extends HttpServlet {
// 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/MdcLoggingFilter.java
b/solr/core/src/java/org/apache/solr/servlet/MdcLoggingFilter.java
deleted file mode 100644
index 30dda327295..00000000000
--- a/solr/core/src/java/org/apache/solr/servlet/MdcLoggingFilter.java
+++ /dev/null
@@ -1,57 +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 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.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 {
-
- 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()) {
- 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
-
Thread.currentThread().setContextClassLoader(getCores().getResourceLoader().getClassLoader());
- chain.doFilter(req, res);
- } finally {
- MDCLoggingContext.reset();
- Thread.currentThread().setContextClassLoader(contextClassLoader);
- }
- }
-}
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.common.util.ExecutorUtil;
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 @@ public class SolrDispatchFilter extends
CoreContainerAwareHttpFilter {
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.common.util.Utils;
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 class JettySolrRunner {
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 class JettySolrRunner {
// 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 @@
<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>
