On Wed, Jul 8, 2020 at 8:55 PM Christopher Schultz <
ch...@christopherschultz.net> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Rémy,
>
> On 7/8/20 11:47, Rémy Maucherat wrote:
> > On Wed, Jul 8, 2020 at 5:10 PM Christopher Schultz
> > <ch...@christopherschultz.net
> > <mailto:ch...@christopherschultz.net>>
> wrote:
> >
> > Rémy,
> >
> > On 7/8/20 10:35, Rémy Maucherat wrote:
> >> On Wed, Jul 8, 2020 at 4:26 PM Christopher Schultz
> >> <ch...@christopherschultz.net
> >> <mailto:ch...@christopherschultz.net>
> >> <mailto:ch...@christopherschultz.net
> > <mailto:ch...@christopherschultz.net>>> wrote:
> >
> >>>> Clearly, no, with multiple elements, the digester rules
> >>>> added to ContextRuleSet would be something like (in addition
> >>>> to the unchanged ones for CookieProcessor):
> >>>
> >>> digester.addObjectCreate(prefix + "Context/CookieProcessor",
> >>>
> >>>> "org.apache.tomcat.util.http.Rfc6265CookieProcessor",
> >>> "className"); digester.addSetProperties(prefix +
> >>> "Context/CookieProcessor"); digester.addSetNext(prefix +
> >>> "Context/CookieProcessor", "setCookieProcessor",
> >>> "org.apache.tomcat.util.http.CookieProcessor");
> >>>
> >>> digester.addObjectCreate(prefix +
> >>>> "Context/CookieProcessor/SameSiteCookie",
> >>>
> >>>> "org.apache.tomcat.util.http.SameSiteCookie",
> >>> "className"); digester.addSetProperties(prefix +
> >>>> "Context/CookieProcessor/SameSiteCookie");
> >>> digester.addSetNext(prefix +
> >>>> "Context/CookieProcessor/SameSiteCookie",
> >>> "addSameSiteCookie",
> >>> "org.apache.tomcat.util.http.SameSiteCookie");
> >>>
> >>>> If the bean class is
> >>>> org.apache.tomcat.util.http.SameSiteCookie.
> >
> >> So you are okay with:
> >
> >> digester.addSetProperties(prefix +
> >> "Context/CookieProcessor/SameSiteCookie");
> >
> >
> >> But not with:
> >
> >> digester.addCallMethod(prefix +
> >> "Context/CookieProcessor/SameSiteCookie/Cookie");
> >
> >> ?
> >
> >
> >> The digester works best with beans so I don't see what is so
> >> surprising about this.
> >
> > What's surprising is that you are -1 to what I see as an
> > improvement to Tomcat without much in the way of disruption. I
> > think it's pretty clean.
> >
> > Would proposing an actual patch help, or will you still be -1 on
> > anything other than a complicated "sameSiteCookies" string value
> > which needs to be unpacked by Tomcat code rather than using
> > standard XML element/attribute syntax?
> >
> >
> >> The digester rules I posted add a new SameSiteCookie element I
> >> only>> mentioned a complex sameSiteCookies attribute syntax once
> >> (I considered it would be better for API compatibility; the
> >> CookieProcessor API should not break in Tomcat 9 when things are
> >> backported, or the feature would be limited to Tomcat 10) but I
> >> didn't mention it again since you did not like it.
>
> I can't tell you if you are saying you'd be -1 to add that new
> SameSiteCookie element (because it is "too big a change", or because
> it requires "changes to digester configuration"), or that you are -1
> to use digester.addCallMethod (because you don't like it), or that you
> are -1 to really supporting any improvements to SameSite at all.
>
> Can you clarify?
>

Ok, another attempt then.

A SameSiteCookie element can be added using the following extra rules in
ContextRuleSet:

        digester.addObjectCreate(prefix +
"Context/CookieProcessor/SameSiteCookie",

 "org.apache.tomcat.util.http.SameSiteCookie",
                                 "className");
        digester.addSetProperties(prefix +
"Context/CookieProcessor/SameSiteCookie");
        digester.addSetNext(prefix +
"Context/CookieProcessor/SameSiteCookie",
                            "addSameSiteCookie",
                            "org.apache.tomcat.util.http.SameSiteCookie");

Other ways to add the element to server.xml don't work as well. It also
keeps the embedded API consistent and storeconfig is likely the usual cut
and paste.

Then I'm wondering about compatibility with the current CookieProcessor
interface. If it's not compatible, the change would de facto be limited to
Tomcat 10.


> I won't waste my time if you are going to -1 anything I do, here.
>

I don't understand what was hard to understand in this case. Anyway, I had
to pull quite a few ideas in the past personally, often due to a "it would
break something" problem.

Rémy


>
> - -chris
> -----BEGIN PGP SIGNATURE-----
> Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/
>
> iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl8GFogACgkQHPApP6U8
> pFg1UQ/+M9SU2f9YkLUnRU0f2MqdIwKFguLHFfFxZSnxoEOhIVJOmJbm5YrZlwIv
> skzgJiPvX7wMWfq0wO8mc/ALjOAn/XhEE66sIWZe9bc9Hte0mrXHV8wmHZXhTW+O
> LqLNGJgXH/m8Mxui27h2EfpRXYDEKEGLUU71j3NMOxd6xa0xfYv/MiBI3asMZpvQ
> k688fWGa4EeoTj4yRtR+eIOYpcPFZlaT+uNFGPNr7HVMvSB0SAx/Ui732bMEXpRT
> 2CRaPSZ4TAFBhZLNXxfBodErGWA6QHQPgnPgB0oPbCM78R0zldVyXLMRpksZL/V7
> Dx6cJE/Da7fISufomcMsdrpFmQ3LylXbKNTdbkzlDT9hdpgSn7kc4tDYJzwTt0NS
> K2/fbi8gQOEg7VB6pRVeXxODhEovFizEKfuGhwFNyuw7q0cxK+w6W+zFdu+wgcj/
> Epa/XDmtuKz5doGOnuwzpudf3For8rdVpdpmRUU7+1PiLX6xsj2cueY1ku7e45LL
> ZR9cMGkW/sEU1oWw2HKM3fMqAuZEkFiNvYhua/KM6tVbqFXC49GeF+K+qe/zx2DK
> W+xrPOnQ2A4viyLVAFbidPkxemdnp5VX+VXeXvgmCeRmsmWqvza73UJzf1Es4qg5
> inLHbksUnsokixJHF2KwbsFFdubldl5HogNIpExmwwlb9Eg2fFI=
> =j71t
> -----END PGP SIGNATURE-----
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Reply via email to