GH-3283: HTTP Inbound handle SpEL errors (#3289)

* GH-3283: HTTP Inbound handle SpEL errors

Fixes #3283

* Process all the request message preparation exceptions
in the provided error channel to let target application
to make a decision about an appropriate HTTP status instead of
default 500 Server Error

* * Rephrase `ResponseStatusException` doc in the http.adoc

Co-authored-by: Gary Russell <grussell@vmware.com>

Co-authored-by: Gary Russell <grussell@vmware.com>

spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingEndpointSupport.java

@@ -59,8 +59,10 @@
 import org.springframework.integration.support.AbstractIntegrationMessageBuilder;
 import org.springframework.integration.support.json.JacksonPresent;
 import org.springframework.messaging.Message;
+import org.springframework.messaging.MessageChannel;
 import org.springframework.messaging.MessageHeaders;
 import org.springframework.messaging.MessagingException;
+import org.springframework.messaging.converter.MessageConversionException;
 import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.LinkedMultiValueMap;
@@ -267,30 +269,46 @@ private Message<?> actualDoHandleRequest(HttpServletRequest servletRequest, Requ
 
 			Map<String, Object> headers = getHeaderMapper().toHeaders(httpEntity.getHeaders());
 			Object payload = null;
-			if (getPayloadExpression() != null) {
-				// create payload based on SpEL
-				payload = getPayloadExpression().getValue(evaluationContext);
-			}
+			Message<?> message = null;
+			try {
+				if (getPayloadExpression() != null) {
+					// create payload based on SpEL
+					payload = getPayloadExpression().getValue(evaluationContext);
+				}
 
-			if (!CollectionUtils.isEmpty(getHeaderExpressions())) {
-				headers.putAll(
-						ExpressionEvalMap.from(getHeaderExpressions())
-								.usingEvaluationContext(evaluationContext)
-								.withRoot(httpEntity)
-								.build());
-			}
+				if (!CollectionUtils.isEmpty(getHeaderExpressions())) {
+					headers.putAll(
+							ExpressionEvalMap.from(getHeaderExpressions())
+									.usingEvaluationContext(evaluationContext)
+									.withRoot(httpEntity)
+									.build());
+				}
 
-			if (payload == null) {
-				if (httpEntity.getBody() != null) {
-					payload = httpEntity.getBody();
+				if (payload == null) {
+					if (httpEntity.getBody() != null) {
+						payload = httpEntity.getBody();
+					}
+					else {
+						payload = requestParams;
+					}
+				}
+
+				message = prepareRequestMessage(servletRequest, httpEntity, headers, payload);
+			}
+			catch (Exception ex) {
+				MessageConversionException conversionException =
+						new MessageConversionException("Cannot create request message", ex);
+				MessageChannel errorChannel = getErrorChannel();
+				if (errorChannel != null) {
+					this.messagingTemplate.send(errorChannel,
+							buildErrorMessage(null,
+									conversionException));
 				}
 				else {
-					payload = requestParams;
+					throw conversionException;
 				}
 			}
 
-			Message<?> message = prepareRequestMessage(servletRequest, httpEntity, headers, payload);
-
 			Message<?> reply = null;
 			if (isExpectReply()) {
 				try {

spring-integration-http/src/test/java/org/springframework/integration/http/dsl/HttpDslTests.java

@@ -40,6 +40,7 @@
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.http.HttpMethod;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
 import org.springframework.http.client.ClientHttpRequestFactory;
@@ -56,6 +57,7 @@
 import org.springframework.messaging.Message;
 import org.springframework.messaging.MessageChannel;
 import org.springframework.messaging.PollableChannel;
+import org.springframework.messaging.support.ErrorMessage;
 import org.springframework.mock.web.MockPart;
 import org.springframework.security.access.AccessDecisionManager;
 import org.springframework.security.access.vote.AffirmativeBased;
@@ -81,6 +83,7 @@
 import org.springframework.web.context.WebApplicationContext;
 import org.springframework.web.multipart.MultipartResolver;
 import org.springframework.web.multipart.support.StandardServletMultipartResolver;
+import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.servlet.DispatcherServlet;
 
 /**
@@ -235,6 +238,30 @@ public void testValidation() throws Exception {
 		flowRegistration.destroy();
 	}
 
+	@Test
+	public void testBadRequest() throws Exception {
+		IntegrationFlow flow =
+				IntegrationFlows.from(
+						Http.inboundGateway("/badRequest")
+								.errorChannel((message, timeout) -> {
+									throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
+											"Not valid request param", ((ErrorMessage) message).getPayload());
+								})
+								.payloadExpression("#requestParams.p1"))
+						.get();
+
+		IntegrationFlowContext.IntegrationFlowRegistration flowRegistration =
+				this.integrationFlowContext.registration(flow).register();
+
+		this.mockMvc.perform(
+				get("/badRequest")
+						.with(httpBasic("user", "user"))
+						.param("p2", "P2"))
+				.andExpect(status().isBadRequest())
+				.andExpect(status().reason("Not valid request param"));
+
+		flowRegistration.destroy();
+	}
 
 	@Configuration
 	@EnableWebSecurity

src/reference/asciidoc/http.adoc

@@ -431,6 +431,11 @@ If the error flow times out after a main flow timeout, `500 Internal Server Erro
 NOTE: Previously, the default status code for a timeout was `200 OK`.
 To restore that behavior, set `reply-timeout-status-code-expression="200"`.
 
+Also starting with version 5.4, an error that is encountered while preparing the request message is sent to the error channel (if provided).
+A decision about throwing an appropriate exception should be done in the error flow by examining the exception.
+Previously, any exceptions were simply thrown, causing an HTTP 500 server error response status, but in some cases the problem can be caused by incorrect request params, so a `ResponseStatusException` with a 4xx client error status should be thrown instead.
+See `ResponseStatusException` for more information.
+The `ErrorMessage` sent to this error channel contains the original exception as the payload for analysis.
 ==== URI Template Variables and Expressions
 
 By using the `path` attribute in conjunction with the `payload-expression` attribute and the `header` element, you have a high degree of flexibility for mapping inbound request data.