Chris, Did you managed to check the PR?
Lazar On Tue, Mar 10, 2020 at 5:41 PM Lazar Kirchev <lazar.kirc...@gmail.com> wrote: > 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 >>> > >>> >>> > >>> >>> > >>> >>
