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 > > >>> > > >>> > > >
