From fa542bacc078a8689437a79dcd6bfd61d1221e4c Mon Sep 17 00:00:00 2001
From: Stephane Nicoll <snicoll@pivotal.io>
Date: Fri, 13 Apr 2018 16:26:04 +0200
Subject: [PATCH] Translate user-defined exception when invoking JMX operation

This commit makes sure to respect the MBeanServer#invoke contract by
wrapping any user-defined exception in an MBeanException. Also, any
exception not from the JDK is translated, as it may lead to unexpected
issue on the client if that class isn't present. This is consistent
with our operation result mapping strategy.

Closes gh-10448
---
 .../actuate/endpoint/jmx/EndpointMBean.java   | 25 ++++++++++++++--
 .../endpoint/jmx/EndpointMBeanTests.java      | 30 +++++++++++++++++++
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java
index f9f848aecee..2335d0b1407 100644
--- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java
+++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java
@@ -93,13 +93,12 @@ public class EndpointMBean implements DynamicMBean {
 		return invoke(operation, params);
 	}
 
-	private Object invoke(JmxOperation operation, Object[] params) {
+	private Object invoke(JmxOperation operation, Object[] params) throws MBeanException {
 		try {
 			String[] parameterNames = operation.getParameters().stream()
 					.map(JmxOperationParameter::getName).toArray(String[]::new);
 			Map<String, Object> arguments = getArguments(parameterNames, params);
-			Object result = operation
-					.invoke(new InvocationContext(SecurityContext.NONE, arguments));
+			Object result = invokeOperation(operation, arguments);
 			if (REACTOR_PRESENT) {
 				result = ReactiveHandler.handle(result);
 			}
@@ -110,6 +109,26 @@ public class EndpointMBean implements DynamicMBean {
 		}
 	}
 
+	private Object invokeOperation(JmxOperation operation,
+			Map<String, Object> arguments) throws MBeanException {
+		try {
+			return operation.invoke(new InvocationContext(SecurityContext.NONE,
+					arguments));
+		}
+		catch (Exception ex) {
+			throw new MBeanException(translateIfNecessary(ex), ex.getMessage());
+		}
+	}
+
+	private Exception translateIfNecessary(Exception exception) {
+		if (exception.getClass().getName().startsWith("java.")) {
+			return exception;
+		}
+		else {
+			return new IllegalStateException(exception.getMessage());
+		}
+	}
+
 	private Map<String, Object> getArguments(String[] parameterNames, Object[] params) {
 		Map<String, Object> arguments = new HashMap<>();
 		for (int i = 0; i < params.length; i++) {
diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java
index dc70b4cd3b1..4c0cd5d32ae 100644
--- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java
+++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java
@@ -29,6 +29,8 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import reactor.core.publisher.Mono;
 
+import org.springframework.beans.FatalBeanException;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.mockito.Mockito.mock;
@@ -84,6 +86,34 @@ public class EndpointMBeanTests {
 		assertThat(result).isEqualTo("result");
 	}
 
+	@Test
+	public void invokeWhenOperationFailedShouldTranslateException()
+			throws MBeanException, ReflectionException {
+		TestExposableJmxEndpoint endpoint = new TestExposableJmxEndpoint(
+				new TestJmxOperation((arguments) -> {
+					throw new FatalBeanException("test failure");
+				}));
+		EndpointMBean bean = new EndpointMBean(this.responseMapper, endpoint);
+		this.thrown.expect(MBeanException.class);
+		this.thrown.expectCause(instanceOf(IllegalStateException.class));
+		this.thrown.expectMessage("test failure");
+		bean.invoke("testOperation", NO_PARAMS, NO_SIGNATURE);
+	}
+
+	@Test
+	public void invokeWhenOperationFailedWithJdkExceptionShouldReuseException()
+			throws MBeanException, ReflectionException {
+		TestExposableJmxEndpoint endpoint = new TestExposableJmxEndpoint(
+				new TestJmxOperation((arguments) -> {
+					throw new UnsupportedOperationException("test failure");
+				}));
+		EndpointMBean bean = new EndpointMBean(this.responseMapper, endpoint);
+		this.thrown.expect(MBeanException.class);
+		this.thrown.expectCause(instanceOf(UnsupportedOperationException.class));
+		this.thrown.expectMessage("test failure");
+		bean.invoke("testOperation", NO_PARAMS, NO_SIGNATURE);
+	}
+
 	@Test
 	public void invokeWhenActionNameIsNotAnOperationShouldThrowException()
 			throws MBeanException, ReflectionException {