From ef939a09a4ef91e2d876dcf099c9882b617d330d Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Thu, 28 May 2020 11:47:15 -0400 Subject: [PATCH] GH-3283: HTTP Inbound handle SpEL errors (#3289) * GH-3283: HTTP Inbound handle SpEL errors Fixes https://github.com/spring-projects/spring-integration/issues/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 Co-authored-by: Gary Russell --- .../HttpRequestHandlingEndpointSupport.java | 52 +++++++++++++------ .../integration/http/dsl/HttpDslTests.java | 27 ++++++++++ src/reference/asciidoc/http.adoc | 5 ++ 3 files changed, 67 insertions(+), 17 deletions(-) diff --git a/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingEndpointSupport.java b/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingEndpointSupport.java index 1755278a87..f804d02c50 100644 --- a/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingEndpointSupport.java +++ b/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingEndpointSupport.java @@ -59,8 +59,10 @@ import org.springframework.integration.http.multipart.MultipartHttpInputMessage; 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 @@ public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInbound Map 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 { diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/dsl/HttpDslTests.java b/spring-integration-http/src/test/java/org/springframework/integration/http/dsl/HttpDslTests.java index 545b70fa83..10e907ab3b 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/dsl/HttpDslTests.java +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/dsl/HttpDslTests.java @@ -40,6 +40,7 @@ import org.springframework.beans.factory.annotation.Qualifier; 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.integration.security.channel.SecuredChannel; 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.client.HttpClientErrorException; 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 class HttpDslTests { 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 diff --git a/src/reference/asciidoc/http.adoc b/src/reference/asciidoc/http.adoc index 3c233588f5..c1cba21f1f 100644 --- a/src/reference/asciidoc/http.adoc +++ b/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.