RFR: 8351983: HttpCookie Parser Incorrectly Handles Cookies with Expires Attribute [v5]

[Volkan Yazici]

On Fri, 13 Jun 2025 14:49:24 GMT, Michael McMahon <michaelm at openjdk.org> wrote:

>> src/java.base/share/classes/java/net/HttpCookie.java line 892:
>> 
>>> 890:             assignAttribute(cookie, name, value);
>>> 891:         }
>>> 892:         assignMaxAgeAttribute(cookie, expiresValue, maxAgeValue);
>> 
>> @Michael-Mc-Mahon, instead of making an exception for `max-age` and `expires`, and removing them from `assignors`, can't we convert the type of `assignors` from `Map` to `List` and add `max-age` & `expires` entries at the end?
>
>> @Michael-Mc-Mahon, instead of making an exception for `max-age` and `expires`, and removing them from `assignors`, can't we convert the type of `assignors` from `Map` to `List` and add `max-age` & `expires` entries at the end?
> 
> Just converting from Map to List wouldn't be enough. The problem is that both attribute types need to be handled together. You could change the attribute name recognition to some kind of pattern match to recognise either of them. Then you need to know which of them was set and what their values were.
> 
> Maybe, I could at least use the assignor pattern to recognise the two attributes and limit the special code to just actioning the values. I'll take a look at that now.

I think the last commit (b22113118) just worsened things – now the logic is spread across `assignMaxAgeAttribute`, `assignors`, and instance variables, whereas earlier it was only in `assignMaxAgeAttribute`. :face_with_diagonal_mouth: I suggest simply reverting it, that is, switching the state back to 9a495d7f9a5e.

I agree that introducing a smarter data structure and iteration scheme to `assignors` would simplify things, though that is probably out of the scope of this work.

Apologies for the inconvenience and thanks so much for your patient cooperation. 🙇

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25636#discussion_r2152942047