GH-3641: Handle duplicate cookies properly

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

Author: Anthonyas007

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

@@ -19,11 +19,8 @@
 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;
@@ -384,10 +381,13 @@ private StandardEvaluationContext prepareEvaluationContext(HttpServletRequest se
 
 		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 =
@@ -500,7 +500,7 @@ protected RequestEntity<Object> prepareRequestEntity(ServletServerHttpRequest re
 		return new RequestEntity<>(requestBody, request.getHeaders(), request.getMethod(), request.getURI());
 	}
 
-	@SuppressWarnings({"unchecked", "rawtypes"})
+	@SuppressWarnings({ "unchecked", "rawtypes" })
 	protected Object extractRequestBody(ServletServerHttpRequest request) throws IOException {
 		MediaType contentType = request.getHeaders().getContentType();
 		if (contentType == null) {

Diff for: 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,25 +16,26 @@
 
 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;
 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 for: 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 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.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 @@
  * @author Artem Bilan
  * @author Biju Kunjummen
  */
-@RunWith(SpringRunner.class)
+@SpringJUnitConfig
 @DirtiesContext
 public class HttpInboundChannelAdapterParserTests extends AbstractHttpInboundTests {
 

Diff for: 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.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 @@
  * @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 void sendOnly() throws Exception {
 	}
 
 	@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 void sendOnlyViewExpression() throws Exception {
 	}
 
 	@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 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
 	}
 
 	@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 @@ protected Message<String> handleRequestMessage(Message<?> requestMessage) {
 	}
 
 	@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 @@ protected Message<String> handleRequestMessage(Message<?> requestMessage) {
 	}
 
 	@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 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
 	}
 
 	@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 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
 	}
 
 	@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 @@ protected boolean doSend(Message<?> message, long timeout) {
 	}
 
 	@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 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
 		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 for: 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 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 for: 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 @@
 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 void withoutExpression() throws IOException {
 
 		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 void withoutExpression() throws IOException {
 	}
 
 	@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 void withPayloadExpressionPointingToPathVariable() throws Exception {
 
 	@SuppressWarnings("unchecked")
 	@Test
-	public void withoutPayloadExpressionPointingToUriVariables() throws Exception {
+	public void withoutPayloadExpressionPointingToUriVariables() {
 
 		DirectChannel echoChannel = new DirectChannel();
 		echoChannel.subscribe(message -> {