On Wed, Jul 26, 2017 at 3:21 PM, Konstantin Kolinko <knst.koli...@gmail.com> wrote:
> (Updated. Occasionally sent too early.) > > 2017-07-26 16:14 GMT+03:00 Konstantin Kolinko <knst.koli...@gmail.com>: > > 1. This does not belong to Coyote (as you wrote in changelog.xml). > > I see why it belongs to Digester: it uses Digester.propertySource. > > In changelog this is "Other" section. > > > 2. Lack of documentation. > > Listeners are documented in config/listeners.xml. E.g.: > > > > http://tomcat.us.apache.org/tomcat-8.5-doc/config/listeners.html > > > > 3. It does not use all property sources used by Digester. (It uses > > Digester.propertySource, not Digester.source). > > > > Generally, this means that it does not perform substitutions of > > ${system properties} (handled by Digester$SystemPropertySource), but > > only of those handled by custom property source. > > > > Generally, substitution of system properties is a wanted feature, but > > there are some properties that do not need such replacement, e.g. > > "common.loader" property. > > > > 4. If this feature is configured as a <Listener>, it means that it > > will be performed only on Tomcat startup, but not on shutdown (where > > only the root element of server.xml is parsed). > > Based on #3, #4 I thought that such feature (replacement of system > properties) could be configurable in catalina.properties, with > configuration of what properties are not subject for replacement. > The purpose is to use the property source configured and only that one. The (weird) source field from digester only used to add a system property replacer, which I feared would be bad here (so it won't property replace "common.loader" or anything internal unless the property source configured has it). It is documented with the property source since this is where it belongs as far as I am concerned. I added the listener this way to the Digester class to not allow wider access to the property source (which is a location where obsfucated values may be accessible). We use the property source for integration with this: https://github.com/picketbox/tomcat-vault I'm not convinced this is a very useful feature, but hey, it doesn't hurt too much that way ... Rémy > > Best regards, > Konstantin Kolinko > > > 2017-07-26 15:15 GMT+03:00 <r...@apache.org>: > >> Author: remm > >> Date: Wed Jul 26 12:15:45 2017 > >> New Revision: 1803038 > >> > >> URL: http://svn.apache.org/viewvc?rev=1803038&view=rev > >> Log: > >> 61345: Add a server listener that can be used to do system property > replacement from the property source configured in the digester. > >> > >> Modified: > >> tomcat/trunk/java/org/apache/tomcat/util/digester/Digester.java > >> tomcat/trunk/java/org/apache/tomcat/util/digester/ > LocalStrings.properties > >> tomcat/trunk/webapps/docs/changelog.xml > >> tomcat/trunk/webapps/docs/config/systemprops.xml > >> > >> Modified: tomcat/trunk/java/org/apache/tomcat/util/digester/Digester. > java > >> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/ > tomcat/util/digester/Digester.java?rev=1803038&r1=1803037& > r2=1803038&view=diff > >> ============================================================ > ================== > >> --- tomcat/trunk/java/org/apache/tomcat/util/digester/Digester.java > (original) > >> +++ tomcat/trunk/java/org/apache/tomcat/util/digester/Digester.java > Wed Jul 26 12:15:45 2017 > >> @@ -29,12 +29,17 @@ import java.util.EmptyStackException; > >> import java.util.HashMap; > >> import java.util.List; > >> import java.util.Map; > >> +import java.util.Properties; > >> import java.util.PropertyPermission; > >> +import java.util.Set; > >> > >> import javax.xml.parsers.ParserConfigurationException; > >> import javax.xml.parsers.SAXParser; > >> import javax.xml.parsers.SAXParserFactory; > >> > >> +import org.apache.catalina.Lifecycle; > >> +import org.apache.catalina.LifecycleEvent; > >> +import org.apache.catalina.LifecycleListener; > >> import org.apache.juli.logging.Log; > >> import org.apache.juli.logging.LogFactory; > >> import org.apache.tomcat.util.ExceptionUtils; > >> @@ -307,6 +312,35 @@ public class Digester extends DefaultHan > >> } > >> } > >> > >> + > >> + public static class SystemPropertyReplacementListener > >> + implements LifecycleListener { > >> + protected Log log = LogFactory.getLog(Digester.class); > >> + protected StringManager sm = StringManager.getManager( > Digester.class); > >> + @Override > >> + public void lifecycleEvent(LifecycleEvent event) { > >> + if (propertySource != null && > >> Lifecycle.BEFORE_INIT_EVENT.equals(event.getType())) > { > >> + IntrospectionUtils.PropertySource[] propertySources = > >> + new IntrospectionUtils.PropertySource[] { > propertySource }; > >> + Properties properties = System.getProperties(); > >> + Set<String> names = properties.stringPropertyNames(); > >> + for (String name : names) { > >> + String value = System.getProperty(name); > >> + if (value != null) { > >> + try { > >> + String newValue = > >> IntrospectionUtils.replaceProperties(value, > null, propertySources); > >> + if (value != newValue) { > >> + System.setProperty(name, newValue); > >> + } > >> + } catch (Exception e) { > >> + > >> log.warn(sm.getString("digester.failedToUpdateSystemProperty", > name, value), e); > >> + } > >> + } > >> + } > >> + } > >> + } > >> + } > >> + > >> > >> // ------------------------------------------------------------- > Properties > >> > >> > >> Modified: tomcat/trunk/java/org/apache/tomcat/util/digester/ > LocalStrings.properties > >> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/ > tomcat/util/digester/LocalStrings.properties?rev= > 1803038&r1=1803037&r2=1803038&view=diff > >> ============================================================ > ================== > >> --- > >> tomcat/trunk/java/org/apache/tomcat/util/digester/LocalStrings.properties > (original) > >> +++ > >> tomcat/trunk/java/org/apache/tomcat/util/digester/LocalStrings.properties > Wed Jul 26 12:15:45 2017 > >> @@ -15,3 +15,4 @@ > >> > >> disgester.encodingInvalid=The encoding [{0}] is not recognised by the > JRE and will be ignored > >> digester.failedToUpdateAttributes=Attribute [{0}] failed to update > and remains [{1}] > >> +digester.failedToUpdateSystemProperty=System property [{0}] failed to > update and remains [{1}] > >> > >> Modified: tomcat/trunk/webapps/docs/changelog.xml > >> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/ > changelog.xml?rev=1803038&r1=1803037&r2=1803038&view=diff > >> ============================================================ > ================== > >> --- tomcat/trunk/webapps/docs/changelog.xml (original) > >> +++ tomcat/trunk/webapps/docs/changelog.xml Wed Jul 26 12:15:45 2017 > >> @@ -52,6 +52,15 @@ > >> decoded. Identified by FindBugs. (markt)</fix> > >> </changelog> > >> </subsection> > >> + <subsection name="Coyote"> > >> + <changelog> > >> + <update> > >> + <bug>61345</bug>: Add a server listener that can be used to do > system > >> + property replacement from the property source configured in > the > >> + digester. (remm) > >> + </update> > >> + </changelog> > >> + </subsection> > >> </section> > >> <section name="Tomcat 9.0.0.M25 (markt)" rtext="release in progress"> > >> <subsection name="Catalina"> > >> > >> Modified: tomcat/trunk/webapps/docs/config/systemprops.xml > >> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/ > config/systemprops.xml?rev=1803038&r1=1803037&r2=1803038&view=diff > >> ============================================================ > ================== > >> --- tomcat/trunk/webapps/docs/config/systemprops.xml (original) > >> +++ tomcat/trunk/webapps/docs/config/systemprops.xml Wed Jul 26 > 12:15:45 2017 > >> @@ -45,6 +45,10 @@ > >> Required to have a public constructor with no arguments.</p> > >> <p>Use this to add a property source, that will be invoked when > <code>${parameter}</code> > >> denoted parameters are found in the XML files that Tomcat > parses.</p> > >> + <p>Property replacement from the specified property source on > the JVM > >> + system properties can also be done by adding the > >> + <code>org.apache.tomcat.util.digester.Digester$ > SystemPropertyReplacementListener</code> > >> + listener as a Server listener in the container.</p> > >> </property> > >> </properties> > >> > >> > >> > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > >> For additional commands, e-mail: dev-h...@tomcat.apache.org > >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
