Chris, I just thought that I have some concerns passing a map with the headers to generateCookie() method. This means that for each call the caller will have to read all headers from the coyote.Response and put them in a map, even if the CookieProcessor will not need them, as is the case with the legacy cookie processor and the rfc cookie processor. This might have performance impact. Isn't it more optimal to just pass the o.a.c.connector.Request - like generateCookie (Request, Cookie) - and then if the cookie processor implementation needs headers, it will take them - only these which it needs - from the Response. What do you think?
Lazar On Fri, Feb 28, 2020, 17:08 Lazar Kirchev <lazar.kirc...@gmail.com> wrote: > > Chris, > > Actually in my preferred option the implementation in the > CookieProcessorBase should not be no-op, but it should call > CookieProcessor.generateCookie(Cookie). And the calls to > CookieProcessor.generateCookie(Cookie) in o.a.c.connector.Response and > o.a.c.core.ApplicationPushBuilder should be replaced with calls to the new > method. > > Lazar > > On Fri, Feb 28, 2020 at 3:58 PM Lazar Kirchev <lazar.kirc...@gmail.com> > wrote: > >> Chris, >> >> Yes, I will prepare a PR in the next days. However, as Tomcat 8.5 should >> be able to work both on Java 7 and Java 8, interface default methods can't >> be used. So would you prefer to have a second >> CookieProcessor.generateCookie(Map<> >> requestHeaders, Cookie) in addition to the existing >> CookieProcessor.generateCookie(Cookie), >> and provide a no-op implementation in the CookieProcessorBase class, or to >> change the signature of the existing method instead? I myself prefer the >> first option, with adding a second method. >> >> Lazar >> >> On Mon, Feb 24, 2020 at 5:19 PM Christopher Schultz < >> ch...@christopherschultz.net> wrote: >> >>> -----BEGIN PGP SIGNED MESSAGE----- >>> Hash: SHA256 >>> >>> Lazar, >>> >>> On 2/24/20 02:05, Lazar Kirchev wrote: >>> > Chris, >>> > >>> > CookieProcessor.generateCookie(Map<> requestHeaders, Cookie) will >>> > work perfectly for me and I guess for anyone who needs to check the >>> > client version. >>> >>> Want to prepare a PR? >>> >>> - -chris >>> >>> > On Fri, Feb 21, 2020 at 7:17 PM Christopher Schultz < >>> > ch...@christopherschultz.net> wrote: >>> > >>> > Lazar, >>> > >>> > On 2/21/20 10:29, Lazar Kirchev wrote: >>> >>>> Yes, the SameSite attribute is still in a draft and this >>> >>>> causes the mess, at least partly.> And yes, I was thinking >>> >>>> about something like that - >>> >>>> CookieProcessor.generateCookie(String userAgent, Cookie) or >>> >>>> CookieProcessor.generateCookie(Map<> requestHeaders, Cookie). >>> >>>> I >>> > absolutely >>> >>>> agree that this would be very hacky. And it might be >>> >>>> dangerous as CookieProcessor is an interface and there >>> >>>> already might be custom implementations. >>> > >>> > We can fix that with default implementations, for Java versions >>> > that support such thing (Java >= 1.8). >>> > >>> >>>> But can you think of another way of making the cookie >>> >>>> generation logic aware of the user agent header value? >>> > >>> > Not really. >>> > >>> > I have a preference for: >>> > >>> > CookieProcessor.generateCookie(Map<> requestHeaders, Cookie) >>> > >>> > This is because User-Agent might not be the only header which is >>> > useful, here. For example, Google Chrome (amusingly enough) will >>> > be removing the User-Agent header[1] in favor of "client >>> > hints"[2]. >>> > >>> > So you might have to look at more than one header at a time, >>> > including possibly User-Agent. >>> > >>> > -chris >>> > >>> > [1] >>> > https://www.zdnet.com/article/google-to-phase-out-user-agent-strings-i >>> n- >>> > >>> > >>> chrome/ >>> > <https://www.zdnet.com/article/google-to-phase-out-user-agent-strings- >>> in-chrome/ >>> <https://www.zdnet.com/article/google-to-phase-out-user-agent-strings-in-chrome/> >>> > >>> > >>> > [2] https://wicg.github.io/ua-client-hints/ >>> > >>> >>>> On Fri, Feb 14, 2020 at 8:59 PM Christopher Schultz < >>> >>>> ch...@christopherschultz.net> wrote: >>> >>>> >>> >>>> Lazar, >>> >>>> >>> >>>> On 2/14/20 05:36, Lazar Kirchev wrote: >>> >>>>>>> Chris, >>> >>>>>>> >>> >>>>>>> Just FYI or in case someone else hits this problem. >>> >>>>>>> >>> >>>>>>> Actually I had to use the response wrapper approach >>> >>>>>>> for Tomcat 8.5.50 as well. As described by Chrome in >>> >>>>>>> https://www.chromium.org/updates/same-site/incompatible-clients, >>> >>>>>>> >>> >>>>>>> >>> > >>> >>>>>>> >>> there are older browser versions which do not support the SameSite >>> >>>>>>> attribute at all and reject the cookies which contain >>> >>>>>>> it. Although Tomcat 8.5.42 and later provide the >>> >>>>>>> CookieProcessor configuration for the SameSite >>> >>>>>>> attribute, it is a problem if one wants to support >>> >>>>>>> older browser versions as well. >>> >>>> Wow, what a huge pain in the neck. I don't see anything in >>> >>>> RFC 6265 that says anything about rejecting cookies with >>> >>>> unknown attributes, but I also don't see anything prohibiting >>> >>>> that behavior, either. Than again, RFC 6265 doesn't mention >>> >>>> the SameSite attribute at all, so ... there is that. >>> >>>> >>> >>>> This is what you get when vendors try to implement standards >>> >>>> before they are standards. >>> >>>> >>> >>>>>>> Adding the SameSite attribute in order to support >>> >>>>>>> newest Chrome breaks the old ones as the configuration >>> >>>>>>> via the CookieProcessor does not allow for user agent >>> >>>>>>> sniffing. Even if you extend the existing >>> >>>>>>> CookieProcessor implementations or create your own, you >>> >>>>>>> cannot get the request headers in it so that you can >>> >>>>>>> check for the browser version. If one needs such >>> >>>>>>> flexibility, only the response wrapper helps. Do you >>> >>>>>>> think that it makes sense to provide a mechanism in >>> >>>>>>> the CookieProcessor to get access to the request >>> >>>>>>> headers in order to check the user agent? >>> >>>> Are you referring to CookieProcessor.generateCookie(Cookie)? >>> >>>> So the proposal would be to change that to >>> >>>> CookieProcessor.generateCookie(String userAgent, Cookie)? Or >>> >>>> maybe even CookieProcessor.generateCookie(Map<> >>> >>>> rquestHeaders, Cookie)? >>> >>>> >>> >>>> It seems super hacky to do it that way, but I'm not sure I >>> >>>> see another option for introducing SameSite in a compatible >>> >>>> way. >>> >>>> >>> >>>> -chris >>> >>>>> >>> >>>>> ------------------------------------------------------------------ >>> - --- >>> >>>>> >>> >>>>> >>> > >>> >>>>> >>> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org >>> >>>>> For additional commands, e-mail: >>> >>>>> users-h...@tomcat.apache.org >>> >>>>> >>> >>>>> >>> >>>> >>> >> >>> >> --------------------------------------------------------------------- >>> >> >>> >> >>> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org >>> >> For additional commands, e-mail: users-h...@tomcat.apache.org >>> >> >>> >> >>> > >>> -----BEGIN PGP SIGNATURE----- >>> Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/ >>> >>> iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl5T6WwACgkQHPApP6U8 >>> pFhGHxAAwiVqrNm6k4LjfFedovPEVPADUqGe1cT9UIB1seFUhPJ2u1REgVhOoAsq >>> EuIxnn69nRpqqtp31petFk7D1XMw9HQHgr6dXBJILL+fPxqZxvavDeM+jqXL/D4O >>> +UTzz85EXMl0A/HVkIYR9tb0kW3lgLEvdyYeWQB+0sz3pzdyIxW6ZtKOfRFOwjff >>> 8ptTKz6XJN3gWyZ5dOwsJ+umPQeqOLoxn9bmlKJnXFZHsfzVhBUy2T0b0NmZguyX >>> hRNfnNDF7cAvQjWPzM9CgqyZlTtcVJGZ2ugwkK7C1PGQXXnMrCLDm6rKLOBYIsXW >>> RHBedq0b+T1QDnM9imYjySNTr5mLZg5sHeeQ8RhWgxMBW4wfvTCqbHm4RZurOeXj >>> ZgMfj8r7zcy2becdo5dCC73e7r8B0F0MumcbqN02y1z6eVysKut4UJFQFB7L408H >>> PMgclJVVNc+bQeRI0Vs8IYA/FP6gm7Cow/Tk6OeAdOx+JhJuWFS/DEwTAahGD2pS >>> bOGUHmOq/HlfofjbSjAiBPrw+18WBPaFscw366s3W6NhETJVsjEF+DShi8SQ/+Ps >>> cOHgfmBn0yHbkKiBDvqe3oqqPBtvh0rP4fIJ8wfVS2BIBEAAj8+XTNoiRciZa/kM >>> afSP2HtGdN/4hxW6lc31kePN82kkO9cjm6IEfck0dzae5/mmlDs= >>> =KXMS >>> -----END PGP SIGNATURE----- >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org >>> For additional commands, e-mail: users-h...@tomcat.apache.org >>> >>>
