fix(cookie): parse request Cookie header as name/value pairs - #12871
fix(cookie): parse request Cookie header as name/value pairs#12871kali834x wants to merge 1 commit into
Conversation
HDPark95
left a comment
HDPark95
left a comment
There was a problem hiding this comment.
I found one malformed-value case where the two server decoders still disagree:
String header = "a=\"unterminated; b=2";At a915b6c, NettyLaxServerCookieDecoder returns only [b], while the new DefaultServerCookieDecoder returns [a, b] (with a's value still starting with the unmatched quote). Netty's decoder drops a value that starts with " but does not end with "; this implementation's unwrapValue instead returns that value unchanged.
That leaves the runtime-dependent behavior this change is trying to remove, and accepts an unparsable pair rather than skipping it. Could addCookie/unwrapValue signal and drop an unbalanced opening quote? I tested a minimal pre-unwrapping guard: the full :micronaut-http:test task passed 1,697 tests, and a cross-decoder spec covering this input passed both assertions.
There was a problem hiding this comment.
Pull request overview
This PR fixes DefaultServerCookieDecoder to correctly parse an inbound HTTP Cookie request header as a sequence of name=value pairs (RFC 6265 §4.2.1), aligning behavior with the Netty-based decoder and avoiding treating Path/Domain as attributes or failing the whole header on malformed pairs.
Changes:
- Replaced
java.net.HttpCookie.parseusage with manual;-delimited cookie-pair parsing and quoted-value unwrapping. - Changed behavior to skip empty/unparsable pairs instead of throwing on malformed input.
- Updated/expanded tests to cover multiple cookies, malformed pairs, and quoted values.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| http/src/main/java/io/micronaut/http/cookie/DefaultServerCookieDecoder.java | Reimplements Cookie header parsing to produce correct cookie-pairs and skip malformed segments safely. |
| http/src/test/java/io/micronaut/http/cookie/DefaultServerCookieDecoderTest.java | Updates expectations and adds coverage for multi-cookie, malformed, and quoted-value parsing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| int equals = header.indexOf('=', start); | ||
| if (equals == -1 || equals >= end) { | ||
| return; | ||
| } |
| try { | ||
| cookies.add(Cookie.of(name, value)); | ||
| } catch (IllegalArgumentException e) { | ||
| // skip a single malformed pair rather than rejecting the whole header | ||
| } |
DefaultServerCookieDecoder.decode parses an inbound Cookie request header on runtimes without http-netty, but delegates to java.net.HttpCookie.parse, a Set-Cookie response parser. on a Cookie header with several pairs it returns only the first and drops the rest (SID=a; JSESSIONID=b yields just SID), throws IllegalArgumentException on a malformed name so a bad request turns into a 500, and treats Path/Domain/Max-Age as attributes on the inbound cookie, letting a client set cookie.getPath() which NettyCookies uses to filter which cookies reach the app. the NettyLaxServerCookieDecoder sibling backed by netty ServerCookieDecoder.LAX already parses it as a plain name/value list, so the two SPI implementations disagree on identical input. rewrite decode to split on ';', unwrap balanced quotes, and skip empty or unparsable pairs instead of interpreting attributes or throwing.