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`**

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<String, Cookie> cookies =
-					Arrays.stream(requestCookies)
-							.collect(Collectors.toMap(Cookie::getName, Function.identity()));
-			evaluationContext.setVariable("cookies", cookies);
+
+			MultiValueMap<String, Cookie> 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 =

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();
 	}

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 {
 

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<String> 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<String> 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");
+	}
+
 
 }

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;

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<Object> 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 -> {

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<String, String> params = new HashMap<String, String>();
+		final Map<String, String> params = new HashMap<>();
 		params.put("foo", "bar");
 		request.setParameters(params);
 		request.setContent("hello".getBytes());