dsmiley commented on code in PR #4119: URL: https://github.com/apache/solr/pull/4119#discussion_r2966027250
########## solr/core/src/java/org/apache/solr/servlet/SolrServlet.java: ########## @@ -0,0 +1,163 @@ +/* + * 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 static org.apache.solr.util.tracing.TraceUtils.getSpan; + +import jakarta.servlet.ServletConfig; +import jakarta.servlet.ServletException; +import jakarta.servlet.UnavailableException; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import org.apache.solr.api.V2HttpCall; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.SolrException.ErrorCode; +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.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Solr's interface to Jetty. Formerly known as {@code SolrDispatchFilter}. */ +public class SolrServlet extends HttpServlet { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + private HttpSolrCallFactory solrCallFactory; + private CoreContainerProvider containerProvider; + + @Override + public void init(ServletConfig config) throws ServletException { + try { + super.init(config); + containerProvider = CoreContainerProvider.serviceForContext(config.getServletContext()); + boolean isCoordinator = + NodeRoles.MODE_ON.equals(getCores().nodeRoles.getRoleMode(NodeRoles.Role.COORDINATOR)); + solrCallFactory = + isCoordinator ? new CoordinatorHttpSolrCall.Factory() : new HttpSolrCallFactory() {}; + if (log.isTraceEnabled()) { + log.trace("init(): {}", this.getClass().getClassLoader()); + } + } catch (Throwable t) { + // catch this so our servlet still works + log.error("Could not start Servlet.", t); + if (t instanceof Error) { + throw (Error) t; + } + } finally { + log.trace("init() done"); + } + } + + /** + * The CoreContainer. It's guaranteed to be constructed before this servlet is initialized, but + * could have been shut down. Never null. + */ + public CoreContainer getCores() throws UnavailableException { + return containerProvider.getCoreContainer(); + } + + @Override + protected void service(HttpServletRequest request, HttpServletResponse response) + throws IOException, ServletException { + // Handle root path specially - forward to index.html to trigger welcome-file mechanism Review Comment: @gus-asf , hope you're cool with this. With pass-through gone, either this mechanism here is needed or PathExclusionFilter needs to be configured to handle the root slash case. Since it's possible to remove PathExclusionFilter and I think that's a good thing, I am leaving this here. The root slash case seems like a perfectly in-scope exception to handle. ########## solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java: ########## @@ -1,355 +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 static org.apache.solr.servlet.ServletUtils.closeShield; -import static org.apache.solr.util.tracing.TraceUtils.getSpan; - -import jakarta.servlet.ServletConfig; -import jakarta.servlet.ServletException; -import jakarta.servlet.UnavailableException; -import jakarta.servlet.http.HttpServlet; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.lang.invoke.MethodHandles; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; -import org.apache.solr.api.V2HttpCall; -import org.apache.solr.common.SolrException; -import org.apache.solr.common.SolrException.ErrorCode; -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.security.AuditEvent; -import org.apache.solr.security.AuditEvent.EventType; -import org.apache.solr.security.AuthenticationPlugin; -import org.apache.solr.security.PKIAuthenticationPlugin; -import org.apache.solr.security.PublicKeyHandler; -import org.apache.solr.util.tracing.TraceUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Solr's interface to Jetty. - * - * @since solr 1.2 - */ -// todo: get rid of this class entirely! Request dispatch is the container's responsibility. Much of -// what we have here should be several separate but composable servlet Filters, wrapping multiple -// 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 HttpServlet { // TODO rename to SolrServlet - private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - - protected String abortErrorMessage = null; - - private HttpSolrCallFactory solrCallFactory; - private CoreContainerProvider containerProvider; - - public final boolean isV2Enabled = V2ApiUtils.isEnabled(); - - /** Enum to define action that needs to be processed. */ - public enum Action { - /** Forwards the request via {@link jakarta.servlet.RequestDispatcher}. */ - FORWARD, - /** - * Returns the control, and no further specific processing is needed. This is generally when an - * error is set and returned. - */ - RETURN, - /** Retry the request. Currently used when a core isn't found, we refresh state, and retry. */ - RETRY, - ADMIN, - REMOTEPROXY, - PROCESS, - ADMIN_OR_REMOTEPROXY - } - - public SolrDispatchFilter() {} - - public static final String PROPERTIES_ATTRIBUTE = "solr.properties"; - - public static final String SOLRHOME_ATTRIBUTE = "solr.solr.home"; - - public static final String SOLR_INSTALL_DIR_ATTRIBUTE = "solr.install.dir"; - - public static final String SOLR_CONFIGSET_DEFAULT_CONFDIR_ATTRIBUTE = - "solr.configset.default.confdir"; - - public static final String SOLR_LOG_MUTECONSOLE = "solr.log.muteconsole"; - - public static final String SOLR_LOG_LEVEL = "solr.log.level"; - - @Override - public void init(ServletConfig config) throws ServletException { - try { - super.init(config); - containerProvider = CoreContainerProvider.serviceForContext(config.getServletContext()); - boolean isCoordinator = - NodeRoles.MODE_ON.equals(getCores().nodeRoles.getRoleMode(NodeRoles.Role.COORDINATOR)); - solrCallFactory = - isCoordinator ? new CoordinatorHttpSolrCall.Factory() : new HttpSolrCallFactory() {}; - if (log.isTraceEnabled()) { - log.trace("SolrDispatchFilter.init(): {}", this.getClass().getClassLoader()); - } - - } catch (Throwable t) { - // catch this so our servlet still works - log.error("Could not start Servlet.", t); - if (t instanceof Error) { - throw (Error) t; - } - } finally { - log.trace("SolrDispatchFilter.init() done"); - } - } - - /** - * The CoreContainer. It's guaranteed to be constructed before this servlet is initialized, but - * could have been shut down. Never null. - */ - public CoreContainer getCores() throws UnavailableException { - return containerProvider.getCoreContainer(); - } - - @Override - protected void service(HttpServletRequest request, HttpServletResponse response) - throws IOException, ServletException { - // internal version that tracks if we are in a retry - - dispatch(closeShield(request), closeShield(response), false); - } - - /* - Wait? Where did X go??? (I hear you ask). - - For over a decade this class did anything and everything - In late 2021 SOLR-15590 moved container startup to CoreContainerProvider - In late 2025 SOLR-18040 moved request wrappers to independent ServletFilters - such as PathExclusionFilter see web.xml for a full, up-to-date list - - This class is moving toward only handling dispatch, please think twice - before adding anything else to it. - */ - - private void dispatch(HttpServletRequest request, HttpServletResponse response, boolean retry) - throws IOException, ServletException { - - AtomicReference<HttpServletRequest> wrappedRequest = new AtomicReference<>(); - try { - authenticateRequest(request, response, wrappedRequest); - } catch (SolrAuthenticationException e) { - // it seems our auth system expects the plugin to set the status on the request. - // If this hasn't happened make sure it does happen now rather than throwing an - // exception, that formerly went on to be ignored in - // org.apache.solr.servlet.ServletUtils.traceHttpRequestExecution2 - if (response.getStatus() < 400) { - log.error( - "Authentication Plugin threw SolrAuthenticationException without setting request status >= 400"); - response.sendError(401, "Authentication Plugin rejected credentials."); - } - return; - } - if (wrappedRequest.get() != null) { - request = wrappedRequest.get(); - } - - var span = getSpan(request); - if (getCores().getAuthenticationPlugin() != null) { - if (log.isDebugEnabled()) { - log.debug("User principal: {}", request.getUserPrincipal()); - } - final String principalName; - if (request.getUserPrincipal() != null) { - principalName = request.getUserPrincipal().getName(); - } else { - principalName = null; - } - TraceUtils.setUser(span, String.valueOf(principalName)); - } - - HttpSolrCall call = getHttpSolrCall(request, response, retry); - ExecutorUtil.setServerThreadFlag(Boolean.TRUE); - try { - Action result = call.call(); - switch (result) { - case RETRY -> { - span.addEvent("SolrDispatchFilter RETRY"); - // RECURSION - dispatch(request, response, true); - } - case FORWARD -> { - span.addEvent("SolrDispatchFilter FORWARD"); - request.getRequestDispatcher(call.getPath()).forward(request, response); - } - default -> {} - } - } finally { - call.destroy(); - ExecutorUtil.setServerThreadFlag(null); - } - } - - /** Review Comment: Here and in a number of other places, I removed needless references to SolrDispatchFilter (now SolrServlet). *Very* few places even need to know about it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
