Hi Grzegorz,

Thanks a lot, this looks great. I will try to test that as soon as I find the 
time for it.

Best regards
Stephan

-----Original Message-----
From: Grzegorz Grzybek <gr.grzy...@gmail.com> 
Sent: Wednesday, 25 May 2022 13:05
To: dev@karaf.apache.org
Subject: Re: Karaf 4.4.0/Pax-Web 8.0.2 Tomcat context handling

Hello

wt., 24 maj 2022 o 15:28 Siano, Stephan <stephan.si...@sap.com.invalid>
napisał(a):

> Hi Grzegorz,
>
> > These are kind of inline dev notes. I can imagine that they may blur 
> > the
> code, but I really tried to ensure that everything is consistent, so I 
> spent a lot of time reading the
> > original Tomcat/Jetty/Undertow code and writing down some observations.
> > This note just states what Tomcat itself is doing and it uses it for
> $CATALINA_HOME/conf/context.xml file which is common to ALL web 
> applications. That's definitely
> > something we DON'T want in Pax Web.
>
> Well, this global context.xml in Tomcat is for customizing all web 
> applications to the environment. This might as well be a use case for 
> the karaf container. Currently some of the stuff like authentication 
> configuration is hard-coded in the pax-web-tomcat coding, however we 
> need to find a way to customize this somehow...
>

I've created and fixed
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fops4j%2Forg.ops4j.pax.web%2Fissues%2F1723&amp;data=05%7C01%7Cstephan.siano%40sap.com%7C617076115dd54a790a2708da3e3e63d6%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C637890734929332755%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=%2FvdjG0ax56a7L2t7pa07zB4Mj1knwRiM79L3CCgjc4A%3D&amp;reserved=0

I found i indeed useful, so I've added support for another PID configuration 
option - `org.ops4j.pax.web.context.file` If you install pax-web-http-jetty or 
pax-web-http-tomcat (no support for Undertow), you can configure a global 
"context configuration file" which
matches:

   - for Jetty, the jetty-web.xml file documented in
   
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.eclipse.org%2Fjetty%2Fdocumentation%2Fjetty-9%2Findex.html%23using-basic-descriptor-files&amp;data=05%7C01%7Cstephan.siano%40sap.com%7C617076115dd54a790a2708da3e3e63d6%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C637890734929332755%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=sssbzXtw3aykNaWivNHMTmj7BYEzUT8i3CpPpIVRvDs%3D&amp;reserved=0
   - it was already handled from /WEB-INF/jetty-web.xml
   - for Tomcat, the tomcat-context.xml file documented in
   
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftomcat.apache.org%2Ftomcat-9.0-doc%2Fconfig%2Fcontext.html&amp;data=05%7C01%7Cstephan.siano%40sap.com%7C617076115dd54a790a2708da3e3e63d6%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C637890734929332755%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=0t4Ey84q3NKm0AAXkwx3f1q1nbXTzR%2BDvyFG4941HMg%3D&amp;reserved=0
 - it was
   already handled from META-INF/context.xml - Tomcat reads it from
   $CATALINA_HOME/conf/context.xml

These options are disabled by default, but I provide template versions of 
jetty-web.xml and tomcat-context.xml

For example, tomcat-context.xml contains:

<Context>
    <!--
    <Valve className="org.apache.catalina.valves.RemoteAddrValve"
        allow="127\.\d+\.\d+\.\d+|::1|0:0:0:0:0:0:0:1"/>
    -->
</Context>

If you uncomment the valve and "org.ops4j.pax.web.context.file = 
${karaf.etc}/tomcat-context.xml" option, the valve rejects request from non 
local addresses - I hope that's what you're looking for.

kind regards
Grzegorz Grzybek


>
> Best regards
> Stephan
>
> -----Original Message-----
> From: Grzegorz Grzybek <gr.grzy...@gmail.com>
> Sent: Tuesday, 24 May 2022 11:44
> To: dev@karaf.apache.org
> Subject: Re: Karaf 4.4.0/Pax-Web 8.0.2 Tomcat context handling
>
> Hello
>
> wt., 24 maj 2022 o 09:59 Siano, Stephan 
> <stephan.si...@sap.com.invalid>
> napisał(a):
>
> > Hi,
> >
> > I have been trying around with Karaf 4.4.0 with the pax-web-tomcat 
> > webcontainer in the last days. First of all I want to state that 
> > Grzegorz did a fantastic job with the Pax-Web refactoring!
> >
>
> It was my pleasure refactoring ;) I'd never have written/designed 
> anything as complex as Pax Web in the first place, so big thanks to original 
> authors!
>
>
> > Nevertheless, I encountered a few issues with tomcat context.xml
> handling.
> >
> > First of all prior versions of pax-web-tomcat were able to load a 
> > default context.xml from the same directory as the tomcat-server.xml.
> > I am not sure how to proceed with this.
> > The BundleWebApplication class in the pax-web-extender-war bundle 
> > contains the following comment:
> >                                 // additionally:
> >                                 //  - Tomcat handles context.xml 
> > files set by 
> > org.apache.catalina.core.StandardContext.setDefaultContextXml()
> >
>
> These are kind of inline dev notes. I can imagine that they may blur 
> the code, but I really tried to ensure that everything is consistent, 
> so I spent a lot of time reading the original Tomcat/Jetty/Undertow 
> code and writing down some observations.
> This note just states what Tomcat itself is doing and it uses it for 
> $CATALINA_HOME/conf/context.xml file which is common to ALL web 
> applications. That's definitely something we DON'T want in Pax Web.
>
>
> > However, as far as I understand, the defaultContextXml in 
> > StandardContext is not used in the embedded Tomcat. The only uses I 
> > could find was in the
> > org.apache.catalina.startup.ContextConfig.contextConfig() method 
> > which does not seem to get called by the embedded tomcat startup. 
> > This method is normally called with a 
> > ContextConfig.lifecycleEvent(Lifecycle.AFTER_INIT_EVENT), but it 
> > seems that this ContextConfig lifecycle is not called. I am not sure 
> > what is the way to go here, either to try to include this 
> > ContextConfig lifecycle, try to exeute the contextConfig method with 
> > the digester we get in the
> > TomcatFactory.createContextDigester() method, or try to funnel this 
> > default context.xml somehow into the serverSpecificDescriptors and 
> > parse it in the tomcat specific OsgiContextConfiguration class. What 
> > do
> you think?
> >
>
> While the default org.ops4j.pax.web PID created when you install 
> pax-web-http-tomcat feature contains:
>
> #org.ops4j.pax.web.config.file = ${karaf.etc}/tomcat-server.xml
>
> and sample tomcat-server.xml is provided, that's more natural to be 
> treated as "global" configuration. I'd like to avoid handling external 
> context.xml for all WABs.
>
>
> >
> > The code does support context.xml files in WARs. The context.xml 
> > files are found within a war by the BundleWebApplication class and 
> > forwarded as serverSpecificDescriptors to the web container implementation.
> > These descriptors are applied in class 
> > org.ops4j.pax.web.service.tomcat.internal.OsgiContextConfiguration.
> > However, the war extender will find context files both in 
> > /META-INF/context.xml and /WEB-INF/classes/META-INF/context.xml
>
>
> Yes - and `/META-INF/context.xml` is searched using
> org.osgi.framework.Bundle#findEntries() (search without using 
> classloaders, include fragments), additional search is performed for 
> all non-jar entries from Bundle-ClassPath headers - just to make it 
> clear that first
> org.osgi.framework.Bundle#findEntries() doesn't return 
> /WEB-INF/classes/META-INF/context.xml.
>
>
> > (actually it will find any invocation of /META-INF/context.xml in 
> > the bundle’s classpath), whereas the OsgiContextConfiguration class 
> > explicitly limits this to path.equals("/META-INF/context.xml") which 
> > will include the first but exclude the second location. I am not 
> > sure if this is correct (but it seems a bit odd to me). This results 
> > in the
> logs below:
> >
> > 2022 05 23
> > 14:37:26#+00#DEBUG#org.ops4j.pax.web.extender.war.internal.model.Bun
> > dl 
> > eWebApplication##anonymous#wab-extender-1-thread-3####na#na#na#na#Fo
> > un
> > d
> > Tomcat-specific descriptor:
> > bundle://2157ae21-cf1a-4966-8b3e-c768aaacf073_230.0:0/META-INF/conte
> > xt
> > .xml|
> > 2022 05 23
> > 14:37:26#+00#DEBUG#org.ops4j.pax.web.extender.war.internal.model.Bun
> > dl 
> > eWebApplication##anonymous#wab-extender-1-thread-3####na#na#na#na#Fo
> > un
> > d
> > Tomcat-specific descriptor:
> > bundle://2157ae21-cf1a-4966-8b3e-c768aaacf073_230.0:0/WEB-INF/classe
> > s/
> > META-INF/context.xml|
> >
> > 2022 05 23
> > 14:37:26#+00#INFO#org.ops4j.pax.web.service.tomcat.internal.OsgiCont
> > ex
> > tConfiguration##anonymous#paxweb-config-3-thread-1
> > (deploy /auth)####na#na#na#na#Processing context specific 
> > bundle://2157ae21-cf1a-4966-8b3e-c768aaacf073_230.0:0/META-INF/conte
> > xt
> > .xml
> > for /auth|
> >
> > Is that behavior correct? It might probably be a good idea to relax 
> > the filter in OsgiContextConfiguration a bit.
> >
>
> and actually you're right - I've explicitly searched for context.xml 
> in multiple locations, but didn't use it when actually applying the 
> configuration. The check is needed to not include for example Jetty's 
> WEB-INF/jetty-web.xml when using Tomcat, but simple change from:
>
> if (path.equals("/META-INF/context.xml")) {
>
> to:
>
> if (path.endsWith("/META-INF/context.xml")) {
>
> is probably what we need.
>
>
> >
> > I am willing to contribute to this, but I would prefer to discuss 
> > the correct way to go first.
> >
>
> I've created
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fops4j%2Forg.ops4j.pax.web%2Fissues%2F1721&amp;data=05%7C01%7C
> stephan.siano%40sap.com%7C617076115dd54a790a2708da3e3e63d6%7C42f7676cf
> 455423c82f6dc2d99791af7%7C0%7C0%7C637890734929332755%7CUnknown%7CTWFpb
> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%7C3000%7C%7C%7C&amp;sdata=4%2FcgSuRlTVkiwJY1wyevOLYBwduhIQVoh60%2F
> E3gWOck%3D&amp;reserved=0
> to track this issue - thanks!
> And while I've checked, I can immediately fix it ;)
>
> kind regards
> Grzegorz Grzybek
>
>
> > Best regards
> > Stephan
> >
>

Reply via email to