This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push: new 52399ecece Review ignored exceptions 52399ecece is described below commit 52399ecececdf510a50bddeaca7b34708845c1d5 Author: Mark Thomas <ma...@apache.org> AuthorDate: Wed Aug 20 11:32:51 2025 +0100 Review ignored exceptions Trying to consistently use "ignore" as the name of the exception Some additional logging Better comments Some related clean-up --- java/jakarta/el/Util.java | 2 +- java/jakarta/servlet/http/HttpServlet.java | 13 ++++----- java/jakarta/websocket/ContainerProvider.java | 2 +- .../websocket/server/ServerEndpointConfig.java | 2 +- .../catalina/authenticator/AuthenticatorBase.java | 5 ++-- .../apache/catalina/connector/CoyoteAdapter.java | 10 +++---- java/org/apache/catalina/connector/Request.java | 15 +++++----- java/org/apache/catalina/core/StandardContext.java | 5 ++-- .../catalina/ha/session/JvmRouteBinderValve.java | 10 +++---- .../apache/catalina/servlets/DefaultServlet.java | 16 +++++----- .../catalina/session/PersistentManagerBase.java | 14 ++++----- .../catalina/storeconfig/StoreFactoryBase.java | 6 ++-- .../tribes/membership/LocalStrings.properties | 1 + .../tribes/membership/McastServiceImpl.java | 23 ++++++++++----- .../catalina/tribes/transport/RxTaskPool.java | 4 +-- .../tribes/transport/nio/ParallelNioSender.java | 9 +++--- .../apache/catalina/tribes/util/StringManager.java | 14 ++++----- .../valves/rewrite/LocalStrings.properties | 2 ++ .../catalina/valves/rewrite/ResolverImpl.java | 34 ++++++++++++++++++---- .../catalina/valves/rewrite/RewriteValve.java | 2 +- .../webresources/AbstractArchiveResourceSet.java | 8 +++-- .../catalina/webresources/LocalStrings.properties | 1 + java/org/apache/coyote/ajp/Constants.java | 8 ++--- java/org/apache/coyote/http11/Http11Processor.java | 4 +-- java/org/apache/coyote/http2/Http2Parser.java | 2 +- .../apache/coyote/http2/Http2UpgradeHandler.java | 4 +-- java/org/apache/coyote/http2/StreamProcessor.java | 2 +- java/org/apache/jasper/JspC.java | 8 ++--- .../apache/jasper/runtime/JspContextWrapper.java | 2 +- java/org/apache/jasper/runtime/JspFactoryImpl.java | 2 +- .../org/apache/jasper/runtime/PageContextImpl.java | 6 ++-- java/org/apache/juli/FileHandler.java | 14 ++++----- java/org/apache/naming/StringManager.java | 8 ++--- java/org/apache/tomcat/util/http/ResponseUtil.java | 4 +-- .../apache/tomcat/util/http/WebdavIfHeader.java | 6 ++-- .../apache/tomcat/util/modeler/BaseModelMBean.java | 21 ++----------- java/org/apache/tomcat/util/net/NioEndpoint.java | 14 ++++++--- java/org/apache/tomcat/util/res/StringManager.java | 7 ++--- .../org/apache/tomcat/util/scan/JarFileUrlJar.java | 15 ++++++---- .../tomcat/websocket/server/WsServerContainer.java | 2 +- .../catalina/valves/rewrite/TestResolverSSL.java | 2 +- 41 files changed, 177 insertions(+), 152 deletions(-) diff --git a/java/jakarta/el/Util.java b/java/jakarta/el/Util.java index 30060cce22..f361c7d062 100644 --- a/java/jakarta/el/Util.java +++ b/java/jakarta/el/Util.java @@ -130,7 +130,7 @@ class Util { try { Method method = clazz.getMethod(methodName, paramTypes); return getMethod(clazz, base, method); - } catch (NoSuchMethodException | SecurityException e) { + } catch (NoSuchMethodException | SecurityException ignore) { // Fall through to broader, slower logic } } diff --git a/java/jakarta/servlet/http/HttpServlet.java b/java/jakarta/servlet/http/HttpServlet.java index c18404102d..fc3e9fb3b8 100644 --- a/java/jakarta/servlet/http/HttpServlet.java +++ b/java/jakarta/servlet/http/HttpServlet.java @@ -683,9 +683,8 @@ public abstract class HttpServlet extends GenericServlet { if (REQUEST_FACADE_CLAZZ.isAssignableFrom(req.getClass())) { try { return ((Boolean) GET_ALLOW_TRACE.invoke(req, (Object[]) null)).booleanValue(); - } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) { + } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException ignore) { // Should never happen given the checks in place. - // Ignore } } } @@ -892,11 +891,11 @@ public abstract class HttpServlet extends GenericServlet { Writer osw = null; try { osw = new OutputStreamWriter(out, encoding); - } catch (UnsupportedEncodingException e) { - // Impossible. - // The same values were used in the constructor. If this method - // gets called then the constructor must have succeeded so the - // above call must also succeed. + } catch (UnsupportedEncodingException ignore) { + /* + * Impossible. The same values were used in the constructor. If this method gets called then the + * constructor must have succeeded so the above call must also succeed. + */ } pw = new PrintWriter(osw); } diff --git a/java/jakarta/websocket/ContainerProvider.java b/java/jakarta/websocket/ContainerProvider.java index 8069d89f41..56381d0dde 100644 --- a/java/jakarta/websocket/ContainerProvider.java +++ b/java/jakarta/websocket/ContainerProvider.java @@ -47,7 +47,7 @@ public abstract class ContainerProvider { Class<WebSocketContainer> clazz = (Class<WebSocketContainer>) Class.forName(DEFAULT_PROVIDER_CLASS_NAME); result = clazz.getConstructor().newInstance(); - } catch (ReflectiveOperationException | IllegalArgumentException | SecurityException e) { + } catch (ReflectiveOperationException | IllegalArgumentException | SecurityException ignore) { // No options left. Just return null. } } diff --git a/java/jakarta/websocket/server/ServerEndpointConfig.java b/java/jakarta/websocket/server/ServerEndpointConfig.java index c09fdca51e..cfe6ed7d43 100644 --- a/java/jakarta/websocket/server/ServerEndpointConfig.java +++ b/java/jakarta/websocket/server/ServerEndpointConfig.java @@ -182,7 +182,7 @@ public interface ServerEndpointConfig extends EndpointConfig { @SuppressWarnings("unchecked") Class<Configurator> clazz = (Class<Configurator>) Class.forName(DEFAULT_IMPL_CLASSNAME); result = clazz.getConstructor().newInstance(); - } catch (ReflectiveOperationException | IllegalArgumentException | SecurityException e) { + } catch (ReflectiveOperationException | IllegalArgumentException | SecurityException ignore) { // No options left. Just return null. } } diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java index dff9aefbef..8f99ee8a12 100644 --- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java +++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java @@ -726,12 +726,13 @@ public abstract class AuthenticatorBase extends ValveBase implements Authenticat Class<?> clazz = null; try { clazz = Class.forName(jaspicCallbackHandlerClass, true, Thread.currentThread().getContextClassLoader()); - } catch (ClassNotFoundException e) { - // Proceed with the retry below + } catch (ClassNotFoundException ignore) { + // Not found in the context class loader (web application class loader). Re-try below. } try { if (clazz == null) { + // Look in the same class loader that loaded this class - usually Tomcat's common loader. clazz = Class.forName(jaspicCallbackHandlerClass); } callbackHandler = (CallbackHandler) clazz.getConstructor().newInstance(); diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java index 373c0bc0ce..c8414eed31 100644 --- a/java/org/apache/catalina/connector/CoyoteAdapter.java +++ b/java/org/apache/catalina/connector/CoyoteAdapter.java @@ -261,8 +261,8 @@ public class CoyoteAdapter implements Adapter { success = false; } } catch (IOException e) { + // Issues that should be logged will have already been logged success = false; - // Ignore } catch (Throwable t) { ExceptionUtils.handleThrowable(t); success = false; @@ -373,8 +373,8 @@ public class CoyoteAdapter implements Adapter { response.finishResponse(); } - } catch (IOException e) { - // Ignore + } catch (IOException ignore) { + // Issues that should be logged will have already been logged } finally { AtomicBoolean error = new AtomicBoolean(false); res.action(ActionCode.IS_ERROR, error); @@ -782,8 +782,8 @@ public class CoyoteAdapter implements Adapter { // point. try { Thread.sleep(1000); - } catch (InterruptedException e) { - // Should never happen + } catch (InterruptedException ignore) { + // Should never happen but, if it does, just continue looping } // Reset mapping request.getMappingData().recycle(); diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java index fa01f21bc6..5f673f10f0 100644 --- a/java/org/apache/catalina/connector/Request.java +++ b/java/org/apache/catalina/connector/Request.java @@ -2305,8 +2305,8 @@ public class Request implements HttpServletRequest { Session session = null; try { session = manager.findSession(requestedSessionId); - } catch (IOException e) { - // Can't find the session + } catch (IOException ignore) { + // Error looking up session. Treat it as not found. } if ((session == null) || !session.isValid()) { @@ -2318,8 +2318,8 @@ public class Request implements HttpServletRequest { if (ctxt.getManager().findSession(requestedSessionId) != null) { return true; } - } catch (IOException e) { - // Ignore + } catch (IOException ignore) { + // Error looking up session. Treat it as not found. } } } @@ -2830,9 +2830,8 @@ public class Request implements HttpServletRequest { found = true; break; } - } catch (IOException e) { - // Ignore. Problems with this manager will be - // handled elsewhere. + } catch (IOException ignore) { + // Error looking up session. Treat it as not found. } } } @@ -2942,7 +2941,7 @@ public class Request implements HttpServletRequest { scookie.getValue().getByteChunk().setCharset(getCookieProcessor().getCharset()); cookie.setValue(unescape(scookie.getValue().toString())); cookies[idx++] = cookie; - } catch (IllegalArgumentException e) { + } catch (IllegalArgumentException ignore) { // Ignore bad cookie } } diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index ade4ac0090..220ad31a70 100644 --- a/java/org/apache/catalina/core/StandardContext.java +++ b/java/org/apache/catalina/core/StandardContext.java @@ -5012,9 +5012,8 @@ public class StandardContext extends ContainerBase implements Context, Notificat if (isUseNaming()) { try { ContextBindings.bindThread(this, getNamingToken()); - } catch (NamingException e) { - // Silent catch, as this is a normal case during the early - // startup stages + } catch (NamingException ignore) { + // Silent catch, as this is a normal case during the early startup stages } } diff --git a/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java b/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java index b23ee05aa9..c22baa9184 100644 --- a/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java +++ b/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java @@ -258,8 +258,8 @@ public class JvmRouteBinderValve extends ValveBase implements ClusterValve { Session catalinaSession = null; try { catalinaSession = getManager(request).findSession(sessionId); - } catch (IOException e) { - // Hups! + } catch (IOException ignore) { + // Error looking for session using old session ID. Treat it as not found. } String id = sessionId.substring(0, index); String newSessionID = id + "." + localJvmRoute; @@ -270,11 +270,11 @@ public class JvmRouteBinderValve extends ValveBase implements ClusterValve { } else { try { catalinaSession = getManager(request).findSession(newSessionID); - } catch (IOException e) { - // Hups! + } catch (IOException ignore) { + // Error looking for session using new session ID. Treat it as not found. } if (catalinaSession != null) { - // session is rewrite at other request, rewrite this also + // Session was rewritten in other, concurrent request. Rewrite this request also. changeRequestSessionID(request, sessionId, newSessionID); } else { if (log.isDebugEnabled()) { diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java index 766162429c..047af0cf28 100644 --- a/java/org/apache/catalina/servlets/DefaultServlet.java +++ b/java/org/apache/catalina/servlets/DefaultServlet.java @@ -697,7 +697,7 @@ public class DefaultServlet extends HttpServlet { if (resourceInputStream != null) { try { resourceInputStream.close(); - } catch (IOException ioe) { + } catch (IOException ignore) { // Ignore } } @@ -1111,8 +1111,8 @@ public class DefaultServlet extends HttpServlet { if (serveContent) { try { response.setBufferSize(output); - } catch (IllegalStateException e) { - // Silent catch + } catch (IllegalStateException ignore) { + // Content has already been written - this must be an include. Ignore the error and continue. } InputStream renderResult = null; if (ostream == null) { @@ -1225,8 +1225,8 @@ public class DefaultServlet extends HttpServlet { if (serveContent) { try { response.setBufferSize(output); - } catch (IllegalStateException e) { - // Silent catch + } catch (IllegalStateException ignore) { + // Content has already been written - this must be an include. Ignore the error and continue. } if (ostream != null) { if (!checkSendfile(request, response, resource, contentLength, range)) { @@ -1243,7 +1243,7 @@ public class DefaultServlet extends HttpServlet { try { response.setBufferSize(output); } catch (IllegalStateException e) { - // Silent catch + // Content has already been written - this must be an include. Ignore the error and continue. } if (ostream != null) { copy(resource, contentLength, ostream, ranges, contentType); @@ -2047,7 +2047,7 @@ public class DefaultServlet extends HttpServlet { if (reader != null) { try { reader.close(); - } catch (IOException e) { + } catch (IOException ignore) { // Ignore } } @@ -2525,7 +2525,7 @@ public class DefaultServlet extends HttpServlet { long headerValueTime = -1L; try { headerValueTime = request.getDateHeader("If-Range"); - } catch (IllegalArgumentException e) { + } catch (IllegalArgumentException ignore) { // Ignore } if (headerValueTime >= 0) { diff --git a/java/org/apache/catalina/session/PersistentManagerBase.java b/java/org/apache/catalina/session/PersistentManagerBase.java index 9b6f46780f..dd7d28ea19 100644 --- a/java/org/apache/catalina/session/PersistentManagerBase.java +++ b/java/org/apache/catalina/session/PersistentManagerBase.java @@ -591,7 +591,7 @@ public abstract class PersistentManagerBase extends ManagerBase implements Store for (Session session : sessions) { try { swapOut(session); - } catch (IOException e) { + } catch (IOException ignore) { // This is logged in writeSession() } } @@ -796,9 +796,9 @@ public abstract class PersistentManagerBase extends ManagerBase implements Store } else { store.save(session); } - } catch (IOException e) { - log.error(sm.getString("persistentManager.serializeError", session.getIdInternal()), e); - throw e; + } catch (IOException ioe) { + log.error(sm.getString("persistentManager.serializeError", session.getIdInternal()), ioe); + throw ioe; } } @@ -899,7 +899,7 @@ public abstract class PersistentManagerBase extends ManagerBase implements Store } try { swapOut(session); - } catch (IOException e) { + } catch (IOException ignore) { // This is logged in writeSession() } } @@ -949,7 +949,7 @@ public abstract class PersistentManagerBase extends ManagerBase implements Store } try { swapOut(session); - } catch (IOException e) { + } catch (IOException ignore) { // This is logged in writeSession() } toswap--; @@ -994,7 +994,7 @@ public abstract class PersistentManagerBase extends ManagerBase implements Store try { writeSession(session); - } catch (IOException e) { + } catch (IOException ignore) { // This is logged in writeSession() } session.setNote(PERSISTED_LAST_ACCESSED_TIME, Long.valueOf(lastAccessedTime)); diff --git a/java/org/apache/catalina/storeconfig/StoreFactoryBase.java b/java/org/apache/catalina/storeconfig/StoreFactoryBase.java index a925c4c783..e24e946108 100644 --- a/java/org/apache/catalina/storeconfig/StoreFactoryBase.java +++ b/java/org/apache/catalina/storeconfig/StoreFactoryBase.java @@ -160,9 +160,9 @@ public class StoreFactoryBase implements IStoreFactory { for (Object element : elements) { try { storeElement(aWriter, indent, element); - } catch (IOException ioe) { - // ignore children report error them self! - // see StandardContext.storeWithBackup() + } catch (IOException ignore) { + // Ignore. Children report error themselves. + // See StandardContext.storeWithBackup() } } } diff --git a/java/org/apache/catalina/tribes/membership/LocalStrings.properties b/java/org/apache/catalina/tribes/membership/LocalStrings.properties index 7cad5730f0..5fe8885dd6 100644 --- a/java/org/apache/catalina/tribes/membership/LocalStrings.properties +++ b/java/org/apache/catalina/tribes/membership/LocalStrings.properties @@ -30,6 +30,7 @@ mcastServiceImpl.bind=Attempting to bind the multicast socket to [{0}:{1}] mcastServiceImpl.bind.failed=Binding to multicast address, failed. Binding to port only. mcastServiceImpl.error.receiving=Error receiving mcast package. Sleeping 500ms mcastServiceImpl.error.receivingNoSleep=Error receiving multicast package +mcastServiceImpl.error.stop=Error during stop of membership service mcastServiceImpl.invalid.startLevel=Invalid start level. Only acceptable levels are Channel.MBR_RX_SEQ and Channel.MBR_TX_SEQ mcastServiceImpl.invalid.stopLevel=Invalid stop level. Only acceptable levels are Channel.MBR_RX_SEQ and Channel.MBR_TX_SEQ mcastServiceImpl.invalidMemberPackage=Invalid member multicast package diff --git a/java/org/apache/catalina/tribes/membership/McastServiceImpl.java b/java/org/apache/catalina/tribes/membership/McastServiceImpl.java index a0c77bc679..ad6959e31a 100644 --- a/java/org/apache/catalina/tribes/membership/McastServiceImpl.java +++ b/java/org/apache/catalina/tribes/membership/McastServiceImpl.java @@ -333,13 +333,19 @@ public class McastServiceImpl extends MembershipProviderBase { // leave mcast group try { socket.leaveGroup(new InetSocketAddress(address, 0), null); - } catch (Exception ignore) { - // NO-OP + } catch (Exception e) { + // Shutting down. Only log at debug. + if (log.isDebugEnabled()) { + log.debug(sm.getString("mcastServiceImpl.error.stop"), e); + } } try { socket.close(); - } catch (Exception ignore) { - // NO-OP + } catch (Exception e) { + // Shutting down. Only log at debug. + if (log.isDebugEnabled()) { + log.debug(sm.getString("mcastServiceImpl.error.stop"), e); + } } member.setServiceStartTime(-1); } @@ -365,10 +371,11 @@ public class McastServiceImpl extends MembershipProviderBase { memberBroadcastsReceived(data); } } - } catch (SocketTimeoutException x) { - // do nothing, this is normal, we don't want to block forever - // since the receive thread is the same thread - // that does membership expiration + } catch (SocketTimeoutException ignore) { + /* + * Do nothing. This is normal. We don't want to block forever since the receive thread is the same thread + * that does membership expiration. + */ } checkExpired(); } diff --git a/java/org/apache/catalina/tribes/transport/RxTaskPool.java b/java/org/apache/catalina/tribes/transport/RxTaskPool.java index 667e0fdd98..7da1e25c9a 100644 --- a/java/org/apache/catalina/tribes/transport/RxTaskPool.java +++ b/java/org/apache/catalina/tribes/transport/RxTaskPool.java @@ -63,8 +63,8 @@ public class RxTaskPool { if (!idle.isEmpty()) { try { worker = idle.remove(0); - } catch (java.util.NoSuchElementException x) { - // this means that there are no available workers + } catch (java.util.NoSuchElementException ignore) { + // Should never happen as access to idle is always synchronized on mutex } } else if (used.size() < this.maxTasks && creator != null) { worker = creator.createRxTask(); diff --git a/java/org/apache/catalina/tribes/transport/nio/ParallelNioSender.java b/java/org/apache/catalina/tribes/transport/nio/ParallelNioSender.java index 71ac24ca3c..a18a7a788b 100644 --- a/java/org/apache/catalina/tribes/transport/nio/ParallelNioSender.java +++ b/java/org/apache/catalina/tribes/transport/nio/ParallelNioSender.java @@ -366,7 +366,7 @@ public class ParallelNioSender extends AbstractSender implements MultiPointSende setConnected(false); try { close(); - } catch (Exception e) { + } catch (Exception ignore) { // Ignore } } @@ -399,8 +399,9 @@ public class ParallelNioSender extends AbstractSender implements MultiPointSende if (result) { try { state.selector.selectNow(); - } catch (Exception e) { - /* Ignore */} + } catch (Exception ignore) { + // Ignore + } } return result; } @@ -422,7 +423,7 @@ public class ParallelNioSender extends AbstractSender implements MultiPointSende NioSender nioSender = iter.next(); try { nioSender.disconnect(); - } catch (Exception e) { + } catch (Exception ignore) { // Ignore } iter.remove(); diff --git a/java/org/apache/catalina/tribes/util/StringManager.java b/java/org/apache/catalina/tribes/util/StringManager.java index 9b95b64b6f..0f2547c543 100644 --- a/java/org/apache/catalina/tribes/util/StringManager.java +++ b/java/org/apache/catalina/tribes/util/StringManager.java @@ -68,14 +68,15 @@ public class StringManager { try { bnd = ResourceBundle.getBundle(bundleName, locale); } catch (MissingResourceException ex) { - // Try from the current loader (that's the case for trusted apps) - // Should only be required if using a TC5 style classloader structure - // where common != shared != server + /* + * Try from the current loader (that's the case for trusted apps). Should only be required if using a class + * loader structure where common != shared != server + */ ClassLoader cl = Thread.currentThread().getContextClassLoader(); if (cl != null) { try { bnd = ResourceBundle.getBundle(bundleName, locale, cl); - } catch (MissingResourceException ex2) { + } catch (MissingResourceException ignore) { // Ignore } } @@ -106,8 +107,7 @@ public class StringManager { */ public String getString(String key) { if (key == null) { - String msg = "key may not have a null value"; - throw new IllegalArgumentException(msg); + throw new IllegalArgumentException("key may not have a null value"); } String str = null; @@ -117,7 +117,7 @@ public class StringManager { if (bundle != null) { str = bundle.getString(key); } - } catch (MissingResourceException mre) { + } catch (MissingResourceException ignore) { // bad: shouldn't mask an exception the following way: // str = "[cannot find message associated with key '" + key + // "' due to " + mre + "]"; diff --git a/java/org/apache/catalina/valves/rewrite/LocalStrings.properties b/java/org/apache/catalina/valves/rewrite/LocalStrings.properties index 0db582a3b2..8b09031857 100644 --- a/java/org/apache/catalina/valves/rewrite/LocalStrings.properties +++ b/java/org/apache/catalina/valves/rewrite/LocalStrings.properties @@ -18,6 +18,8 @@ quotedStringTokenizer.tokenizeError=Error tokenizing text [{0}] after position [{1}] from mode [{2}] +resolverImpl.tlsError=Unable to obtain TLS information + rewriteMap.tooManyParameters=Too many parameters for this map rewriteMap.txtInvalidLine=Invalid line [{0}] in text file [{1}] rewriteMap.txtReadError=Error reading text file [{0}] diff --git a/java/org/apache/catalina/valves/rewrite/ResolverImpl.java b/java/org/apache/catalina/valves/rewrite/ResolverImpl.java index 2f3c7a4c7c..fb86c99ca1 100644 --- a/java/org/apache/catalina/valves/rewrite/ResolverImpl.java +++ b/java/org/apache/catalina/valves/rewrite/ResolverImpl.java @@ -34,21 +34,42 @@ import java.util.concurrent.TimeUnit; import org.apache.catalina.WebResource; import org.apache.catalina.WebResourceRoot; import org.apache.catalina.connector.Request; +import org.apache.juli.logging.Log; import org.apache.tomcat.util.http.FastHttpDateFormat; import org.apache.tomcat.util.net.SSLSupport; import org.apache.tomcat.util.net.jsse.PEMFile; import org.apache.tomcat.util.net.openssl.ciphers.Cipher; import org.apache.tomcat.util.net.openssl.ciphers.EncryptionLevel; import org.apache.tomcat.util.net.openssl.ciphers.OpenSSLCipherConfigurationParser; +import org.apache.tomcat.util.res.StringManager; public class ResolverImpl extends Resolver { - protected Request request; + private static final StringManager sm = StringManager.getManager(ResolverImpl.class); + protected final Request request; + private final Log containerLog; + + + /** + * Create a resolver for the given request. + * + * @param request The request + * + * @deprecated Will be removed in Tomcat 12 onwards. Use {@link #ResolverImpl(Request, Log)} + */ + @Deprecated public ResolverImpl(Request request) { + this(request, request.getContext().getLogger()); + } + + + public ResolverImpl(Request request, Log containerLog) { this.request = request; + this.containerLog = containerLog; } + /** * The following are not implemented: * <ul> @@ -228,8 +249,11 @@ public class ResolverImpl extends Resolver { } } } - } catch (IOException e) { + } catch (IOException ioe) { // TLS access error + if (containerLog.isDebugEnabled()) { + containerLog.debug(sm.getString("resolverImpl.tlsError"), ioe); + } } return null; } @@ -275,14 +299,14 @@ public class ResolverImpl extends Resolver { } else if (key.equals("CERT")) { try { return PEMFile.toPEM(certificates[0]); - } catch (CertificateEncodingException e) { + } catch (CertificateEncodingException ignore) { // Ignore } } else if (key.startsWith("CERT_CHAIN_")) { key = key.substring("CERT_CHAIN_".length()); try { return PEMFile.toPEM(certificates[Integer.parseInt(key)]); - } catch (NumberFormatException | ArrayIndexOutOfBoundsException | CertificateEncodingException e) { + } catch (NumberFormatException | ArrayIndexOutOfBoundsException | CertificateEncodingException ignore) { // Ignore } } @@ -317,7 +341,7 @@ public class ResolverImpl extends Resolver { return elements.get(n); } } - } catch (NumberFormatException | ArrayIndexOutOfBoundsException | CertificateParsingException e) { + } catch (NumberFormatException | ArrayIndexOutOfBoundsException | CertificateParsingException ignore) { // Ignore } return null; diff --git a/java/org/apache/catalina/valves/rewrite/RewriteValve.java b/java/org/apache/catalina/valves/rewrite/RewriteValve.java index 2c79aefc2f..08fd570bcd 100644 --- a/java/org/apache/catalina/valves/rewrite/RewriteValve.java +++ b/java/org/apache/catalina/valves/rewrite/RewriteValve.java @@ -320,7 +320,7 @@ public class RewriteValve extends ValveBase { try { - Resolver resolver = new ResolverImpl(request); + Resolver resolver = new ResolverImpl(request, containerLog); invoked.set(Boolean.TRUE); diff --git a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java index 2180d0806d..17ebc866ea 100644 --- a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java +++ b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java @@ -31,9 +31,13 @@ import java.util.zip.ZipFile; import org.apache.catalina.WebResource; import org.apache.catalina.WebResourceRoot; import org.apache.catalina.util.ResourceSet; +import org.apache.juli.logging.Log; +import org.apache.juli.logging.LogFactory; public abstract class AbstractArchiveResourceSet extends AbstractResourceSet { + private static final Log log = LogFactory.getLog(AbstractArchiveResourceSet.class); + private URL baseUrl; private String baseUrlString; protected JarFile archive = null; @@ -342,8 +346,8 @@ public abstract class AbstractArchiveResourceSet extends AbstractResourceSet { if (archive != null && archiveUseCount == 0) { try { archive.close(); - } catch (IOException e) { - // Log at least WARN + } catch (IOException ioe) { + log.warn(sm.getString("abstractArchiveResourceSet.archiveCloseFailed"), ioe); } archive = null; archiveEntries = null; diff --git a/java/org/apache/catalina/webresources/LocalStrings.properties b/java/org/apache/catalina/webresources/LocalStrings.properties index 2590428e09..61ddd56043 100644 --- a/java/org/apache/catalina/webresources/LocalStrings.properties +++ b/java/org/apache/catalina/webresources/LocalStrings.properties @@ -16,6 +16,7 @@ # Do not edit this file directly. # To edit translations see: https://tomcat.apache.org/getinvolved.html#Translations +abstractArchiveResourceSet.archiveCloseFailed=Error closing archive. Archive may still be open. abstractArchiveResourceSet.setReadOnlyFalse=Archive based WebResourceSets such as those based on JARs are hard-coded to be read-only and may not be configured to be read-write abstractFileResourceSet.canonicalfileCheckFailed=Resource for web application [{0}] at path [{1}] was not loaded as the canonical path [{2}] did not match. Use of symlinks is one possible cause. diff --git a/java/org/apache/coyote/ajp/Constants.java b/java/org/apache/coyote/ajp/Constants.java index 2e4e4de60b..1fa5700a6e 100644 --- a/java/org/apache/coyote/ajp/Constants.java +++ b/java/org/apache/coyote/ajp/Constants.java @@ -172,12 +172,8 @@ public final class Constants { private static final Map<String,Integer> responseTransMap = new HashMap<>(20); static { - try { - for (int i = 0; i < SC_RESP_AJP13_MAX; i++) { - responseTransMap.put(getResponseHeaderForCode(i), Integer.valueOf(0xA001 + i)); - } - } catch (Exception e) { - // Do nothing + for (int i = 0; i < SC_RESP_AJP13_MAX; i++) { + responseTransMap.put(getResponseHeaderForCode(i), Integer.valueOf(0xA001 + i)); } } diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java index 20c7da6899..477a5ad7a8 100644 --- a/java/org/apache/coyote/http11/Http11Processor.java +++ b/java/org/apache/coyote/http11/Http11Processor.java @@ -576,7 +576,7 @@ public class Http11Processor extends AbstractProcessor { long contentLength = -1; try { contentLength = request.getContentLengthLong(); - } catch (Exception e) { + } catch (Exception ignore) { // Ignore, an error here is already processed in prepareRequest // but is done again since the content length is still -1 } @@ -758,7 +758,7 @@ public class Http11Processor extends AbstractProcessor { try { hostValueMB = headers.setValue("host"); hostValueMB.setBytes(uriB, uriBCStart + pos, slashPos - pos); - } catch (IllegalStateException e) { + } catch (IllegalStateException ignore) { // Edge case // If the request has too many headers it won't be // possible to create the host header. Ignore this as diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java index 594b3f1bb5..a76d1ecab4 100644 --- a/java/org/apache/coyote/http2/Http2Parser.java +++ b/java/org/apache/coyote/http2/Http2Parser.java @@ -298,7 +298,7 @@ class Http2Parser { // RFC 7450 priority frames are ignored. Still need to treat as overhead. try { swallowPayload(streamId, FrameType.PRIORITY.getId(), 5, false, buffer); - } catch (ConnectionException e) { + } catch (ConnectionException ignore) { // Will never happen because swallowPayload() is called with isPadding set // to false } diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java index 8041b45234..2e17a6eba1 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -526,7 +526,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH try { writeGoAwayFrame((1 << 31) - 1, Http2Error.NO_ERROR.getCode(), null); - } catch (IOException ioe) { + } catch (IOException ignore) { // This is fatal for the connection. Ignore it here. There will be // further attempts at I/O in upgradeDispatch() and it can better // handle the IO errors. @@ -635,7 +635,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH } try { writeGoAwayFrame(maxProcessedStreamId, code, msg); - } catch (IOException ioe) { + } catch (IOException ignore) { // Ignore. GOAWAY is sent on a best efforts basis and the original // error has already been logged. } diff --git a/java/org/apache/coyote/http2/StreamProcessor.java b/java/org/apache/coyote/http2/StreamProcessor.java index b8fb78469d..d92aaffe51 100644 --- a/java/org/apache/coyote/http2/StreamProcessor.java +++ b/java/org/apache/coyote/http2/StreamProcessor.java @@ -314,7 +314,7 @@ class StreamProcessor extends AbstractProcessor implements NonPipeliningProcesso stream.getInputBuffer().insertReplayedBody(body); try { stream.receivedEndOfStream(); - } catch (ConnectionException e) { + } catch (ConnectionException ignore) { // Exception will not be thrown in this case } } diff --git a/java/org/apache/jasper/JspC.java b/java/org/apache/jasper/JspC.java index dbb1daf823..9faddd2bdd 100644 --- a/java/org/apache/jasper/JspC.java +++ b/java/org/apache/jasper/JspC.java @@ -1414,7 +1414,7 @@ public class JspC extends Task implements Options { errorCount++; log.error(Localizer.getMessage("jspc.error.compilation"), e); } - } catch (InterruptedException e) { + } catch (InterruptedException ignore) { // Ignore } } @@ -1518,8 +1518,8 @@ public class JspC extends Task implements Options { mapout.write(Localizer.getMessage("jspc.webinc.footer")); } mapout.close(); - } catch (IOException ioe) { - // nothing to do if it fails since we are done with it + } catch (IOException ignore) { + // Nothing to do if it fails since we are done with it. } } } @@ -1691,7 +1691,7 @@ public class JspC extends Task implements Options { uriRoot = froot.getCanonicalPath(); } } - } catch (IOException ioe) { + } catch (IOException ignore) { // Missing uriRoot will be handled in the caller. } } diff --git a/java/org/apache/jasper/runtime/JspContextWrapper.java b/java/org/apache/jasper/runtime/JspContextWrapper.java index aace947cec..49ae1191d2 100644 --- a/java/org/apache/jasper/runtime/JspContextWrapper.java +++ b/java/org/apache/jasper/runtime/JspContextWrapper.java @@ -193,7 +193,7 @@ public class JspContextWrapper extends PageContext implements VariableResolver { if (getSession() != null) { try { o = rootJspCtxt.getAttribute(name, SESSION_SCOPE); - } catch (IllegalStateException ise) { + } catch (IllegalStateException ignore) { // Session has been invalidated. // Ignore and fall through to application scope. } diff --git a/java/org/apache/jasper/runtime/JspFactoryImpl.java b/java/org/apache/jasper/runtime/JspFactoryImpl.java index 957ea739a6..72c2aa818f 100644 --- a/java/org/apache/jasper/runtime/JspFactoryImpl.java +++ b/java/org/apache/jasper/runtime/JspFactoryImpl.java @@ -102,7 +102,7 @@ public class JspFactoryImpl extends JspFactory { try { pc.initialize(servlet, request, response, errorPageURL, needsSession, bufferSize, autoflush); - } catch (IOException ioe) { + } catch (IOException ignore) { // Implementation never throws IOE but can't change the signature // since it is part of the JSP API } diff --git a/java/org/apache/jasper/runtime/PageContextImpl.java b/java/org/apache/jasper/runtime/PageContextImpl.java index f6df2756f8..00100f3c60 100644 --- a/java/org/apache/jasper/runtime/PageContextImpl.java +++ b/java/org/apache/jasper/runtime/PageContextImpl.java @@ -319,7 +319,7 @@ public class PageContextImpl extends PageContext { if (session.getAttribute(name) != null) { return SESSION_SCOPE; } - } catch (IllegalStateException ise) { + } catch (IllegalStateException ignore) { // Session has been invalidated. // Ignore and fall through to application scope. } @@ -351,7 +351,7 @@ public class PageContextImpl extends PageContext { if (session != null) { try { o = session.getAttribute(name); - } catch (IllegalStateException ise) { + } catch (IllegalStateException ignore) { // Session has been invalidated. // Ignore and fall through to application scope. } @@ -398,7 +398,7 @@ public class PageContextImpl extends PageContext { if (session != null) { try { removeAttribute(name, SESSION_SCOPE); - } catch (IllegalStateException ise) { + } catch (IllegalStateException ignore) { // Session has been invalidated. // Ignore and fall throw to application scope. } diff --git a/java/org/apache/juli/FileHandler.java b/java/org/apache/juli/FileHandler.java index 205bcd339c..3c0a8b18a5 100644 --- a/java/org/apache/juli/FileHandler.java +++ b/java/org/apache/juli/FileHandler.java @@ -366,7 +366,7 @@ public class FileHandler extends Handler { if (encoding != null && !encoding.isEmpty()) { try { setEncoding(encoding); - } catch (UnsupportedEncodingException ex) { + } catch (UnsupportedEncodingException ignore) { // Ignore } } @@ -379,7 +379,7 @@ public class FileHandler extends Handler { if (filterName != null) { try { setFilter((Filter) cl.loadClass(filterName).getConstructor().newInstance()); - } catch (Exception e) { + } catch (Exception ignore) { // Ignore } } @@ -389,7 +389,7 @@ public class FileHandler extends Handler { if (formatterName != null) { try { setFormatter((Formatter) cl.loadClass(formatterName).getConstructor().newInstance()); - } catch (Exception e) { + } catch (Exception ignore) { // Ignore and fallback to defaults setFormatter(new OneLineFormatter()); } @@ -454,14 +454,14 @@ public class FileHandler extends Handler { if (fos != null) { try { fos.close(); - } catch (IOException e1) { + } catch (IOException ignore) { // Ignore } } if (os != null) { try { os.close(); - } catch (IOException e1) { + } catch (IOException ignore) { // Ignore } } @@ -495,8 +495,8 @@ public class FileHandler extends Handler { try { LocalDate dateFromFile = LocalDate.from(DateTimeFormatter.ISO_LOCAL_DATE.parse(date)); result = dateFromFile.isBefore(maxDaysOffset); - } catch (DateTimeException e) { - // no-op + } catch (DateTimeException ignore) { + // Unable to determine date from path. File will not be included. } } return result; diff --git a/java/org/apache/naming/StringManager.java b/java/org/apache/naming/StringManager.java index 3af12442c0..533511f286 100644 --- a/java/org/apache/naming/StringManager.java +++ b/java/org/apache/naming/StringManager.java @@ -69,7 +69,7 @@ public class StringManager { if (cl != null) { try { tempBundle = ResourceBundle.getBundle(bundleName, Locale.getDefault(), cl); - } catch (MissingResourceException ex2) { + } catch (MissingResourceException ignore) { // Ignore } } @@ -94,9 +94,7 @@ public class StringManager { */ public String getString(String key) { if (key == null) { - String msg = "key may not have a null value"; - - throw new IllegalArgumentException(msg); + throw new IllegalArgumentException("key may not have a null value"); } String str = null; @@ -106,7 +104,7 @@ public class StringManager { if (bundle != null) { str = bundle.getString(key); } - } catch (MissingResourceException mre) { + } catch (MissingResourceException ignore) { // bad: shouldn't mask an exception the following way: // str = "[cannot find message associated with key '" + key + "' due to " + mre + "]"; // because it hides the fact that the String was missing diff --git a/java/org/apache/tomcat/util/http/ResponseUtil.java b/java/org/apache/tomcat/util/http/ResponseUtil.java index 092d16b906..f9e90b9100 100644 --- a/java/org/apache/tomcat/util/http/ResponseUtil.java +++ b/java/org/apache/tomcat/util/http/ResponseUtil.java @@ -82,8 +82,8 @@ public class ResponseUtil { StringReader input = new StringReader(varyHeader); try { TokenList.parseTokenList(input, fieldNames); - } catch (IOException ioe) { - // Should never happen + } catch (IOException ignore) { + // Should never happen because a StringReader is used. } } diff --git a/java/org/apache/tomcat/util/http/WebdavIfHeader.java b/java/org/apache/tomcat/util/http/WebdavIfHeader.java index 4316b4b6ea..cc406961aa 100644 --- a/java/org/apache/tomcat/util/http/WebdavIfHeader.java +++ b/java/org/apache/tomcat/util/http/WebdavIfHeader.java @@ -226,8 +226,10 @@ public class WebdavIfHeader { firstChar = readWhiteSpace(reader); reader.reset(); } catch (IOException ignore) { - // may be thrown according to API but is only thrown by the - // StringReader class if the reader is already closed. + /* + * May be thrown according to API but is only thrown by the StringReader class if the reader is + * already closed. + */ } if (firstChar == '(') { diff --git a/java/org/apache/tomcat/util/modeler/BaseModelMBean.java b/java/org/apache/tomcat/util/modeler/BaseModelMBean.java index d19e4192a2..871a534387 100644 --- a/java/org/apache/tomcat/util/modeler/BaseModelMBean.java +++ b/java/org/apache/tomcat/util/modeler/BaseModelMBean.java @@ -298,7 +298,7 @@ public class BaseModelMBean implements DynamicMBean, MBeanRegistration, ModelMBe if (cl != null) { return cl.loadClass(signature); } - } catch (ClassNotFoundException e) { + } catch (ClassNotFoundException ignore) { // Ignore } try { @@ -407,7 +407,7 @@ public class BaseModelMBean implements DynamicMBean, MBeanRegistration, ModelMBe names[n++] = item.getName(); try { setAttribute(item); - } catch (Exception e) { + } catch (Exception ignore) { // Ignore all exceptions } } @@ -467,30 +467,13 @@ public class BaseModelMBean implements DynamicMBean, MBeanRegistration, ModelMBe sm.getString("baseModelMBean.nullResource")); } - // if (!"objectreference".equalsIgnoreCase(type)) - // throw new InvalidTargetObjectTypeException(type); - this.resource = resource; this.resourceType = resource.getClass().getName(); - - // // Make the resource aware of the model mbean. - // try { - // Method m=resource.getClass().getMethod("setModelMBean", - // new Class[] {ModelMBean.class}); - // if( m!= null ) { - // m.invoke(resource, new Object[] {this}); - // } - // } catch( NoSuchMethodException t ) { - // // ignore - // } catch( Throwable t ) { - // log.error( "Can't set model mbean ", t ); - // } } // ------------------------------ ModelMBeanNotificationBroadcaster Methods - @Override public void addAttributeChangeNotificationListener(NotificationListener listener, String name, Object handback) throws IllegalArgumentException { diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index c5fdb0d892..d34ac9c973 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -1320,8 +1320,11 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> } else { readLock.wait(); } - } catch (InterruptedException e) { - // Continue + } catch (InterruptedException ignore) { + /* + * Most likely the Poller signalling there is data to read but could be spurious. Exit + * the wait, check status and proceed accordingly. + */ } } } @@ -1427,8 +1430,11 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> } else { writeLock.wait(); } - } catch (InterruptedException e) { - // Continue + } catch (InterruptedException ignore) { + /* + * Most likely the Poller signalling that data can be written but could be spurious. + * Exit the wait, check status and proceed accordingly. + */ } } else if (startNanos > 0) { // If something was written, reset timeout diff --git a/java/org/apache/tomcat/util/res/StringManager.java b/java/org/apache/tomcat/util/res/StringManager.java index db9d6a6c5e..26b3057cfb 100644 --- a/java/org/apache/tomcat/util/res/StringManager.java +++ b/java/org/apache/tomcat/util/res/StringManager.java @@ -81,7 +81,7 @@ public class StringManager { if (cl != null) { try { bnd = ResourceBundle.getBundle(bundleName, locale, cl); - } catch (MissingResourceException ex2) { + } catch (MissingResourceException ignore) { // Ignore } } @@ -112,8 +112,7 @@ public class StringManager { */ public String getString(String key) { if (key == null) { - String msg = "key may not have a null value"; - throw new IllegalArgumentException(msg); + throw new IllegalArgumentException("key may not have a null value"); } String str = null; @@ -123,7 +122,7 @@ public class StringManager { if (bundle != null) { str = bundle.getString(key); } - } catch (MissingResourceException mre) { + } catch (MissingResourceException ignore) { // bad: shouldn't mask an exception the following way: // str = "[cannot find message associated with key '" + key + // "' due to " + mre + "]"; diff --git a/java/org/apache/tomcat/util/scan/JarFileUrlJar.java b/java/org/apache/tomcat/util/scan/JarFileUrlJar.java index cd94c12e24..168a6527b9 100644 --- a/java/org/apache/tomcat/util/scan/JarFileUrlJar.java +++ b/java/org/apache/tomcat/util/scan/JarFileUrlJar.java @@ -67,11 +67,14 @@ public class JarFileUrlJar implements Jar { boolean multiReleaseValue = false; try { multiReleaseValue = jarFile.isMultiRelease(); - } catch (IllegalStateException e) { - // ISE can be thrown if the JAR URL is bad, for example: - // https://github.com/spring-projects/spring-boot/issues/33633 - // The Javadoc does not document that ISE and given what it does for a vanilla IOE, - // this looks like a Java bug, it should return false instead. + } catch (IllegalStateException ignore) { + /* + * ISE can be thrown if the JAR URL is bad, for example: + * https://github.com/spring-projects/spring-boot/issues/33633 + * + * The Javadoc does not document that ISE and given what it does for a vanilla IOE, this looks like a Java + * bug, it should return false instead. + */ } multiRelease = multiReleaseValue; } @@ -122,7 +125,7 @@ public class JarFileUrlJar implements Jar { if (jarFile != null) { try { jarFile.close(); - } catch (IOException e) { + } catch (IOException ignore) { // Ignore } } diff --git a/java/org/apache/tomcat/websocket/server/WsServerContainer.java b/java/org/apache/tomcat/websocket/server/WsServerContainer.java index 3b588d2bea..431044f2c6 100644 --- a/java/org/apache/tomcat/websocket/server/WsServerContainer.java +++ b/java/org/apache/tomcat/websocket/server/WsServerContainer.java @@ -383,7 +383,7 @@ public class WsServerContainer extends WsWebSocketContainer implements ServerCon for (WsSession wsSession : wsSessions) { try { wsSession.close(AUTHENTICATED_HTTP_SESSION_CLOSED); - } catch (IOException e) { + } catch (IOException ignore) { // Any IOExceptions during close will have been caught and the // onError method called. } diff --git a/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java b/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java index 4252beec09..e4c104e3af 100644 --- a/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java +++ b/test/org/apache/catalina/valves/rewrite/TestResolverSSL.java @@ -160,7 +160,7 @@ public class TestResolverSSL extends TomcatBaseTest { @Override public void invoke(Request request, Response response) throws IOException, ServletException { PrintWriter writer = response.getWriter(); - Resolver resolver = new ResolverImpl(request); + Resolver resolver = new ResolverImpl(request, request.getContext().getLogger()); for (String key : keys) { resolve(key, resolver, writer); } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org