micronaut-core icon indicating copy to clipboard operation
micronaut-core copied to clipboard

Cookie Parsing Failure

Open diamondq opened this issue 1 year ago • 3 comments

Expected Behavior

Parsing multiple cookies on a single Cookie HTTP Header should see all the cookies.

Actual Behaviour

Cookies are not parsed correctly due to two bugs.

  1. Even when the NettyLaxServerCookieDecoder is present, it's not used because of the presence of the META-INF/services/io.micronaut.http.cookie.ServerCookieDecoder within the micronaut-http library that points to io.micronaut.http.cookie.DefaultServerCookieDecoder. This service files should not be present, since all the loading code already has this class as a fallback, so having them present can cause problems since the loading order is indeterminate.

  2. While the NettyLaxServerCookieDecoder does decode cookies properly, the fallback, DefaultServerCookieDecoder does not. It relies on the java.net.HttpCookie.parse methods to perform parsing. However, these methods are based on parsing the Set-Cookie header, not the Cookie header. The format of these headers are quite different. Set-Cookie is Set-Cookie: <cookie-name>=<cookie-value>; Domain=<domain-value>; Secure; HttpOnly, while Cookie is Cookie: <cookie-name-1>=<cookie-value-1>; <cookie-name-2>=<cookie-value-2>; <cookie-name-3>=<cookie-value-3>. This means that if you receive multiple cookies in a Cookie header, the HttpCookie code treats it as one cookie with multiple parameters, instead of multiple cookies.

Steps To Reproduce

No response

Environment Information

  • JDK 17

Example Application

No response

Version

4.3.3

diamondq avatar Jan 31 '24 00:01 diamondq

@diamondq thanks for submitting an issue. Do you have an example which reproduces cookies not being parsed correctly?

sdelamo avatar Mar 28 '24 09:03 sdelamo

It is really difficult from the interface names (ClientCookieEncoder, ServerCookieDecoder, ServerCookieEncoder) to tell which interface encodes/decodes which header.

  • ClientCookieEncoder encodes the Cookie header
  • ServerCookieEncoder encodes the Set-Cookie header
  • ServerCookieDecoder decodes the Cookie header

I can see two issues:

  • CookieElement/CookiesElement in the netty access log handler mixes the two headers arbitrarily and uses ServerCookieDecoder for both
  • DefaultServerCookieDecoder parses Set-Cookie syntax instead of Cookie syntax.

imo the docs need to be a bit more clear on this, and we need to fix DefaultServerCookieDecoder. I don't see a Cookie header decoder in the jdk though so not sure how.

Also we might want to fix the access log handler.

yawkat avatar Mar 28 '24 11:03 yawkat

Following this, it seems that core > 4.3.13 removed the failing cookie decoder which solves the issue. Otherwise adding this service will fix the issue for those in lower versions

brian-mulier-p avatar Apr 24 '24 09:04 brian-mulier-p