This is an automated email from the ASF dual-hosted git repository.
dsmiley 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 8f18729f97e SOLR-17485 Remove CheckLoggingConfiguration; unnecessary
(#2763)
8f18729f97e is described below
commit 8f18729f97e897f3755421c65b4b539fbc871746
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
---
.../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 605f1c6c668..f9be5246b3d 100644
--- a/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java
+++ b/solr/core/src/java/org/apache/solr/servlet/ServletUtils.java
@@ -72,78 +72,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 5cda46ddcb3..189ebeec134 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -33,9 +33,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;
@@ -69,7 +68,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());
private CoreContainerProvider containerProvider;
@@ -160,38 +159,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;
- }
setTracer(request, getCores().getTracer());
RateLimitManager rateLimitManager =
containerProvider.getRateLimitManager();
try {
@@ -248,7 +238,7 @@ public class SolrDispatchFilter extends BaseSolrFilter
implements PathExcluder {
break;
case RETRY:
span.addEvent("SolrDispatchFilter RETRY");
- doFilter(request, response, chain, true); // RECURSION
+ doFilterRetry(request, response, chain, true); // RECURSION
break;
case FORWARD:
span.addEvent("SolrDispatchFilter FORWARD");
