Reactor Netty can only get the first cookie value when multiple cookies with the same name exist

[jsonwan]

Affects: main

The way to confirm:

Modify two lines in CookieIntegrationTests and the test will fail.

	@ParameterizedHttpServerTest
	public void basicTest(HttpServer httpServer) throws Exception {
		startServer(httpServer);

		URI url = new URI("http://localhost:" + port);
		// Modified line 1:add lang=zh-CN
		String header = "SID=31d4d96e407aad42; lang=en-US; lang=zh-CN";
		ResponseEntity<Void> response = new RestTemplate().exchange(
				RequestEntity.get(url).header("Cookie", header).build(), Void.class);

		Map<String, List<HttpCookie>> requestCookies = this.cookieHandler.requestCookies;
		assertThat(requestCookies.size()).isEqualTo(2);

		List<HttpCookie> list = requestCookies.get("SID");
		assertThat(list.size()).isEqualTo(1);
		assertThat(list.iterator().next().getValue()).isEqualTo("31d4d96e407aad42");

		list = requestCookies.get("lang");
		// Modified line 2:add this line
		assertThat(list.size()).isEqualTo(2);
		assertThat(list.iterator().next().getValue()).isEqualTo("en-US");

		List<String> headerValues = response.getHeaders().get("Set-Cookie");
		assertThat(headerValues.size()).isEqualTo(2);

		List<String> cookie0 = splitCookie(headerValues.get(0));
		assertThat(cookie0.remove("SID=31d4d96e407aad42")).as("SID").isTrue();
		assertThat(cookie0.stream().map(String::toLowerCase))
				.containsExactlyInAnyOrder("path=/", "secure", "httponly");
		List<String> cookie1 = splitCookie(headerValues.get(1));
		assertThat(cookie1.remove("lang=en-US")).as("lang").isTrue();
		assertThat(cookie1.stream().map(String::toLowerCase))
				.containsExactlyInAnyOrder("path=/", "domain=example.com");
	}

According to rfc 6265，server should be able to handle cookies with the same name instead of parsing the first one and ignoring the others. I think it is more reasonable for spring-web to give all values to application and let application decide how to handle multiple values.

By the way, the bottom frameworks Reactor-netty, Jetty, and Tomcat can parse multiple cookie values with the same name in their newer versions, wish spring can support soon.

Related Issues

• Cookie decoding for multiple cookies with same name has issues reactor/reactor-netty#1641

[aooohan]

I don't think this is an issue with the spring framework.You can debug it at org.springframework.http.server.reactive.AbstractServerHttpRequest#initCookies

[jsonwan]

I don't think this is an issue with the spring framework.You can debug it at org.springframework.http.server.reactive.AbstractServerHttpRequest#initCookies

This method only get the first cookie value when multi cookies with the same name exist, because the implementation class ReactorServerHttpRequest.initCookies() use HttpServerRequest.cookies() to get cookies, which can be replaced by HttpServerRequest.allCookies() to get all the values.
Besides, the return type of this method is MultiValueMap, which can be used to save cookies with same name. I guess may be designed so.

[aooohan]

I don't think this is an issue with the spring framework.You can debug it at org.springframework.http.server.reactive.AbstractServerHttpRequest#initCookies

This method only get the first cookie value when multi cookies with the same name exist, because the implementation class ReactorServerHttpRequest.initCookies() use HttpServerRequest.cookies() to get cookies, which can be replaced by HttpServerRequest.allCookies() to get all the values. Besides, the return type of this method is MultiValueMap, which can be used to save cookies with same name. I guess may be designed so.

Yes, you're right, I just took a closer look at the ReactorServerHttpRequest implementation and reactor-netty's HttpServerRequest does provide a way to get all cookies, HttpServerRequest#allCookies().

[aooohan]

@jsonwan Can I propose a pr to solve this issue?

[jsonwan]

@jsonwan Can I propose a pr to solve this issue?

Yes, I tried to solve this problem by using HttpServerRequest.allCookies() instead of cookies() and completing the test cases, however, Jetty, Reactor-netty and Tomcat test cases passed but the undertow test cases failed because undertow does not provide a method to get all cookies. Wish for your help.
Related undertow issue: https://issues.redhat.com/browse/UNDERTOW-1676#section-4.2.2

[aooohan]

@jsonwan Can I propose a pr to solve this issue?

Yes, I tried to solve this problem by using HttpServerRequest.allCookies() instead of cookies() and completing the test cases, however, Jetty, Reactor-netty and Tomcat test cases passed but the undertow test cases failed because undertow does not provide a method to get all cookies. Wish for your help. Related undertow issue: https://issues.redhat.com/browse/UNDERTOW-1676#section-4.2.2

Undertow does not offer the ability to parse multiple cookies, there is nothing we can do about it. I think we just need to solve the ReactorServerHttpRequest problem, but of course, I think it's a problem, and it's up to the maintainers of the spring framework to decide if it ends up being what we think it is

[akvone]

We also stumbled upon this. Currently we fixed that by using nativeRequest:

HttpServerRequest nativeRequest = ((AbstractServerHttpRequest) serverWebExchange.getRequest()).getNativeRequest();
Map<CharSequence, List<Cookie>> allCookies = nativeRequest.allCookies();

By the way, it seems that default implementation of HttpServletRequest#getCookies returns all cookies.

[jhoeller]

Note that we are upgrading to the Servlet 6.0 based Undertow 2.3 for Spring Framework 6.0 RC4 (#29435), aiming to retain runtime compatibility with Undertow 2.2. This allows us to rely on the new Undertow 2.2 cookies API, hopefully helping here.

[sdeleuze]

I confirm @aooohan analysis, Undertow handling of multiple cookies with the same name looks incorrect. They may have mistakenly interpreted "e.g., that were set with different Path or Domain attributes" as "i.e. , that were set with different Path or Domain attributes" ("for example" in the spec versus "that is" in their implementation description).

As a consequence, I am going to fix this bug with Reactor Netty and ignore the related test with Undertow (until they fix this bug).