Chris, Any update on this? Did you have a chance to have a look on the PR?
Lazar On Wed, Mar 4, 2020 at 10:55 AM Lazar Kirchev <lazar.kirc...@gmail.com> wrote: > Chris, Martin, > > Here is the PR: https://github.com/apache/tomcat/pull/252 > > Lazar > > On Sat, Feb 29, 2020 at 8:27 AM Martin Grigorov <mgrigo...@apache.org> > wrote: > >> On Fri, Feb 28, 2020 at 7:31 PM Lazar Kirchev <lazar.kirc...@gmail.com> >> wrote: >> >> > 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? >> > >> >> I agree that this is a better way! >> >> Martin >> >> >> > >> > 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 >> > >>> >> > >>> >> > >> >
