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