diff --git a/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/CloudFoundryWebEndpointServletHandlerMapping.java b/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/CloudFoundryWebEndpointServletHandlerMapping.java index 178dff7df9a..735b42b251d 100644 --- a/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/CloudFoundryWebEndpointServletHandlerMapping.java +++ b/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/CloudFoundryWebEndpointServletHandlerMapping.java @@ -34,6 +34,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.boot.actuate.endpoint.EndpointInfo; import org.springframework.boot.actuate.endpoint.OperationInvoker; import org.springframework.boot.actuate.endpoint.ParameterMappingException; +import org.springframework.boot.actuate.endpoint.ParametersMissingException; import org.springframework.boot.actuate.endpoint.web.EndpointLinksResolver; import org.springframework.boot.actuate.endpoint.web.Link; import org.springframework.boot.actuate.endpoint.web.WebEndpointOperation; @@ -165,7 +166,7 @@ class CloudFoundryWebEndpointServletHandlerMapping try { return handleResult(this.operationInvoker.invoke(arguments), httpMethod); } - catch (ParameterMappingException ex) { + catch (ParametersMissingException | ParameterMappingException ex) { return new ResponseEntity(HttpStatus.BAD_REQUEST); } } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/audit/AuditEventsWebEndpointExtension.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/audit/AuditEventsWebEndpointExtension.java index 0489c2f1b48..3cde1d36df8 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/audit/AuditEventsWebEndpointExtension.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/audit/AuditEventsWebEndpointExtension.java @@ -22,6 +22,7 @@ import org.springframework.boot.actuate.audit.AuditEventsEndpoint.AuditEventsDes import org.springframework.boot.actuate.endpoint.annotation.ReadOperation; import org.springframework.boot.actuate.endpoint.web.WebEndpointResponse; import org.springframework.boot.actuate.endpoint.web.annotation.WebEndpointExtension; +import org.springframework.lang.Nullable; /** * {@link WebEndpointExtension} for the {@link AuditEventsEndpoint}. @@ -40,10 +41,7 @@ public class AuditEventsWebEndpointExtension { @ReadOperation public WebEndpointResponse eventsWithPrincipalDateAfterAndType( - String principal, Date after, String type) { - if (after == null) { - return new WebEndpointResponse<>(WebEndpointResponse.STATUS_BAD_REQUEST); - } + @Nullable String principal, Date after, @Nullable String type) { AuditEventsDescriptor auditEvents = this.delegate .eventsWithPrincipalDateAfterAndType(principal, after, type); return new WebEndpointResponse<>(auditEvents); diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ParameterMappingException.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ParameterMappingException.java index a7899247466..e0c74742192 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ParameterMappingException.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ParameterMappingException.java @@ -39,7 +39,7 @@ public class ParameterMappingException extends RuntimeException { * @param cause the cause of the mapping failure */ public ParameterMappingException(Object input, Class type, Throwable cause) { - super("Failed to map " + input + " of type " + input.getClass() + " to type " + super("Failed to map " + input + " of type " + input.getClass() + " to type " + type, cause); this.input = input; this.type = type; diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ParametersMissingException.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ParametersMissingException.java new file mode 100644 index 00000000000..8839bef64c6 --- /dev/null +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ParametersMissingException.java @@ -0,0 +1,48 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.endpoint; + +import java.util.Set; + +import org.springframework.util.StringUtils; + +/** + * {@link RuntimeException} thrown when an endpoint invocation does + * not contain required parameters. + * + * @author Madhura Bhave + * @since 2.0.0 + */ +public class ParametersMissingException extends RuntimeException { + + private final Set parameters; + + public ParametersMissingException(Set parameters) { + super("Failed to invoke operation because the following required " + + "parameters were missing: " + StringUtils.collectionToCommaDelimitedString(parameters)); + this.parameters = parameters; + } + + /** + * Returns the parameters that were missing. + * @return the parameters + */ + public Set getParameters() { + return this.parameters; + } +} + diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ReflectiveOperationInvoker.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ReflectiveOperationInvoker.java index 37f48622527..16c092fad0a 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ReflectiveOperationInvoker.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ReflectiveOperationInvoker.java @@ -19,9 +19,11 @@ package org.springframework.boot.actuate.endpoint; import java.lang.reflect.Method; import java.lang.reflect.Parameter; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.springframework.lang.Nullable; import org.springframework.util.ReflectionUtils; /** @@ -56,10 +58,30 @@ public class ReflectiveOperationInvoker implements OperationInvoker { @Override public Object invoke(Map arguments) { + validateRequiredParameters(arguments); return ReflectionUtils.invokeMethod(this.method, this.target, resolveArguments(arguments)); } + private void validateRequiredParameters(Map arguments) { + Set missingParameters = Stream.of(this.method.getParameters()) + .filter(p -> isMissing(p, arguments)) + .map(Parameter::getName) + .collect(Collectors.toSet()); + if (!missingParameters.isEmpty()) { + throw new ParametersMissingException(missingParameters); + } + } + + private boolean isMissing(Parameter parameter, Map arguments) { + Object resolved = arguments.get(parameter.getName()); + return (resolved == null && !isExplicitNullable(parameter)); + } + + private boolean isExplicitNullable(Parameter parameter) { + return (parameter.getAnnotationsByType(Nullable.class).length != 0); + } + private Object[] resolveArguments(Map arguments) { return Stream.of(this.method.getParameters()) .map((parameter) -> resolveArgument(parameter, arguments)) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java index 16553be8f85..9119c0795e7 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java @@ -33,6 +33,8 @@ import javax.management.ReflectionException; import reactor.core.publisher.Mono; import org.springframework.boot.actuate.endpoint.EndpointInfo; +import org.springframework.boot.actuate.endpoint.ParameterMappingException; +import org.springframework.boot.actuate.endpoint.ParametersMissingException; import org.springframework.util.ClassUtils; /** @@ -79,11 +81,17 @@ public class EndpointMBean implements DynamicMBean { if (operation != null) { Map arguments = getArguments(params, operation.getParameters()); - Object result = operation.getInvoker().invoke(arguments); - if (REACTOR_PRESENT) { - result = ReactiveHandler.handle(result); + try { + Object result = operation.getInvoker().invoke(arguments); + if (REACTOR_PRESENT) { + result = ReactiveHandler.handle(result); + } + return this.operationResponseConverter.apply(result); } - return this.operationResponseConverter.apply(result); + catch (ParametersMissingException | ParameterMappingException ex) { + throw new IllegalArgumentException(ex.getMessage()); + } + } throw new ReflectionException(new IllegalArgumentException( String.format("Endpoint with id '%s' has no operation named %s", diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/jersey/JerseyEndpointResourceFactory.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/jersey/JerseyEndpointResourceFactory.java index f8775200ec4..e4bd6cb3e5d 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/jersey/JerseyEndpointResourceFactory.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/jersey/JerseyEndpointResourceFactory.java @@ -41,6 +41,7 @@ import reactor.core.publisher.Mono; import org.springframework.boot.actuate.endpoint.EndpointInfo; import org.springframework.boot.actuate.endpoint.OperationInvoker; import org.springframework.boot.actuate.endpoint.ParameterMappingException; +import org.springframework.boot.actuate.endpoint.ParametersMissingException; import org.springframework.boot.actuate.endpoint.web.EndpointLinksResolver; import org.springframework.boot.actuate.endpoint.web.Link; import org.springframework.boot.actuate.endpoint.web.OperationRequestPredicate; @@ -145,7 +146,7 @@ public class JerseyEndpointResourceFactory { Object response = this.operationInvoker.invoke(arguments); return convertToJaxRsResponse(response, data.getRequest().getMethod()); } - catch (ParameterMappingException ex) { + catch (ParametersMissingException | ParameterMappingException ex) { return Response.status(Status.BAD_REQUEST).build(); } } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/reactive/WebFluxEndpointHandlerMapping.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/reactive/WebFluxEndpointHandlerMapping.java index 23783f5abbf..3e616a48388 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/reactive/WebFluxEndpointHandlerMapping.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/reactive/WebFluxEndpointHandlerMapping.java @@ -32,6 +32,7 @@ import org.springframework.boot.actuate.endpoint.EndpointInfo; import org.springframework.boot.actuate.endpoint.OperationInvoker; import org.springframework.boot.actuate.endpoint.OperationType; import org.springframework.boot.actuate.endpoint.ParameterMappingException; +import org.springframework.boot.actuate.endpoint.ParametersMissingException; import org.springframework.boot.actuate.endpoint.web.EndpointLinksResolver; import org.springframework.boot.actuate.endpoint.web.Link; import org.springframework.boot.actuate.endpoint.web.OperationRequestPredicate; @@ -219,6 +220,8 @@ public class WebFluxEndpointHandlerMapping extends RequestMappingInfoHandlerMapp private Publisher> handleResult(Publisher result, HttpMethod httpMethod) { return Mono.from(result).map(this::toResponseEntity) + .onErrorReturn(ParametersMissingException.class, + new ResponseEntity<>(HttpStatus.BAD_REQUEST)) .onErrorReturn(ParameterMappingException.class, new ResponseEntity<>(HttpStatus.BAD_REQUEST)) .defaultIfEmpty(new ResponseEntity<>(httpMethod == HttpMethod.GET diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java index 78860b8563f..0de4648a7de 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/servlet/WebMvcEndpointHandlerMapping.java @@ -28,6 +28,7 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.boot.actuate.endpoint.EndpointInfo; import org.springframework.boot.actuate.endpoint.OperationInvoker; import org.springframework.boot.actuate.endpoint.ParameterMappingException; +import org.springframework.boot.actuate.endpoint.ParametersMissingException; import org.springframework.boot.actuate.endpoint.web.EndpointLinksResolver; import org.springframework.boot.actuate.endpoint.web.Link; import org.springframework.boot.actuate.endpoint.web.WebEndpointOperation; @@ -127,7 +128,7 @@ public class WebMvcEndpointHandlerMapping extends AbstractWebMvcEndpointHandlerM try { return handleResult(this.operationInvoker.invoke(arguments), httpMethod); } - catch (ParameterMappingException ex) { + catch (ParametersMissingException | ParameterMappingException ex) { return new ResponseEntity(HttpStatus.BAD_REQUEST); } } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/env/EnvironmentEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/env/EnvironmentEndpoint.java index 68b995f91c5..906d73bb378 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/env/EnvironmentEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/env/EnvironmentEndpoint.java @@ -42,6 +42,7 @@ import org.springframework.core.env.Environment; import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; import org.springframework.core.env.StandardEnvironment; +import org.springframework.lang.Nullable; import org.springframework.util.PropertyPlaceholderHelper; import org.springframework.util.StringUtils; import org.springframework.util.SystemPropertyUtils; @@ -72,7 +73,7 @@ public class EnvironmentEndpoint { } @ReadOperation - public EnvironmentDescriptor environment(String pattern) { + public EnvironmentDescriptor environment(@Nullable String pattern) { if (StringUtils.hasText(pattern)) { return getEnvironmentDescriptor(Pattern.compile(pattern).asPredicate()); } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/management/HeapDumpWebEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/management/HeapDumpWebEndpoint.java index 0991472745e..dfd0c69ecbd 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/management/HeapDumpWebEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/management/HeapDumpWebEndpoint.java @@ -42,6 +42,7 @@ import org.springframework.boot.actuate.endpoint.annotation.ReadOperation; import org.springframework.boot.actuate.endpoint.web.WebEndpointResponse; import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.Resource; +import org.springframework.lang.Nullable; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; @@ -72,7 +73,7 @@ public class HeapDumpWebEndpoint { } @ReadOperation - public WebEndpointResponse heapDump(Boolean live) { + public WebEndpointResponse heapDump(@Nullable Boolean live) { try { if (this.lock.tryLock(this.timeout, TimeUnit.MILLISECONDS)) { try { diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/session/SessionsWebEndpointExtension.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/session/SessionsWebEndpointExtension.java index fbed5298512..bb42cd20330 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/session/SessionsWebEndpointExtension.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/session/SessionsWebEndpointExtension.java @@ -38,9 +38,6 @@ public class SessionsWebEndpointExtension { @ReadOperation public WebEndpointResponse sessionsForUsername(String username) { - if (username == null) { - return new WebEndpointResponse<>(WebEndpointResponse.STATUS_BAD_REQUEST); - } SessionsReport sessions = this.delegate.sessionsForUsername(username); return new WebEndpointResponse<>(sessions); } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java index bde5a78f13d..7a5ff317d4f 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java @@ -50,6 +50,7 @@ import org.springframework.boot.actuate.endpoint.convert.ConversionServiceOperat import org.springframework.boot.actuate.endpoint.jmx.annotation.JmxAnnotationEndpointDiscoverer; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.lang.Nullable; import static org.assertj.core.api.Assertions.assertThat; @@ -265,6 +266,52 @@ public class EndpointMBeanTests { }); } + @Test + public void invokeWithParameterMappingExceptionMapsToIllegalArgumentException() throws Exception { + load(FooEndpoint.class, (discoverer) -> { + ObjectName objectName = registerEndpoint(discoverer, "foo"); + try { + this.server.invoke(objectName, "getOne", + new Object[] { "wrong" }, new String[] { String.class.getName() }); + } + catch (Exception ex) { + assertThat(ex.getCause()).isExactlyInstanceOf(IllegalArgumentException.class); + assertThat(ex.getCause().getMessage()).isEqualTo(String.format("Failed to map wrong of type " + + "%s to type %s", String.class, FooName.class)); + } + }); + } + + @Test + public void invokeWithMissingRequiredParameterExceptionMapsToIllegalArgumentException() throws Exception { + load(RequiredParametersEndpoint.class, (discoverer) -> { + ObjectName objectName = registerEndpoint(discoverer, "requiredparameters"); + try { + this.server.invoke(objectName, "read", + new Object[] {}, new String[] { String.class.getName() }); + } + catch (Exception ex) { + assertThat(ex.getCause()).isExactlyInstanceOf(IllegalArgumentException.class); + assertThat(ex.getCause().getMessage()).isEqualTo("Failed to invoke operation because the following " + + "required parameters were missing: foo,baz"); + } + }); + } + + @Test + public void invokeWithMissingNullableParameter() throws Exception { + load(RequiredParametersEndpoint.class, (discoverer) -> { + ObjectName objectName = registerEndpoint(discoverer, "requiredparameters"); + try { + this.server.invoke(objectName, "read", + new Object[] {null, "hello", "world"}, new String[] { String.class.getName() }); + } + catch (Exception ex) { + throw new AssertionError("Nullable parameter should not be required."); + } + }); + } + private ObjectName registerEndpoint(JmxAnnotationEndpointDiscoverer discoverer, String endpointId) { Collection mBeans = this.jmxEndpointMBeanFactory @@ -326,6 +373,16 @@ public class EndpointMBeanTests { } + @Endpoint(id = "requiredparameters") + static class RequiredParametersEndpoint { + + @ReadOperation + public String read(@Nullable String bar, String foo, String baz) { + return foo; + } + + } + @Endpoint(id = "reactive") static class ReactiveEndpoint { diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/AbstractWebEndpointIntegrationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/AbstractWebEndpointIntegrationTests.java index bc522ae8132..8a177915186 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/AbstractWebEndpointIntegrationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/AbstractWebEndpointIntegrationTests.java @@ -47,6 +47,7 @@ import org.springframework.core.env.MapPropertySource; import org.springframework.core.io.ByteArrayResource; import org.springframework.core.io.Resource; import org.springframework.http.MediaType; +import org.springframework.lang.Nullable; import org.springframework.test.web.reactive.server.WebTestClient; import static org.assertj.core.api.Assertions.assertThat; @@ -281,6 +282,20 @@ public abstract class AbstractWebEndpointIntegrationTests client.get().uri("/requiredparameters").exchange() + .expectStatus().isBadRequest()); + } + + @Test + public void readOperationWithMissingNullableParametersIsOk() throws Exception { + load(RequiredParameterEndpointConfiguration.class, + (client) -> client.get().uri("/requiredparameters?foo=hello").exchange() + .expectStatus().isOk()); + } + protected abstract T createApplicationContext(Class... config); protected abstract int getPort(T context); @@ -489,6 +504,17 @@ public abstract class AbstractWebEndpointIntegrationTests