On Fri, 13 Jun 2025 14:49:24 GMT, Michael McMahon <micha...@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
