From 1cd53c41e21383c0df503704f434344b687c7196 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 4 Jul 2016 08:41:08 -0400 Subject: [PATCH] Do not catch throwable Today throughout the codebase, catch throwable is used with reckless abandon. This is dangerous because the throwable could be a fatal virtual machine error resulting from an internal error in the JVM, or an out of memory error or a stack overflow error that leaves the virtual machine in an unstable and unpredictable state. This commit removes catch throwable from the codebase and removes the temptation to use it by modifying listener APIs to receive instances of Exception instead of the top-level Throwable. Relates elastic/elasticsearch#2694 Original commit: elastic/x-pack-elasticsearch@7ecdd7d9783721ad4851e3ce65d9c83ec1ccb34d --- .../messy/tests/ScriptConditionTests.java | 5 +- .../action/TransportGraphExploreAction.java | 10 ++-- .../xpack/graph/test/GraphTests.java | 5 +- .../delete/TransportDeleteLicenseAction.java | 2 +- .../action/put/TransportPutLicenseAction.java | 2 +- .../license/plugin/core/LicensesService.java | 4 +- .../AbstractLicensesIntegrationTestCase.java | 4 +- .../license/plugin/TestUtils.java | 2 +- .../core/LicensesAcknowledgementTests.java | 2 +- .../core/LicensesManagerServiceTests.java | 2 +- .../action/MonitoringBulkResponse.java | 2 +- .../action/TransportMonitoringBulkAction.java | 4 +- .../xpack/monitoring/agent/AgentService.java | 4 +- .../agent/exporter/http/HttpExporter.java | 7 +-- .../agent/exporter/local/LocalExporter.java | 6 +-- .../monitoring/cleaner/CleanerService.java | 8 ++-- .../action/MonitoringBulkTests.java | 6 +-- .../TransportMonitoringBulkActionTests.java | 4 +- .../agent/exporter/ExportersTests.java | 6 +-- .../security/SecurityLifecycleService.java | 6 +-- .../security/SecurityTemplateService.java | 4 +- .../action/filter/SecurityActionFilter.java | 9 ++-- .../role/TransportDeleteRoleAction.java | 2 +- .../action/role/TransportGetRolesAction.java | 4 +- .../action/role/TransportPutRoleAction.java | 2 +- .../user/TransportChangePasswordAction.java | 3 +- .../user/TransportDeleteUserAction.java | 2 +- .../action/user/TransportGetUsersAction.java | 4 +- .../action/user/TransportPutUserAction.java | 2 +- .../security/audit/index/IndexAuditTrail.java | 2 +- .../authc/esnative/NativeUsersStore.java | 47 ++++++++++--------- .../authc/esnative/ReservedRealm.java | 2 +- .../authc/file/FileUserPasswdStore.java | 7 +-- .../authc/file/FileUserRolesStore.java | 7 +-- .../security/authc/support/DnRoleMapper.java | 4 +- .../security/authz/store/FileRolesStore.java | 7 +-- .../authz/store/NativeRolesStore.java | 16 +++---- .../crypto/InternalCryptoService.java | 40 ++++++---------- .../security/ssl/AbstractSSLService.java | 4 +- .../support/SelfReschedulingRunnable.java | 4 +- .../SecurityClientTransportService.java | 4 +- .../SecurityServerTransportService.java | 15 +++--- .../netty/SecurityNettyTransport.java | 4 +- .../integration/ClearRealmsCacheTests.java | 2 +- .../role/TransportDeleteRoleActionTests.java | 12 ++--- .../role/TransportGetRolesActionTests.java | 16 +++---- .../role/TransportPutRoleActionTests.java | 12 ++--- .../TransportAuthenticateActionTests.java | 6 +-- .../TransportChangePasswordActionTests.java | 14 +++--- .../user/TransportDeleteUserActionTests.java | 16 +++---- .../user/TransportGetUsersActionTests.java | 20 ++++---- .../user/TransportPutUserActionTests.java | 16 +++---- .../security/audit/AuditTrailModuleTests.java | 8 +--- .../authc/file/FileUserRolesStoreTests.java | 7 +-- .../authc/ldap/LdapSessionFactoryTests.java | 12 ++--- .../SelfReschedulingRunnableTests.java | 8 ++-- .../netty/HandshakeWaitingHandlerTests.java | 6 +-- .../extensions/XPackExtensionsService.java | 5 +- .../action/TransportXPackInfoActionTests.java | 2 +- .../watcher/WatcherLifeCycleService.java | 4 +- .../watcher/execution/ExecutionService.java | 2 +- .../execution/TriggeredWatchStore.java | 6 +-- .../support/SearchRequestEquivalence.java | 4 +- .../actions/get/TransportGetWatchAction.java | 6 +-- .../execution/ManualExecutionTests.java | 4 +- 65 files changed, 216 insertions(+), 258 deletions(-) diff --git a/elasticsearch/qa/messy-test-watcher-with-groovy/src/test/java/org/elasticsearch/messy/tests/ScriptConditionTests.java b/elasticsearch/qa/messy-test-watcher-with-groovy/src/test/java/org/elasticsearch/messy/tests/ScriptConditionTests.java index 9c3679edc47..39ff34d307d 100644 --- a/elasticsearch/qa/messy-test-watcher-with-groovy/src/test/java/org/elasticsearch/messy/tests/ScriptConditionTests.java +++ b/elasticsearch/qa/messy-test-watcher-with-groovy/src/test/java/org/elasticsearch/messy/tests/ScriptConditionTests.java @@ -165,14 +165,15 @@ public class ScriptConditionTests extends ESTestCase { public void testScriptConditionThrowException() throws Exception { ScriptServiceProxy scriptService = getScriptServiceProxy(tp); ExecutableScriptCondition condition = new ExecutableScriptCondition( - new ScriptCondition(Script.inline("assert false").build()), logger, scriptService); + new ScriptCondition(Script.inline("null.foo").build()), logger, scriptService); SearchResponse response = new SearchResponse(InternalSearchResponse.empty(), "", 3, 3, 500L, new ShardSearchFailure[0]); WatchExecutionContext ctx = mockExecutionContext("_name", new Payload.XContent(response)); ScriptCondition.Result result = condition.execute(ctx); assertThat(result, notNullValue()); assertThat(result.status(), is(Condition.Result.Status.FAILURE)); assertThat(result.reason(), notNullValue()); - assertThat(result.reason(), containsString("Assertion")); + assertThat(result.reason(), containsString("NullPointerException")); + assertThat(result.reason(), containsString("Cannot get property 'foo' on null object")); } public void testScriptConditionReturnObject() throws Exception { diff --git a/elasticsearch/x-pack/graph/src/main/java/org/elasticsearch/xpack/graph/action/TransportGraphExploreAction.java b/elasticsearch/x-pack/graph/src/main/java/org/elasticsearch/xpack/graph/action/TransportGraphExploreAction.java index ec4d045ce10..0d4cc3db988 100644 --- a/elasticsearch/x-pack/graph/src/main/java/org/elasticsearch/xpack/graph/action/TransportGraphExploreAction.java +++ b/elasticsearch/x-pack/graph/src/main/java/org/elasticsearch/xpack/graph/action/TransportGraphExploreAction.java @@ -509,7 +509,7 @@ public class TransportGraphExploreAction extends HandledTransportAction 0) { - throw ((ShardSearchFailure) response.getShardFailures()[0]).getCause(); + expectedError = response.getShardFailures()[0].getCause(); } - } catch (Throwable rte) { + } catch (Exception rte) { expectedError = rte; - } assertNotNull(expectedError); String message = expectedError.toString(); diff --git a/elasticsearch/x-pack/license-plugin/src/main/java/org/elasticsearch/license/plugin/action/delete/TransportDeleteLicenseAction.java b/elasticsearch/x-pack/license-plugin/src/main/java/org/elasticsearch/license/plugin/action/delete/TransportDeleteLicenseAction.java index d3b27d58012..f36a6b3f5e4 100644 --- a/elasticsearch/x-pack/license-plugin/src/main/java/org/elasticsearch/license/plugin/action/delete/TransportDeleteLicenseAction.java +++ b/elasticsearch/x-pack/license-plugin/src/main/java/org/elasticsearch/license/plugin/action/delete/TransportDeleteLicenseAction.java @@ -59,7 +59,7 @@ public class TransportDeleteLicenseAction extends TransportMasterNodeActionreadThrowable()); + this(in.readException()); } @Override diff --git a/elasticsearch/x-pack/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringBulkAction.java b/elasticsearch/x-pack/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringBulkAction.java index 6c0a83b8a3e..66acc6d28f6 100644 --- a/elasticsearch/x-pack/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringBulkAction.java +++ b/elasticsearch/x-pack/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringBulkAction.java @@ -114,8 +114,8 @@ public class TransportMonitoringBulkAction extends HandledTransportAction exporting thread [{}] exports {} documents", threadNum, threadDocs); threads[i] = new Thread(new AbstractRunnable() { @Override - public void onFailure(Throwable t) { - logger.error("unexpected error in exporting thread", t); - exceptions.add(t); + public void onFailure(Exception e) { + logger.error("unexpected error in exporting thread", e); + exceptions.add(e); } @Override diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java index f174df21d52..f977bee3a51 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java @@ -76,7 +76,7 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust if (nativeUserStore.canStart(event.state(), master)) { threadPool.generic().execute(new AbstractRunnable() { @Override - public void onFailure(Throwable throwable) { + public void onFailure(Exception throwable) { logger.error("failed to start native user store service", throwable); assert false : "security lifecycle services startup failed"; } @@ -95,7 +95,7 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust if (nativeRolesStore.canStart(event.state(), master)) { threadPool.generic().execute(new AbstractRunnable() { @Override - public void onFailure(Throwable throwable) { + public void onFailure(Exception throwable) { logger.error("failed to start native roles store services", throwable); assert false : "security lifecycle services startup failed"; } @@ -117,7 +117,7 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust threadPool.generic().execute(new AbstractRunnable() { @Override - public void onFailure(Throwable throwable) { + public void onFailure(Exception throwable) { logger.error("failed to start index audit trail services", throwable); assert false : "security lifecycle services startup failed"; } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityTemplateService.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityTemplateService.java index eef53799cf2..d4d580a4978 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityTemplateService.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityTemplateService.java @@ -90,8 +90,8 @@ public class SecurityTemplateService extends AbstractComponent implements Cluste if (createTemplate && templateCreationPending.compareAndSet(false, true)) { threadPool.generic().execute(new AbstractRunnable() { @Override - public void onFailure(Throwable t) { - logger.warn("failed to create security index template", t); + public void onFailure(Exception e) { + logger.warn("failed to create security index template", e); templateCreationPending.set(false); } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java index bafdacac3e3..6adde44d6dc 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java @@ -44,9 +44,6 @@ import java.util.function.Predicate; import static org.elasticsearch.xpack.security.support.Exceptions.authorizationError; -/** - * - */ public class SecurityActionFilter extends AbstractComponent implements ActionFilter { private static final Predicate LICENSE_EXPIRATION_ACTION_MATCHER = HealthAndStatsPrivilege.INSTANCE.predicate(); @@ -109,8 +106,8 @@ public class SecurityActionFilter extends AbstractComponent implements ActionFil } else { chain.proceed(task, action, request, listener); } - } catch (Throwable t) { - listener.onFailure(t); + } catch (Exception e) { + listener.onFailure(e); } } @@ -231,7 +228,7 @@ public class SecurityActionFilter extends AbstractComponent implements ActionFil } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { if (threadContext != null) { threadContext.restore(); } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleAction.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleAction.java index 48b65921541..7b5f50c7aed 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleAction.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleAction.java @@ -44,7 +44,7 @@ public class TransportDeleteRoleAction extends HandledTransportAction passwordHash = new AtomicReference<>(); - final AtomicReference failure = new AtomicReference<>(); + final AtomicReference failure = new AtomicReference<>(); final CountDownLatch latch = new CountDownLatch(1); client.prepareGet(SecurityTemplateService.SECURITY_INDEX_NAME, RESERVED_USER_DOC_TYPE, username) .execute(new LatchedActionListener<>(new ActionListener() { @@ -615,7 +615,7 @@ public class NativeUsersStore extends AbstractComponent implements ClusterStateL } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { if (e instanceof IndexNotFoundException) { logger.trace("could not retrieve built in user [{}] password since security index does not exist", e, username); } else { @@ -634,7 +634,7 @@ public class NativeUsersStore extends AbstractComponent implements ClusterStateL failure.set(e); } - Throwable failureCause = failure.get(); + Exception failureCause = failure.get(); if (failureCause != null) { // if there is any sort of failure we need to throw an exception to prevent the fallback to the default password... throw failureCause; @@ -651,7 +651,7 @@ public class NativeUsersStore extends AbstractComponent implements ClusterStateL } @Override - public void onFailure(Throwable t) { + public void onFailure(Exception t) { // Not really much to do here except for warn about it... logger.warn("failed to clear scroll [{}]", t, scrollId); } @@ -669,7 +669,7 @@ public class NativeUsersStore extends AbstractComponent implements ClusterStateL } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { logger.error("unable to clear realm cache for user [{}]", e, username); ElasticsearchException exception = new ElasticsearchException("clearing the cache for [" + username + "] failed. please clear the realm cache manually", e); @@ -838,21 +838,22 @@ public class NativeUsersStore extends AbstractComponent implements ClusterStateL } // call listeners - Throwable th = null; + RuntimeException ex = null; for (ChangeListener listener : listeners) { try { listener.onUsersChanged(changedUsers); - } catch (Throwable t) { - th = ExceptionsHelper.useOrSuppress(th, t); + } catch (Exception e) { + if (ex == null) ex = new RuntimeException("exception while notifying listeners"); + ex.addSuppressed(e); } } - ExceptionsHelper.reThrowIfNotNull(th); + if (ex != null) throw ex; } @Override - public void onFailure(Throwable t) { - logger.error("error occurred while checking the native users for changes", t); + public void onFailure(Exception e) { + logger.error("error occurred while checking the native users for changes", e); } private boolean isStopped() { diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealm.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealm.java index 2c2d0a09a29..52aa20550af 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealm.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealm.java @@ -131,7 +131,7 @@ public class ReservedRealm extends CachingUsernamePasswordRealm { return DEFAULT_PASSWORD_HASH; } return passwordHash; - } catch (Throwable e) { + } catch (Exception e) { logger.error("failed to retrieve password hash for reserved user [{}]", e, username); return null; } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserPasswdStore.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserPasswdStore.java index ebd14bae17b..8190ac0e692 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserPasswdStore.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserPasswdStore.java @@ -37,9 +37,6 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableMap; import static org.elasticsearch.xpack.security.support.SecurityFiles.openAtomicMoveWriter; -/** - * - */ public class FileUserPasswdStore { private final ESLogger logger; @@ -111,8 +108,8 @@ public class FileUserPasswdStore { static Map parseFileLenient(Path path, ESLogger logger) { try { return parseFile(path, logger); - } catch (Throwable t) { - logger.error("failed to parse users file [{}]. skipping/removing all users...", t, path.toAbsolutePath()); + } catch (Exception e) { + logger.error("failed to parse users file [{}]. skipping/removing all users...", e, path.toAbsolutePath()); return emptyMap(); } } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStore.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStore.java index b5bee42e46b..aa03091112b 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStore.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStore.java @@ -37,9 +37,6 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableMap; import static org.elasticsearch.xpack.security.support.SecurityFiles.openAtomicMoveWriter; -/** - * - */ public class FileUserRolesStore { private static final Pattern USERS_DELIM = Pattern.compile("\\s*,\\s*"); @@ -103,8 +100,8 @@ public class FileUserRolesStore { static Map parseFileLenient(Path path, ESLogger logger) { try { return parseFile(path, logger); - } catch (Throwable t) { - logger.error("failed to parse users_roles file [{}]. skipping/removing all entries...", t, path.toAbsolutePath()); + } catch (Exception e) { + logger.error("failed to parse users_roles file [{}]. skipping/removing all entries...", e, path.toAbsolutePath()); return emptyMap(); } } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapper.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapper.java index bfa74b9f190..2eefccdc2db 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapper.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DnRoleMapper.java @@ -95,8 +95,8 @@ public class DnRoleMapper { public static Map> parseFileLenient(Path path, ESLogger logger, String realmType, String realmName) { try { return parseFile(path, logger, realmType, realmName); - } catch (Throwable t) { - logger.error("failed to parse role mappings file [{}]. skipping/removing all mappings...", t, path.toAbsolutePath()); + } catch (Exception e) { + logger.error("failed to parse role mappings file [{}]. skipping/removing all mappings...", e, path.toAbsolutePath()); return emptyMap(); } } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java index 57f8a16eee7..31be9d091ae 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java @@ -46,9 +46,6 @@ import static java.util.Collections.emptySet; import static java.util.Collections.unmodifiableMap; import static org.elasticsearch.xpack.security.Security.setting; -/** - * - */ public class FileRolesStore extends AbstractLifecycleComponent implements RolesStore { public static final Setting ROLES_FILE_SETTING = @@ -288,8 +285,8 @@ public class FileRolesStore extends AbstractLifecycleComponent implements RolesS try { permissions = parseFile(file, logger, settings); logger.info("updated roles (roles file [{}] changed)", file.toAbsolutePath()); - } catch (Throwable t) { - logger.error("could not reload roles file [{}]. Current roles remain unmodified", t, file.toAbsolutePath()); + } catch (Exception e) { + logger.error("could not reload roles file [{}]. Current roles remain unmodified", e, file.toAbsolutePath()); return; } listener.onRefresh(); diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java index 63786df2aeb..ad8785ebdbe 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java @@ -231,7 +231,7 @@ public class NativeRolesStore extends AbstractComponent implements RolesStore, C } @Override - public void onFailure(Throwable t) { + public void onFailure(Exception t) { // attempt to clear the scroll request if (lastResponse != null && lastResponse.getScrollId() != null) { clearScollRequest(lastResponse.getScrollId()); @@ -277,7 +277,7 @@ public class NativeRolesStore extends AbstractComponent implements RolesStore, C } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { logger.error("failed to delete role from the index", e); listener.onFailure(e); } @@ -311,7 +311,7 @@ public class NativeRolesStore extends AbstractComponent implements RolesStore, C } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { logger.error("failed to put role [{}]", e, request.name()); listener.onFailure(e); } @@ -345,7 +345,7 @@ public class NativeRolesStore extends AbstractComponent implements RolesStore, C } @Override - public void onFailure(Throwable t) { + public void onFailure(Exception t) { if (t instanceof IndexNotFoundException) { logger.trace("failed to retrieve role [{}] since security index does not exist", t, roleId); } else { @@ -403,7 +403,7 @@ public class NativeRolesStore extends AbstractComponent implements RolesStore, C } @Override - public void onFailure(Throwable t) { + public void onFailure(Exception t) { // Not really much to do here except for warn about it... logger.warn("failed to clear scroll [{}] after retrieving roles", t, scrollId); } @@ -441,7 +441,7 @@ public class NativeRolesStore extends AbstractComponent implements RolesStore, C } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { logger.error("unable to clear cache for role [{}]", e, role); ElasticsearchException exception = new ElasticsearchException("clearing the cache for [" + role + "] failed. please clear the role cache manually", e); @@ -568,8 +568,8 @@ public class NativeRolesStore extends AbstractComponent implements RolesStore, C } @Override - public void onFailure(Throwable t) { - logger.error("error occurred while checking the native roles for changes", t); + public void onFailure(Exception e) { + logger.error("error occurred while checking the native roles for changes", e); } private boolean isStopped() { diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/crypto/InternalCryptoService.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/crypto/InternalCryptoService.java index 63b6fe75aca..08155f3b34e 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/crypto/InternalCryptoService.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/crypto/InternalCryptoService.java @@ -12,13 +12,12 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.settings.SettingsModule; import org.elasticsearch.env.Environment; -import org.elasticsearch.xpack.security.authc.support.CharArrays; import org.elasticsearch.watcher.FileChangesListener; import org.elasticsearch.watcher.FileWatcher; import org.elasticsearch.watcher.ResourceWatcherService; import org.elasticsearch.xpack.XPackPlugin; +import org.elasticsearch.xpack.security.authc.support.CharArrays; import javax.crypto.BadPaddingException; import javax.crypto.Cipher; @@ -28,6 +27,7 @@ import javax.crypto.Mac; import javax.crypto.SecretKey; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.SecretKeySpec; + import java.io.IOException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; @@ -45,12 +45,9 @@ import java.util.Objects; import java.util.concurrent.CopyOnWriteArrayList; import java.util.regex.Pattern; -import static org.elasticsearch.xpack.security.authc.support.SecuredString.constantTimeEquals; import static org.elasticsearch.xpack.security.Security.setting; +import static org.elasticsearch.xpack.security.authc.support.SecuredString.constantTimeEquals; -/** - * - */ public class InternalCryptoService extends AbstractLifecycleComponent implements CryptoService { public static final String KEY_ALGO = "HmacSHA512"; @@ -232,8 +229,8 @@ public class InternalCryptoService extends AbstractLifecycleComponent implements base64RandomKey = pieces[2]; receivedSignature = pieces[3].substring(0, length); text = pieces[3].substring(length); - } catch (Throwable t) { - logger.error("error occurred while parsing signed text", t); + } catch (Exception e) { + logger.error("error occurred while parsing signed text", e); throw new IllegalArgumentException("tampered signed text"); } @@ -270,8 +267,8 @@ public class InternalCryptoService extends AbstractLifecycleComponent implements if (constantTimeEquals(sig, receivedSignature)) { return text; } - } catch (Throwable t) { - logger.error("error occurred while verifying signed text", t); + } catch (Exception e) { + logger.error("error occurred while verifying signed text", e); throw new IllegalStateException("error while verifying the signed text"); } @@ -543,29 +540,20 @@ public class InternalCryptoService extends AbstractLifecycleComponent implements } private void callListeners(SecretKey oldSystemKey, SecretKey oldEncryptionKey) { - Throwable th = null; + RuntimeException ex = null; for (Listener listener : listeners) { try { listener.onKeyChange(oldSystemKey, oldEncryptionKey); - } catch (Throwable t) { - if (th == null) { - th = t; - } else { - th.addSuppressed(t); - } + } catch (Exception e) { + if (ex == null) ex = new RuntimeException("exception calling key change listeners"); + ex.addSuppressed(e); } } // all listeners were notified now rethrow - if (th != null) { - logger.error("called all key change listeners but one or more exceptions was thrown", th); - if (th instanceof RuntimeException) { - throw (RuntimeException) th; - } else if (th instanceof Error) { - throw (Error) th; - } else { - throw new RuntimeException(th); - } + if (ex != null) { + logger.error("called all key change listeners but one or more exceptions was thrown", ex); + throw ex; } } } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/ssl/AbstractSSLService.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/ssl/AbstractSSLService.java index 63e69307107..8b1a276da95 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/ssl/AbstractSSLService.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/ssl/AbstractSSLService.java @@ -123,8 +123,8 @@ public abstract class AbstractSSLService extends AbstractComponent { sslEngine.setEnabledCipherSuites(supportedCiphers(sslEngine.getSupportedCipherSuites(), ciphers, false)); } catch (ElasticsearchException e) { throw e; - } catch (Throwable t) { - throw new IllegalArgumentException("failed loading cipher suites [" + Arrays.asList(ciphers) + "]", t); + } catch (Exception e) { + throw new IllegalArgumentException("failed loading cipher suites [" + Arrays.asList(ciphers) + "]", e); } try { diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/support/SelfReschedulingRunnable.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/support/SelfReschedulingRunnable.java index c74e875cfa9..d30a0897deb 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/support/SelfReschedulingRunnable.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/support/SelfReschedulingRunnable.java @@ -49,8 +49,8 @@ public class SelfReschedulingRunnable extends AbstractRunnable { } @Override - public void onFailure(Throwable t) { - logger.warn("failed to run scheduled task", t); + public void onFailure(Exception e) { + logger.warn("failed to run scheduled task", e); } @Override diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityClientTransportService.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityClientTransportService.java index b6de8448652..350e1292afc 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityClientTransportService.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityClientTransportService.java @@ -34,8 +34,8 @@ public class SecurityClientTransportService extends TransportService { try { clientFilter.outbound(action, request); super.sendRequest(node, action, request, options, handler); - } catch (Throwable t) { - handler.handleException(new TransportException("failed sending request", t)); + } catch (Exception e) { + handler.handleException(new TransportException("failed sending request", e)); } } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportService.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportService.java index 589a762eb82..e6026d37464 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportService.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportService.java @@ -38,9 +38,6 @@ import static org.elasticsearch.xpack.security.transport.netty.SecurityNettyTran import static org.elasticsearch.xpack.security.transport.netty.SecurityNettyTransport.PROFILE_CLIENT_AUTH_SETTING; import static org.elasticsearch.xpack.security.transport.netty.SecurityNettyTransport.SSL_SETTING; -/** - * - */ public class SecurityServerTransportService extends TransportService { public static final String SETTING_NAME = "xpack.security.type"; @@ -81,16 +78,16 @@ public class SecurityServerTransportService extends TransportService { try { clientFilter.outbound(action, request); super.sendRequest(node, action, request, options, new ContextRestoreResponseHandler<>(original, handler)); - } catch (Throwable t) { - handler.handleException(new TransportException("failed sending request", t)); + } catch (Exception e) { + handler.handleException(new TransportException("failed sending request", e)); } } } else { try { clientFilter.outbound(action, request); super.sendRequest(node, action, request, options, handler); - } catch (Throwable t) { - handler.handleException(new TransportException("failed sending request", t)); + } catch (Exception e) { + handler.handleException(new TransportException("failed sending request", e)); } } } @@ -194,8 +191,8 @@ public class SecurityServerTransportService extends TransportService { RequestContext context = new RequestContext(request, threadContext); RequestContext.setCurrent(context); handler.messageReceived(request, channel, task); - } catch (Throwable t) { - channel.sendResponse(t); + } catch (Exception e) { + channel.sendResponse(e); } finally { RequestContext.removeCurrent(); } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/netty/SecurityNettyTransport.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/netty/SecurityNettyTransport.java index ae7f224f025..9a4adc8e320 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/netty/SecurityNettyTransport.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/transport/netty/SecurityNettyTransport.java @@ -26,14 +26,12 @@ import org.jboss.netty.channel.ChannelHandlerContext; import org.jboss.netty.channel.ChannelPipeline; import org.jboss.netty.channel.ChannelPipelineFactory; import org.jboss.netty.channel.ChannelStateEvent; -import org.jboss.netty.channel.ExceptionEvent; import org.jboss.netty.channel.SimpleChannelHandler; import org.jboss.netty.handler.ssl.SslHandler; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLParameters; import java.net.InetSocketAddress; -import java.util.Collections; import java.util.List; import static org.elasticsearch.xpack.security.Security.featureEnabledSetting; @@ -113,7 +111,7 @@ public class SecurityNettyTransport extends NettyTransport { } @Override - protected void onException(Channel channel, Throwable e) { + protected void onException(Channel channel, Exception e) { if (isNotSslRecordException(e)) { if (logger.isTraceEnabled()) { logger.trace("received plaintext traffic on a encrypted channel, closing connection {}", e, channel); diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/integration/ClearRealmsCacheTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/integration/ClearRealmsCacheTests.java index a5e9d01bc35..7bb5fa93f25 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/integration/ClearRealmsCacheTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/integration/ClearRealmsCacheTests.java @@ -146,7 +146,7 @@ public class ClearRealmsCacheTests extends SecurityIntegTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { error.set(e); latch.countDown(); } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java index e0afd716a0a..e9560e6cfd4 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java @@ -55,7 +55,7 @@ public class TransportDeleteRoleActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -96,7 +96,7 @@ public class TransportDeleteRoleActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -108,7 +108,7 @@ public class TransportDeleteRoleActionTests extends ESTestCase { } public void testException() { - final Throwable t = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException()); + final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException()); final String roleName = randomFrom("admin", "dept_a", "restricted"); NativeRolesStore rolesStore = mock(NativeRolesStore.class); TransportDeleteRoleAction action = new TransportDeleteRoleAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), @@ -124,7 +124,7 @@ public class TransportDeleteRoleActionTests extends ESTestCase { Object[] args = invocation.getArguments(); assert args.length == 2; ActionListener listener = (ActionListener) args[1]; - listener.onFailure(t); + listener.onFailure(e); return null; } }).when(rolesStore).deleteRole(eq(request), any(ActionListener.class)); @@ -138,14 +138,14 @@ public class TransportDeleteRoleActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); assertThat(responseRef.get(), is(nullValue())); assertThat(throwableRef.get(), is(notNullValue())); - assertThat(throwableRef.get(), is(sameInstance(t))); + assertThat(throwableRef.get(), is(sameInstance(e))); verify(rolesStore, times(1)).deleteRole(eq(request), any(ActionListener.class)); } } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java index 825f2540af5..2205f0cbcf7 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java @@ -89,7 +89,7 @@ public class TransportGetRolesActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -158,7 +158,7 @@ public class TransportGetRolesActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -251,7 +251,7 @@ public class TransportGetRolesActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -273,7 +273,7 @@ public class TransportGetRolesActionTests extends ESTestCase { } public void testException() { - final Throwable t = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException()); + final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException()); final List storeRoleDescriptors = randomRoleDescriptors(); NativeRolesStore rolesStore = mock(NativeRolesStore.class); SecurityContext context = mock(SecurityContext.class); @@ -290,7 +290,7 @@ public class TransportGetRolesActionTests extends ESTestCase { Object[] args = invocation.getArguments(); assert args.length == 2; ActionListener listener = (ActionListener) args[1]; - listener.onFailure(t); + listener.onFailure(e); return null; } }).when(rolesStore).getRoleDescriptor(eq(request.names()[0]), any(ActionListener.class)); @@ -301,7 +301,7 @@ public class TransportGetRolesActionTests extends ESTestCase { Object[] args = invocation.getArguments(); assert args.length == 2; ActionListener> listener = (ActionListener>) args[1]; - listener.onFailure(t); + listener.onFailure(e); return null; } }).when(rolesStore).getRoleDescriptors(aryEq(request.names()), any(ActionListener.class)); @@ -316,13 +316,13 @@ public class TransportGetRolesActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); assertThat(throwableRef.get(), is(notNullValue())); - assertThat(throwableRef.get(), is(t)); + assertThat(throwableRef.get(), is(e)); assertThat(responseRef.get(), is(nullValue())); } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java index 3a7150e4679..6ea5bf6b25c 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java @@ -56,7 +56,7 @@ public class TransportPutRoleActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -97,7 +97,7 @@ public class TransportPutRoleActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -109,7 +109,7 @@ public class TransportPutRoleActionTests extends ESTestCase { } public void testException() { - final Throwable t = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException()); + final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException()); final String roleName = randomFrom("admin", "dept_a", "restricted"); NativeRolesStore rolesStore = mock(NativeRolesStore.class); TransportPutRoleAction action = new TransportPutRoleAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), @@ -124,7 +124,7 @@ public class TransportPutRoleActionTests extends ESTestCase { Object[] args = invocation.getArguments(); assert args.length == 3; ActionListener listener = (ActionListener) args[2]; - listener.onFailure(t); + listener.onFailure(e); return null; } }).when(rolesStore).putRole(eq(request), any(RoleDescriptor.class), any(ActionListener.class)); @@ -138,14 +138,14 @@ public class TransportPutRoleActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); assertThat(responseRef.get(), is(nullValue())); assertThat(throwableRef.get(), is(notNullValue())); - assertThat(throwableRef.get(), is(sameInstance(t))); + assertThat(throwableRef.get(), is(sameInstance(e))); verify(rolesStore, times(1)).putRole(eq(request), any(RoleDescriptor.class), any(ActionListener.class)); } } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java index e7b820642a9..236dce76b97 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java @@ -47,7 +47,7 @@ public class TransportAuthenticateActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -72,7 +72,7 @@ public class TransportAuthenticateActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -99,7 +99,7 @@ public class TransportAuthenticateActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportChangePasswordActionTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportChangePasswordActionTests.java index 450e53caf65..35344ba2394 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportChangePasswordActionTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportChangePasswordActionTests.java @@ -68,7 +68,7 @@ public class TransportChangePasswordActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -97,7 +97,7 @@ public class TransportChangePasswordActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -135,7 +135,7 @@ public class TransportChangePasswordActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -152,13 +152,13 @@ public class TransportChangePasswordActionTests extends ESTestCase { ChangePasswordRequest request = new ChangePasswordRequest(); request.username(user.principal()); request.passwordHash(Hasher.BCRYPT.hash(new SecuredString("changeme".toCharArray()))); - final Throwable t = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException(), new RuntimeException()); + final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException(), new RuntimeException()); doAnswer(new Answer() { public Void answer(InvocationOnMock invocation) { Object[] args = invocation.getArguments(); assert args.length == 2; ActionListener listener = (ActionListener) args[1]; - listener.onFailure(t); + listener.onFailure(e); return null; } }).when(usersStore).changePassword(eq(request), any(ActionListener.class)); @@ -174,14 +174,14 @@ public class TransportChangePasswordActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); assertThat(responseRef.get(), is(nullValue())); assertThat(throwableRef.get(), is(notNullValue())); - assertThat(throwableRef.get(), sameInstance(t)); + assertThat(throwableRef.get(), sameInstance(e)); verify(usersStore, times(1)).changePassword(eq(request), any(ActionListener.class)); } } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportDeleteUserActionTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportDeleteUserActionTests.java index 9735a698e8b..614681491df 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportDeleteUserActionTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportDeleteUserActionTests.java @@ -63,7 +63,7 @@ public class TransportDeleteUserActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -90,7 +90,7 @@ public class TransportDeleteUserActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -118,7 +118,7 @@ public class TransportDeleteUserActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -156,7 +156,7 @@ public class TransportDeleteUserActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -168,7 +168,7 @@ public class TransportDeleteUserActionTests extends ESTestCase { } public void testException() { - final Throwable t = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException(), new RuntimeException()); + final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException(), new RuntimeException()); final User user = new User("joe"); NativeUsersStore usersStore = mock(NativeUsersStore.class); TransportDeleteUserAction action = new TransportDeleteUserAction(Settings.EMPTY, mock(ThreadPool.class), @@ -180,7 +180,7 @@ public class TransportDeleteUserActionTests extends ESTestCase { Object[] args = invocation.getArguments(); assert args.length == 2; ActionListener listener = (ActionListener) args[1]; - listener.onFailure(t); + listener.onFailure(e); return null; } }).when(usersStore).deleteUser(eq(request), any(ActionListener.class)); @@ -194,14 +194,14 @@ public class TransportDeleteUserActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); assertThat(responseRef.get(), is(nullValue())); assertThat(throwableRef.get(), is(notNullValue())); - assertThat(throwableRef.get(), sameInstance(t)); + assertThat(throwableRef.get(), sameInstance(e)); verify(usersStore, times(1)).deleteUser(eq(request), any(ActionListener.class)); } } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportGetUsersActionTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportGetUsersActionTests.java index 4d8e0e32c37..c5573602968 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportGetUsersActionTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportGetUsersActionTests.java @@ -84,7 +84,7 @@ public class TransportGetUsersActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -116,7 +116,7 @@ public class TransportGetUsersActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -147,7 +147,7 @@ public class TransportGetUsersActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -185,7 +185,7 @@ public class TransportGetUsersActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -242,7 +242,7 @@ public class TransportGetUsersActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -261,7 +261,7 @@ public class TransportGetUsersActionTests extends ESTestCase { } public void testException() { - final Throwable t = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException(), new ValidationException()); + final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException(), new ValidationException()); final List storeUsers = randomFrom(Collections.singletonList(new User("joe")), Arrays.asList(new User("jane"), new User("fred")), randomUsers()); final String[] storeUsernames = storeUsers.stream().map(User::principal).collect(Collectors.toList()).toArray(Strings.EMPTY_ARRAY); @@ -277,7 +277,7 @@ public class TransportGetUsersActionTests extends ESTestCase { Object[] args = invocation.getArguments(); assert args.length == 2; ActionListener> listener = (ActionListener>) args[1]; - listener.onFailure(t); + listener.onFailure(e); return null; } }).when(usersStore).getUsers(aryEq(storeUsernames), any(ActionListener.class)); @@ -288,7 +288,7 @@ public class TransportGetUsersActionTests extends ESTestCase { Object[] args = invocation.getArguments(); assert args.length == 2; ActionListener listener = (ActionListener) args[1]; - listener.onFailure(t); + listener.onFailure(e); return null; } }).when(usersStore).getUser(eq(storeUsernames[0]), any(ActionListener.class)); @@ -303,13 +303,13 @@ public class TransportGetUsersActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); assertThat(throwableRef.get(), is(notNullValue())); - assertThat(throwableRef.get(), is(sameInstance(t))); + assertThat(throwableRef.get(), is(sameInstance(e))); assertThat(responseRef.get(), is(nullValue())); if (request.usernames().length == 1) { verify(usersStore, times(1)).getUser(eq(request.usernames()[0]), any(ActionListener.class)); diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportPutUserActionTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportPutUserActionTests.java index a99e0f9da00..52b7a2e247f 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportPutUserActionTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportPutUserActionTests.java @@ -67,7 +67,7 @@ public class TransportPutUserActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -95,7 +95,7 @@ public class TransportPutUserActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -124,7 +124,7 @@ public class TransportPutUserActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -167,7 +167,7 @@ public class TransportPutUserActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); @@ -179,7 +179,7 @@ public class TransportPutUserActionTests extends ESTestCase { } public void testException() { - final Throwable t = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException(), new ValidationException()); + final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException(), new ValidationException()); final User user = new User("joe"); NativeUsersStore usersStore = mock(NativeUsersStore.class); TransportPutUserAction action = new TransportPutUserAction(Settings.EMPTY, mock(ThreadPool.class), @@ -192,7 +192,7 @@ public class TransportPutUserActionTests extends ESTestCase { Object[] args = invocation.getArguments(); assert args.length == 2; ActionListener listener = (ActionListener) args[1]; - listener.onFailure(t); + listener.onFailure(e); return null; } }).when(usersStore).putUser(eq(request), any(ActionListener.class)); @@ -206,14 +206,14 @@ public class TransportPutUserActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { throwableRef.set(e); } }); assertThat(responseRef.get(), is(nullValue())); assertThat(throwableRef.get(), is(notNullValue())); - assertThat(throwableRef.get(), sameInstance(t)); + assertThat(throwableRef.get(), sameInstance(e)); verify(usersStore, times(1)).putUser(eq(request), any(ActionListener.class)); } } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailModuleTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailModuleTests.java index 9072197572d..660304b31b7 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailModuleTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailModuleTests.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.security.audit; -import org.elasticsearch.Version; import org.elasticsearch.common.inject.Guice; import org.elasticsearch.common.inject.Injector; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; @@ -15,20 +14,17 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsModule; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.node.Node; -import org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.local.LocalTransport; +import org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; -/** - * - */ public class AuditTrailModuleTests extends ESTestCase { public void testEnabled() throws Exception { Settings settings = Settings.builder() @@ -93,7 +89,7 @@ public class AuditTrailModuleTests extends ESTestCase { try { Guice.createInjector(settingsModule, new AuditTrailModule(settings)); fail("Expect initialization to fail when an unknown audit trail output is configured"); - } catch (Throwable t) { + } catch (Exception e) { // expected } } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStoreTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStoreTests.java index 43826371603..67285e81451 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStoreTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStoreTests.java @@ -49,9 +49,6 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -/** - * - */ public class FileUserRolesStoreTests extends ESTestCase { private Settings settings; private Environment env; @@ -217,8 +214,8 @@ public class FileUserRolesStoreTests extends ESTestCase { try { FileUserRolesStore.parseFile(file, logger); fail("expected a parse failure"); - } catch (Throwable t) { - this.logger.info("expected", t); + } catch (Exception e) { + this.logger.info("expected", e); } } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java index e745513984f..f4550255b1a 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java @@ -56,9 +56,9 @@ public class LdapSessionFactoryTests extends LdapTestCase { ldapServer.setProcessingDelayMillis(500L); try (LdapSession session = sessionFactory.session(user, userPass)) { fail("expected connection timeout error here"); - } catch (Throwable t) { - assertThat(t, instanceOf(ElasticsearchSecurityException.class)); - assertThat(t.getCause().getMessage(), containsString("A client-side timeout was encountered while waiting ")); + } catch (Exception e) { + assertThat(e, instanceOf(ElasticsearchSecurityException.class)); + assertThat(e.getCause().getMessage(), containsString("A client-side timeout was encountered while waiting ")); } finally { ldapServer.setProcessingDelayMillis(0L); } @@ -85,11 +85,11 @@ public class LdapSessionFactoryTests extends LdapTestCase { long start = System.currentTimeMillis(); try (LdapSession session = sessionFactory.session(user, userPass)) { fail("expected connection timeout error here"); - } catch (Throwable t) { + } catch (Exception e) { long time = System.currentTimeMillis() - start; assertThat(time, lessThan(10000L)); - assertThat(t, instanceOf(IOException.class)); - assertThat(t.getCause().getCause().getMessage(), containsString("within the configured timeout of")); + assertThat(e, instanceOf(IOException.class)); + assertThat(e.getCause().getCause().getMessage(), containsString("within the configured timeout of")); } } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/support/SelfReschedulingRunnableTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/support/SelfReschedulingRunnableTests.java index b81b879e09f..c871da38ea8 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/support/SelfReschedulingRunnableTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/support/SelfReschedulingRunnableTests.java @@ -55,7 +55,7 @@ public class SelfReschedulingRunnableTests extends ESTestCase { final ThreadPool threadPool = mock(ThreadPool.class); final AbstractRunnable runnable = new AbstractRunnable() { @Override - public void onFailure(Throwable throwable) { + public void onFailure(Exception throwable) { } @Override @@ -88,7 +88,7 @@ public class SelfReschedulingRunnableTests extends ESTestCase { final CountDownLatch pauseLatch = new CountDownLatch(1); final AbstractRunnable runnable = new AbstractRunnable() { @Override - public void onFailure(Throwable throwable) { + public void onFailure(Exception throwable) { } @Override @@ -195,7 +195,7 @@ public class SelfReschedulingRunnableTests extends ESTestCase { final AtomicInteger runCounter = new AtomicInteger(0); final AbstractRunnable runnable = new AbstractRunnable() { @Override - public void onFailure(Throwable throwable) { + public void onFailure(Exception throwable) { failureCounter.incrementAndGet(); } @@ -238,7 +238,7 @@ public class SelfReschedulingRunnableTests extends ESTestCase { final CountDownLatch stopCalledLatch = new CountDownLatch(1); final AbstractRunnable runnable = new AbstractRunnable() { @Override - public void onFailure(Throwable throwable) { + public void onFailure(Exception throwable) { throw new IllegalStateException("we should never be in this method!"); } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/transport/netty/HandshakeWaitingHandlerTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/transport/netty/HandshakeWaitingHandlerTests.java index f54934556d9..e848eb44f3d 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/transport/netty/HandshakeWaitingHandlerTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/transport/netty/HandshakeWaitingHandlerTests.java @@ -192,13 +192,13 @@ public class HandshakeWaitingHandlerTests extends ESTestCase { try { serverBootstrap.bind(new InetSocketAddress(InetAddress.getLoopbackAddress(), randomPort)); break; - } catch (Throwable t) { - if (t.getCause() instanceof BindException) { + } catch (Exception e) { + if (e.getCause() instanceof BindException) { logger.error("Tried to bind to port [{}], going to retry", randomPort); randomPort = randomIntBetween(49000, 65500); } if (tries >= maxTries) { - throw new RuntimeException("Failed to start server bootstrap [" + tries + "] times, stopping", t); + throw new RuntimeException("Failed to start server bootstrap [" + tries + "] times, stopping", e); } tries++; } diff --git a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/extensions/XPackExtensionsService.java b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/extensions/XPackExtensionsService.java index 922b86e47de..4c9d903f1de 100644 --- a/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/extensions/XPackExtensionsService.java +++ b/elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/extensions/XPackExtensionsService.java @@ -28,9 +28,6 @@ import java.util.Arrays; import static org.elasticsearch.common.io.FileSystemUtils.isAccessibleDirectory; -/** - * - */ public class XPackExtensionsService { private final Settings settings; @@ -186,7 +183,7 @@ public class XPackExtensionsService { "Settings instance"); } } - } catch (Throwable e) { + } catch (Exception e) { throw new ElasticsearchException("Failed to load extension class [" + extClass.getName() + "]", e); } } diff --git a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/action/TransportXPackInfoActionTests.java b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/action/TransportXPackInfoActionTests.java index 77318043924..fc3b5faf986 100644 --- a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/action/TransportXPackInfoActionTests.java +++ b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/action/TransportXPackInfoActionTests.java @@ -105,7 +105,7 @@ public class TransportXPackInfoActionTests extends ESTestCase { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { error.set(e); latch.countDown(); } diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherLifeCycleService.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherLifeCycleService.java index 5c96c3a3daa..2b3576c27a6 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherLifeCycleService.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherLifeCycleService.java @@ -166,7 +166,7 @@ public class WatcherLifeCycleService extends AbstractComponent implements Cluste } @Override - public void onFailure(Throwable throwable) { + public void onFailure(Exception throwable) { logger.warn("updating manually stopped isn't acked", throwable); latch.countDown(); } @@ -199,7 +199,7 @@ public class WatcherLifeCycleService extends AbstractComponent implements Cluste } @Override - public void onFailure(String source, Throwable throwable) { + public void onFailure(String source, Exception throwable) { latch.countDown(); logger.warn("couldn't update watcher metadata [{}]", throwable, source); } diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java index 219f7941586..488f1a51c9f 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java @@ -206,7 +206,7 @@ public class ExecutionService extends AbstractComponent { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { Throwable cause = ExceptionsHelper.unwrapCause(e); if (cause instanceof EsRejectedExecutionException) { logger.debug("failed to store watch records due to overloaded threadpool [{}]", ExceptionsHelper.detailedMessage(e)); diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/TriggeredWatchStore.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/TriggeredWatchStore.java index 0f5a19aa312..e5619f2096c 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/TriggeredWatchStore.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/TriggeredWatchStore.java @@ -125,7 +125,7 @@ public class TriggeredWatchStore extends AbstractComponent { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { listener.onFailure(e); } }); @@ -149,7 +149,7 @@ public class TriggeredWatchStore extends AbstractComponent { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { listener.onFailure(e); } }); @@ -183,7 +183,7 @@ public class TriggeredWatchStore extends AbstractComponent { } @Override - public void onFailure(Throwable e) { + public void onFailure(Exception e) { listener.onFailure(e); } }); diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/SearchRequestEquivalence.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/SearchRequestEquivalence.java index fec055628f1..f4e40a59817 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/SearchRequestEquivalence.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/SearchRequestEquivalence.java @@ -42,8 +42,8 @@ public final class SearchRequestEquivalence { r2.writeTo(output1); byte[] bytes2 = BytesReference.toBytes(output1.bytes()); return Arrays.equals(bytes1, bytes2); - } catch (Throwable t) { - throw illegalState("could not compare search requests", t); + } catch (Exception e) { + throw illegalState("could not compare search requests", e); } } } diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/get/TransportGetWatchAction.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/get/TransportGetWatchAction.java index cb581d5c470..224f56052f3 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/get/TransportGetWatchAction.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/get/TransportGetWatchAction.java @@ -85,9 +85,9 @@ public class TransportGetWatchAction extends WatcherTransportAction