This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_9x in repository https://gitbox.apache.org/repos/asf/solr.git
commit 93ac7d4456e36ebaf386820dc075248b14764940 Author: David Smiley <[email protected]> AuthorDate: Tue Oct 29 21:26:05 2024 -0400 SOLR-17485 Remove CheckLoggingConfiguration; unnecessary (#2763) A refactoring, and removing needless CheckLoggingConfiguration. Misc changes: * Extend HttpFilter etc.; remove needless subclasses. Thus don't need to cast HttpServletRequest. * remove no-op needless destroy(); same as super * Move excludePath check to very first thing * rename doFilter(...retry) to doFilterRetry * move closeShield to before doFilterRetry so it doesn't have to know about retry, simplifying things. * no longer have leading underscore param names since no longer have variable masking (cherry picked from commit 8f18729f97e897f3755421c65b4b539fbc871746) --- .../org/apache/solr/servlet/BaseSolrFilter.java | 31 ------- .../org/apache/solr/servlet/BaseSolrServlet.java | 32 -------- .../solr/servlet/CheckLoggingConfiguration.java | 37 --------- .../apache/solr/servlet/LoadAdminUiServlet.java | 11 +-- .../org/apache/solr/servlet/RedirectServlet.java | 3 +- .../java/org/apache/solr/servlet/ServletUtils.java | 96 +++++++++------------- .../apache/solr/servlet/SolrDispatchFilter.java | 32 +++----- 7 files changed, 59 insertions(+), 183 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/servlet/BaseSolrFilter.java b/solr/core/src/java/org/apache/solr/servlet/BaseSolrFilter.java deleted file mode 100644 index d16d722447d..00000000000 --- a/solr/core/src/java/org/apache/solr/servlet/BaseSolrFilter.java +++ /dev/null @@ -1,31 +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 javax.servlet.Filter; - -/** - * All Solr filters available to the user's webapp should extend this class and not just implement - * {@link Filter}. This class ensures that the logging configuration is correct before any Solr - * specific code is executed. - */ -abstract class BaseSolrFilter implements Filter { - - static { - CheckLoggingConfiguration.check(); - } -} diff --git a/solr/core/src/java/org/apache/solr/servlet/BaseSolrServlet.java b/solr/core/src/java/org/apache/solr/servlet/BaseSolrServlet.java deleted file mode 100644 index 85aca08cd86..00000000000 --- a/solr/core/src/java/org/apache/solr/servlet/BaseSolrServlet.java +++ /dev/null @@ -1,32 +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 javax.servlet.http.HttpServlet; - -/** - * All Solr servlets available to the user's webapp should extend this class and not {@link - * HttpServlet}. This class ensures that the logging configuration is correct before any Solr - * specific code is executed. - */ -@SuppressWarnings("serial") -abstract class BaseSolrServlet extends HttpServlet { - - static { - CheckLoggingConfiguration.check(); - } -} diff --git a/solr/core/src/java/org/apache/solr/servlet/CheckLoggingConfiguration.java b/solr/core/src/java/org/apache/solr/servlet/CheckLoggingConfiguration.java deleted file mode 100644 index 70506dea12e..00000000000 --- a/solr/core/src/java/org/apache/solr/servlet/CheckLoggingConfiguration.java +++ /dev/null @@ -1,37 +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 java.lang.invoke.MethodHandles; -import org.slf4j.LoggerFactory; - -final class CheckLoggingConfiguration { - - static void check() { - try { - LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - } catch (NoClassDefFoundError e) { - throw new NoClassDefFoundError( - "Failed to initialize Apache Solr: " - + "Could not find necessary SLF4j logging jars. If using Jetty, the SLF4j logging jars need to go in " - + "the jetty lib/ext directory. For other containers, the corresponding directory should be used. " - + "For more information, see: https://cwiki.apache.org/confluence/display/solr/SolrLogging#SolrLogging-UsingtheexampleloggingsetupincontainersotherthanJetty";); - } - } - - private CheckLoggingConfiguration() {} -} 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 9da4bc84775..436906d3a69 100644 --- a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java +++ b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java @@ -25,6 +25,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.commons.io.output.CloseShieldOutputStream; @@ -36,7 +37,7 @@ import org.apache.solr.core.SolrCore; * * @since solr 4.0 */ -public final class LoadAdminUiServlet extends BaseSolrServlet { +public final class LoadAdminUiServlet extends HttpServlet { // check system properties for whether or not admin UI is disabled, default is false private static final boolean disabled = @@ -45,16 +46,16 @@ public final class LoadAdminUiServlet extends BaseSolrServlet { public static final String SYSPROP_CSP_CONNECT_SRC_URLS = "solr.ui.headers.csp.connect-src.urls"; @Override - public void doGet(HttpServletRequest _request, HttpServletResponse _response) throws IOException { + public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { if (disabled) { - _response.sendError( + response.sendError( 404, "Solr Admin UI is disabled. To enable it, change the default value of SOLR_ADMIN_UI_" + "ENABLED in bin/solr.in.sh or solr.in.cmd."); return; } - HttpServletRequest request = ServletUtils.closeShield(_request, false); - HttpServletResponse response = ServletUtils.closeShield(_response, false); + request = ServletUtils.closeShield(request); + response = ServletUtils.closeShield(response); response.addHeader( "X-Frame-Options", "DENY"); // security: SOLR-7966 - avoid clickjacking for admin interface diff --git a/solr/core/src/java/org/apache/solr/servlet/RedirectServlet.java b/solr/core/src/java/org/apache/solr/servlet/RedirectServlet.java index a67e20831de..a0c11b15a04 100644 --- a/solr/core/src/java/org/apache/solr/servlet/RedirectServlet.java +++ b/solr/core/src/java/org/apache/solr/servlet/RedirectServlet.java @@ -19,11 +19,12 @@ package org.apache.solr.servlet; import java.io.IOException; import javax.servlet.ServletConfig; import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; /** A Simple redirection servlet to help us deprecate old UI elements */ -public class RedirectServlet extends BaseSolrServlet { +public class RedirectServlet extends HttpServlet { static final String CONTEXT_KEY = "${context}"; diff --git a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java index c4da8e4d7b0..80234081971 100644 --- a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java +++ b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java @@ -76,78 +76,62 @@ public abstract class ServletUtils { } /** - * Wrap the request's input stream with a close shield. If this is a retry, we will assume that - * the stream has already been wrapped and do nothing. + * Wrap the request's input stream with a close shield. * * <p>Only the container should ever actually close the servlet output stream. This method * possibly should be turned into a servlet filter * * @param request The request to wrap. - * @param retry If this is an original request or a retry. * @return A request object with an {@link InputStream} that will ignore calls to close. */ - public static HttpServletRequest closeShield(HttpServletRequest request, boolean retry) { - if (!retry) { - return new HttpServletRequestWrapper(request) { - - @Override - public ServletInputStream getInputStream() throws IOException { - - return new ServletInputStreamWrapper(super.getInputStream()) { - @Override - public void close() { - // even though we skip closes, we let local tests know not to close so that a full - // understanding can take place - assert !Thread.currentThread() - .getStackTrace()[2] - .getClassName() - .matches("org\\.apache\\.(?:solr|lucene).*") - : CLOSE_STREAM_MSG; - this.stream = ClosedServletInputStream.CLOSED_SERVLET_INPUT_STREAM; - } - }; - } - }; - } else { - return request; - } + public static HttpServletRequest closeShield(HttpServletRequest request) { + return new HttpServletRequestWrapper(request) { + @Override + public ServletInputStream getInputStream() throws IOException { + return new ServletInputStreamWrapper(super.getInputStream()) { + @Override + public void close() { + // even though we skip closes, we let local tests know not to close so that a full + // understanding can take place + assert !Thread.currentThread() + .getStackTrace()[2] + .getClassName() + .matches("org\\.apache\\.(?:solr|lucene).*") + : CLOSE_STREAM_MSG; + this.stream = ClosedServletInputStream.CLOSED_SERVLET_INPUT_STREAM; + } + }; + } + }; } /** - * Wrap the response's output stream with a close shield. If this is a retry, we will assume that - * the stream has already been wrapped and do nothing. + * Wrap the response's output stream with a close shield. * * <p>Only the container should ever actually close the servlet request stream. * * @param response The response to wrap. - * @param retry If this response corresponds to an original request or a retry. * @return A response object with an {@link OutputStream} that will ignore calls to close. */ - public static HttpServletResponse closeShield(HttpServletResponse response, boolean retry) { - if (!retry) { - return new HttpServletResponseWrapper(response) { - - @Override - public ServletOutputStream getOutputStream() throws IOException { - - return new ServletOutputStreamWrapper(super.getOutputStream()) { - @Override - public void close() { - // even though we skip closes, we let local tests know not to close so that a full - // understanding can take place - assert !Thread.currentThread() - .getStackTrace()[2] - .getClassName() - .matches("org\\.apache\\.(?:solr|lucene).*") - : CLOSE_STREAM_MSG; - stream = ClosedServletOutputStream.CLOSED_SERVLET_OUTPUT_STREAM; - } - }; - } - }; - } else { - return response; - } + public static HttpServletResponse closeShield(HttpServletResponse response) { + return new HttpServletResponseWrapper(response) { + @Override + public ServletOutputStream getOutputStream() throws IOException { + return new ServletOutputStreamWrapper(super.getOutputStream()) { + @Override + public void close() { + // even though we skip closes, we let local tests know not to close so that a full + // understanding can take place + assert !Thread.currentThread() + .getStackTrace()[2] + .getClassName() + .matches("org\\.apache\\.(?:solr|lucene).*") + : CLOSE_STREAM_MSG; + stream = ClosedServletOutputStream.CLOSED_SERVLET_OUTPUT_STREAM; + } + }; + } + }; } static boolean excludedPath( 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 bd408f886cf..cfdac952858 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -36,9 +36,8 @@ import java.util.regex.Pattern; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; import javax.servlet.UnavailableException; +import javax.servlet.http.HttpFilter; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.http.client.HttpClient; @@ -70,7 +69,7 @@ import org.slf4j.LoggerFactory; // 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 BaseSolrFilter implements PathExcluder { +public class SolrDispatchFilter extends HttpFilter implements PathExcluder { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); public static final String ATTR_TRACING_SPAN = Span.class.getName(); public static final String ATTR_TRACING_TRACER = Tracer.class.getName(); @@ -163,38 +162,29 @@ public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder { return containerProvider.getCoreContainer(); } - @Override - public void destroy() { - // CoreService shuts itself down as a ContextListener. The filter does not own anything with a - // lifecycle anymore! Yay! - } - @Override @SuppressForbidden( reason = "Set the thread contextClassLoader for all 3rd party dependencies that we cannot control") - public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + 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 MDCLoggingContext.reset(); MDCLoggingContext.setNode(getCores()); Thread.currentThread().setContextClassLoader(getCores().getResourceLoader().getClassLoader()); - doFilter(request, response, chain, false); + doFilterRetry(closeShield(request), closeShield(response), chain, false); } } - public void doFilter( - ServletRequest _request, ServletResponse _response, FilterChain chain, boolean retry) + protected void doFilterRetry( + HttpServletRequest request, HttpServletResponse response, FilterChain chain, boolean retry) throws IOException, ServletException { - if (!(_request instanceof HttpServletRequest)) return; - HttpServletRequest request = closeShield((HttpServletRequest) _request, retry); - HttpServletResponse response = closeShield((HttpServletResponse) _response, retry); - - if (excludedPath(excludePatterns, request, response, chain)) { - return; - } Tracer t = getCores() == null ? GlobalTracer.get() : getCores().getTracer(); request.setAttribute(ATTR_TRACING_TRACER, t); RateLimitManager rateLimitManager = containerProvider.getRateLimitManager(); @@ -256,7 +246,7 @@ public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder { break; case RETRY: getSpan(request).log("SolrDispatchFilter RETRY"); - doFilter(request, response, chain, true); // RECURSION + doFilterRetry(request, response, chain, true); // RECURSION break; case FORWARD: getSpan(request).log("SolrDispatchFilter FORWARD");
