From 4e6ce367d032cd3e3e36dc75f70684a2262a9512 Mon Sep 17 00:00:00 2001 From: Anthony Schweigard Date: Fri, 8 Oct 2021 22:35:46 -0500 Subject: [PATCH] GH-3641: Handle duplicate cookies properly Fixes https://github.com/spring-projects/spring-integration/issues/3641 When a duplicate cookie name appears in a request, an `IllegalStateException` is thrown. The default `Collectors.toMap()` does not allow a duplicated keys. * Handle `servletRequest.getCookies()` as a `MultiValueMap` * Call `toSingleValueMap()` for the evaluation context variable to restore previous behavior. The next major version must expose the `MultiValueMap` as is to give access to all cookies from end-user expressions * Rework some HTTP tests to JUnit 5 **Cherry-pick to `5.4.x` & `5.3.x`** --- .../HttpRequestHandlingEndpointSupport.java | 14 ++-- .../http/AbstractHttpInboundTests.java | 11 +-- .../HttpInboundChannelAdapterParserTests.java | 9 ++- .../HttpRequestHandlingControllerTests.java | 69 ++++++++++++++++--- ...pRequestHandlingMessagingGatewayTests.java | 4 +- ...gMessagingGatewayWithPathMappingTests.java | 13 ++-- ...Int2312RequestMappingIntegrationTests.java | 13 ++-- 7 files changed, 87 insertions(+), 46 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..baaf94ffaa 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 @@ -19,11 +19,8 @@ package org.springframework.integration.http.inbound; import java.io.IOException; import java.lang.reflect.Type; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; @@ -361,10 +358,13 @@ public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInbound Cookie[] requestCookies = servletRequest.getCookies(); if (!ObjectUtils.isEmpty(requestCookies)) { - Map cookies = - Arrays.stream(requestCookies) - .collect(Collectors.toMap(Cookie::getName, Function.identity())); - evaluationContext.setVariable("cookies", cookies); + + MultiValueMap cookies = new LinkedMultiValueMap<>(requestCookies.length); + for (Cookie requestCookie : requestCookies) { + cookies.add(requestCookie.getName(), requestCookie); + } + // TODO no toSingleValueMap() in the next major version + evaluationContext.setVariable("cookies", cookies.toSingleValueMap()); } Map pathVariables = diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/AbstractHttpInboundTests.java b/spring-integration-http/src/test/java/org/springframework/integration/http/AbstractHttpInboundTests.java index 14138886ee..dffae3ea3c 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/AbstractHttpInboundTests.java +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/AbstractHttpInboundTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2021 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. @@ -16,8 +16,8 @@ package org.springframework.integration.http; -import org.junit.After; -import org.junit.Before; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.web.context.request.RequestContextHolder; @@ -25,16 +25,17 @@ import org.springframework.web.context.request.ServletRequestAttributes; /** * @author Artem Bilan + * * @since 3.0 */ public abstract class AbstractHttpInboundTests { - @Before + @BeforeEach public void setupHttpInbound() { RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(new MockHttpServletRequest())); } - @After + @AfterEach public void tearDownHttpInbound() { RequestContextHolder.resetRequestAttributes(); } diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/config/HttpInboundChannelAdapterParserTests.java b/spring-integration-http/src/test/java/org/springframework/integration/http/config/HttpInboundChannelAdapterParserTests.java index dc5e9a85ca..41c5c62ba6 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/config/HttpInboundChannelAdapterParserTests.java +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/config/HttpInboundChannelAdapterParserTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. @@ -30,8 +30,7 @@ import java.util.Properties; import javax.servlet.http.HttpServletResponse; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; @@ -52,7 +51,7 @@ import org.springframework.messaging.PollableChannel; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.test.annotation.DirtiesContext; -import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; import org.springframework.util.AntPathMatcher; import org.springframework.util.MultiValueMap; import org.springframework.validation.Validator; @@ -68,7 +67,7 @@ import org.springframework.web.servlet.HandlerMapping; * @author Artem Bilan * @author Biju Kunjummen */ -@RunWith(SpringRunner.class) +@SpringJUnitConfig @DirtiesContext public class HttpInboundChannelAdapterParserTests extends AbstractHttpInboundTests { diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingControllerTests.java b/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingControllerTests.java index 46af895425..66e4585263 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingControllerTests.java +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingControllerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. @@ -25,8 +25,10 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import javax.servlet.http.Cookie; + import org.apache.commons.logging.LogFactory; -import org.junit.Test; +import org.junit.jupiter.api.Test; import org.springframework.beans.factory.BeanFactory; import org.springframework.expression.Expression; @@ -51,12 +53,14 @@ import org.springframework.web.servlet.View; * @author Gunnar Hillert * @author Biju Kunjummen * @author Artem Bilan + * @author Anthony Schweigard + * * @since 2.0 */ public class HttpRequestHandlingControllerTests extends AbstractHttpInboundTests { @Test - public void sendOnly() throws Exception { + public void sendOnly() { QueueChannel requestChannel = new QueueChannel(); HttpRequestHandlingController controller = new HttpRequestHandlingController(false); controller.setBeanFactory(mock(BeanFactory.class)); @@ -83,7 +87,7 @@ public class HttpRequestHandlingControllerTests extends AbstractHttpInboundTests } @Test - public void sendOnlyViewExpression() throws Exception { + public void sendOnlyViewExpression() { QueueChannel requestChannel = new QueueChannel(); HttpRequestHandlingController controller = new HttpRequestHandlingController(false); controller.setBeanFactory(mock(BeanFactory.class)); @@ -111,9 +115,10 @@ public class HttpRequestHandlingControllerTests extends AbstractHttpInboundTests } @Test - public void requestReply() throws Exception { + public void requestReply() { DirectChannel requestChannel = new DirectChannel(); AbstractReplyProducingMessageHandler handler = new AbstractReplyProducingMessageHandler() { + @Override protected Object handleRequestMessage(Message requestMessage) { return requestMessage.getPayload().toString().toUpperCase(); @@ -145,9 +150,10 @@ public class HttpRequestHandlingControllerTests extends AbstractHttpInboundTests } @Test - public void requestReplyViewExpressionString() throws Exception { + public void requestReplyViewExpressionString() { DirectChannel requestChannel = new DirectChannel(); AbstractReplyProducingMessageHandler handler = new AbstractReplyProducingMessageHandler() { + @Override protected Message handleRequestMessage(Message requestMessage) { return MessageBuilder.withPayload("foo") @@ -177,10 +183,11 @@ public class HttpRequestHandlingControllerTests extends AbstractHttpInboundTests } @Test - public void requestReplyViewExpressionView() throws Exception { + public void requestReplyViewExpressionView() { final View view = mock(View.class); DirectChannel requestChannel = new DirectChannel(); AbstractReplyProducingMessageHandler handler = new AbstractReplyProducingMessageHandler() { + @Override protected Message handleRequestMessage(Message requestMessage) { return MessageBuilder.withPayload("foo") @@ -210,9 +217,10 @@ public class HttpRequestHandlingControllerTests extends AbstractHttpInboundTests } @Test - public void requestReplyWithCustomReplyKey() throws Exception { + public void requestReplyWithCustomReplyKey() { DirectChannel requestChannel = new DirectChannel(); AbstractReplyProducingMessageHandler handler = new AbstractReplyProducingMessageHandler() { + @Override protected Object handleRequestMessage(Message requestMessage) { return requestMessage.getPayload().toString().toUpperCase(); @@ -245,9 +253,10 @@ public class HttpRequestHandlingControllerTests extends AbstractHttpInboundTests } @Test - public void requestReplyWithFullMessageInModel() throws Exception { + public void requestReplyWithFullMessageInModel() { DirectChannel requestChannel = new DirectChannel(); AbstractReplyProducingMessageHandler handler = new AbstractReplyProducingMessageHandler() { + @Override protected Object handleRequestMessage(Message requestMessage) { return requestMessage.getPayload().toString().toUpperCase(); @@ -281,8 +290,9 @@ public class HttpRequestHandlingControllerTests extends AbstractHttpInboundTests } @Test - public void testSendWithError() throws Exception { + public void testSendWithError() { QueueChannel requestChannel = new QueueChannel() { + @Override protected boolean doSend(Message message, long timeout) { throw new RuntimeException("Planned"); @@ -310,11 +320,12 @@ public class HttpRequestHandlingControllerTests extends AbstractHttpInboundTests } @Test - public void shutDown() throws Exception { + public void shutDown() { DirectChannel requestChannel = new DirectChannel(); final CountDownLatch latch1 = new CountDownLatch(1); final CountDownLatch latch2 = new CountDownLatch(1); AbstractReplyProducingMessageHandler handler = new AbstractReplyProducingMessageHandler() { + @Override protected Object handleRequestMessage(Message requestMessage) { try { @@ -379,5 +390,41 @@ public class HttpRequestHandlingControllerTests extends AbstractHttpInboundTests assertThat(reply).isEqualTo("HELLO"); } + @Test + public void handleRequestDuplicateCookies() { + DirectChannel requestChannel = new DirectChannel(); + requestChannel.subscribe(new AbstractReplyProducingMessageHandler() { + + @Override + protected Object handleRequestMessage(Message requestMessage) { + return requestMessage.getPayload().toString(); + } + }); + + HttpRequestHandlingController controller = new HttpRequestHandlingController(true); + controller.setErrorsKey("errors"); + controller.setRequestChannel(requestChannel); + controller.setViewName("foo"); + controller.setReplyKey("cookiesReply"); + controller.setExtractReplyPayload(true); + controller.setPayloadExpression(new SpelExpressionParser().parseExpression("#cookies['c1']?.value")); + controller.setBeanFactory(mock(BeanFactory.class)); + controller.afterPropertiesSet(); + controller.start(); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("POST"); + request.setContent("hello".getBytes()); + request.addHeader("Content-Type", "text/plain"); + request.setCookies(new Cookie("c1", "first"), new Cookie("c1", "last")); + + MockHttpServletResponse response = new MockHttpServletResponse(); + + ModelAndView modelAndView = controller.handleRequest(request, response); + assertThat(modelAndView.getModelMap()).doesNotContainKey("errors"); + assertThat(modelAndView.getModelMap()).containsKey("cookiesReply"); + assertThat(modelAndView.getModelMap()).containsEntry("cookiesReply", "first"); + } + } diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayTests.java b/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayTests.java index 9b62254274..7951d65593 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayTests.java +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. @@ -31,7 +31,7 @@ import java.util.List; import javax.servlet.http.HttpServletRequest; -import org.junit.Test; +import org.junit.jupiter.api.Test; import org.springframework.beans.factory.BeanFactory; import org.springframework.core.ParameterizedTypeReference; diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayWithPathMappingTests.java b/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayWithPathMappingTests.java index 4ac5e396cb..5efabd1f20 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayWithPathMappingTests.java +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayWithPathMappingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. @@ -19,10 +19,9 @@ package org.springframework.integration.http.inbound; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; -import java.io.IOException; import java.util.Map; -import org.junit.Test; +import org.junit.jupiter.api.Test; import org.springframework.beans.factory.BeanFactory; import org.springframework.expression.ExpressionParser; @@ -52,7 +51,7 @@ public class HttpRequestHandlingMessagingGatewayWithPathMappingTests extends Abs @Test - public void withoutExpression() throws IOException { + public void withoutExpression() { DirectChannel echoChannel = new DirectChannel(); echoChannel.subscribe(message -> { MessageChannel replyChannel = (MessageChannel) message.getHeaders().getReplyChannel(); @@ -78,8 +77,6 @@ public class HttpRequestHandlingMessagingGatewayWithPathMappingTests extends Abs gateway.setRequestChannel(echoChannel); - MockHttpServletResponse response = new MockHttpServletResponse(); - RequestEntity httpEntity = prepareRequestEntity(body, new ServletServerHttpRequest(request)); Object result = gateway.doHandleRequest(request, httpEntity); @@ -89,7 +86,7 @@ public class HttpRequestHandlingMessagingGatewayWithPathMappingTests extends Abs } @Test - public void withPayloadExpressionPointingToPathVariable() throws Exception { + public void withPayloadExpressionPointingToPathVariable() { DirectChannel echoChannel = new DirectChannel(); echoChannel.subscribe(message -> { MessageChannel replyChannel = (MessageChannel) message.getHeaders().getReplyChannel(); @@ -132,7 +129,7 @@ public class HttpRequestHandlingMessagingGatewayWithPathMappingTests extends Abs @SuppressWarnings("unchecked") @Test - public void withoutPayloadExpressionPointingToUriVariables() throws Exception { + public void withoutPayloadExpressionPointingToUriVariables() { DirectChannel echoChannel = new DirectChannel(); echoChannel.subscribe(message -> { diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/Int2312RequestMappingIntegrationTests.java b/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/Int2312RequestMappingIntegrationTests.java index ebf09f4c2b..b4a8edeb45 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/Int2312RequestMappingIntegrationTests.java +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/Int2312RequestMappingIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2021 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. @@ -24,8 +24,7 @@ import java.util.Map; import javax.servlet.http.Cookie; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpHeaders; @@ -36,8 +35,7 @@ import org.springframework.messaging.SubscribableChannel; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.test.annotation.DirtiesContext; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; import org.springframework.util.MultiValueMap; import org.springframework.web.bind.UnsatisfiedServletRequestParameterException; import org.springframework.web.context.request.RequestAttributes; @@ -53,8 +51,7 @@ import org.springframework.web.servlet.mvc.HttpRequestHandlerAdapter; * @since 3.0 */ //INT-2312 -@RunWith(SpringJUnit4ClassRunner.class) -@ContextConfiguration +@SpringJUnitConfig @DirtiesContext public class Int2312RequestMappingIntegrationTests extends AbstractHttpInboundTests { @@ -100,7 +97,7 @@ public class Int2312RequestMappingIntegrationTests extends AbstractHttpInboundTe String requestURI = "/test/" + testRequest; request.setRequestURI(requestURI); request.setContentType("text/plain"); - final Map params = new HashMap(); + final Map params = new HashMap<>(); params.put("foo", "bar"); request.setParameters(params); request.setContent("hello".getBytes());