Log exceptions in onFailure instead of bubbling up (elastic/elasticsearch#4709)

Today we wrap the checked IOException in an unchecked exception when
sending back a failure the security transport interceptor. Yet, if that failure handling
in-turn fails due to a broken response channel we should rather log the exception instead
of bubbling it up since it can have unforeseeable side-effects.

Relates to elastic/elasticsearch#4706

* fix line len

Original commit: elastic/x-pack-elasticsearch@03c3826112
This commit is contained in:
Simon Willnauer 2017-01-23 15:08:48 +01:00 committed by GitHub
parent b6703c1515
commit 8651c0ad9f
1 changed files with 15 additions and 11 deletions

View File

@ -5,10 +5,12 @@
*/
package org.elasticsearch.xpack.security.transport;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.DestructiveOperations;
import org.elasticsearch.common.CheckedConsumer;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
import org.elasticsearch.common.util.concurrent.ThreadContext;
@ -38,8 +40,6 @@ import org.elasticsearch.xpack.security.user.SystemUser;
import org.elasticsearch.xpack.security.user.User;
import org.elasticsearch.xpack.ssl.SSLService;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
@ -48,7 +48,7 @@ import java.util.concurrent.Executor;
import static org.elasticsearch.xpack.XPackSettings.TRANSPORT_SSL_ENABLED;
import static org.elasticsearch.xpack.security.Security.setting;
public class SecurityServerTransportInterceptor implements TransportInterceptor {
public class SecurityServerTransportInterceptor extends AbstractComponent implements TransportInterceptor {
private static final String SETTING_NAME = "xpack.security.type";
@ -70,6 +70,7 @@ public class SecurityServerTransportInterceptor implements TransportInterceptor
SSLService sslService,
SecurityContext securityContext,
DestructiveOperations destructiveOperations) {
super(settings);
this.settings = settings;
this.threadPool = threadPool;
this.authcService = authcService;
@ -131,7 +132,7 @@ public class SecurityServerTransportInterceptor implements TransportInterceptor
public <T extends TransportRequest> TransportRequestHandler<T> interceptHandler(String action, String executor,
boolean forceExecution,
TransportRequestHandler<T> actualHandler) {
return new ProfileSecuredRequestHandler<>(action, forceExecution, executor, actualHandler, profileFilters,
return new ProfileSecuredRequestHandler<>(logger, action, forceExecution, executor, actualHandler, profileFilters,
licenseState, threadPool);
}
@ -173,18 +174,20 @@ public class SecurityServerTransportInterceptor implements TransportInterceptor
public static class ProfileSecuredRequestHandler<T extends TransportRequest> implements TransportRequestHandler<T> {
protected final String action;
protected final TransportRequestHandler<T> handler;
private final String action;
private final TransportRequestHandler<T> handler;
private final Map<String, ServerTransportFilter> profileFilters;
private final XPackLicenseState licenseState;
private final ThreadContext threadContext;
private final String executorName;
private final ThreadPool threadPool;
private final boolean forceExecution;
private final Logger logger;
ProfileSecuredRequestHandler(String action, boolean forceExecution, String executorName, TransportRequestHandler<T> handler,
Map<String, ServerTransportFilter> profileFilters, XPackLicenseState licenseState,
ThreadPool threadPool) {
ProfileSecuredRequestHandler(Logger logger, String action, boolean forceExecution, String executorName,
TransportRequestHandler<T> handler, Map<String, ServerTransportFilter> profileFilters,
XPackLicenseState licenseState, ThreadPool threadPool) {
this.logger = logger;
this.action = action;
this.executorName = executorName;
this.handler = handler;
@ -206,8 +209,9 @@ public class SecurityServerTransportInterceptor implements TransportInterceptor
public void onFailure(Exception e) {
try {
channel.sendResponse(e);
} catch (IOException e1) {
throw new UncheckedIOException(e1);
} catch (Exception e1) {
e1.addSuppressed(e);
logger.warn("failed to send exception response for action [" + action + "]", e1);
}
}