This is an automated email from the ASF dual-hosted git repository.
juanpablo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jspwiki.git
commit 183b7a83db39000731f63e6f95d6bbfe004a2b82
Author: Juan Pablo Santos RodrÃguez <juanpablo.san...@gmail.com>
AuthorDate: Sat Apr 27 15:45:37 2024 +0200
Extract system property expansion to its own method
* this ensures property load order remains as before, so we don't
unexpectedly change resolved properties for anyone
* more importantly, we also check for system env variables if the system
property isn't defined. This allows us to check for things like i.e.
* added javadocs here and there and some formatting changes along the way,
and a new assertion on the test
---
.../java/org/apache/wiki/util/PropertyReader.java | 140 ++++++++++++---------
.../org/apache/wiki/util/PropertyReaderTest.java | 56 +++++----
2 files changed, 110 insertions(+), 86 deletions(-)
diff --git
a/jspwiki-util/src/main/java/org/apache/wiki/util/PropertyReader.java
b/jspwiki-util/src/main/java/org/apache/wiki/util/PropertyReader.java
index 03bf7cfe5..0c6ae65ca 100644
--- a/jspwiki-util/src/main/java/org/apache/wiki/util/PropertyReader.java
+++ b/jspwiki-util/src/main/java/org/apache/wiki/util/PropertyReader.java
@@ -41,12 +41,12 @@ import java.nio.file.Files;
* @since 2.5.x
*/
public final class PropertyReader {
-
- private static final Logger LOG = LogManager.getLogger(
PropertyReader.class );
-
+
+ private static final Logger LOG = LogManager.getLogger(
PropertyReader.class );
+
/**
- * Path to the base property file, usually overridden by values provided in
- * a jspwiki-custom.properties file {@value #DEFAULT_JSPWIKI_CONFIG}
+ * Path to the base property file, {@value}, usually overridden by values
provided in
+ * a jspwiki-custom.properties file.
*/
public static final String DEFAULT_JSPWIKI_CONFIG =
"/ini/jspwiki.properties";
@@ -58,8 +58,8 @@ public final class PropertyReader {
public static final String PARAM_CUSTOMCONFIG = "jspwiki.custom.config";
/**
- * The prefix when you are cascading properties.
- *
+ * The prefix when you are cascading properties.
+ *
* @see #loadWebAppProps(ServletContext)
*/
public static final String PARAM_CUSTOMCONFIG_CASCADEPREFIX =
"jspwiki.custom.cascade.";
@@ -114,7 +114,7 @@ public final class PropertyReader {
* and so on. You have to number your cascade in a descending way starting
* with "1". This means you cannot leave out numbers in your cascade. This
* method is based on an idea by Olaf Kaus, see [JSPWiki:MultipleWikis].
- *
+ *
* @param context A Servlet Context which is used to find the properties
* @return A filled Properties object with all the cascaded properties in
place
*/
@@ -136,6 +136,15 @@ public final class PropertyReader {
// this will add additional properties to the default ones:
LOG.debug( "Loading cascading properties..." );
+ // now load the cascade (new in 2.5)
+ loadWebAppPropsCascade( context, props );
+
+ // property expansion so we can resolve things like ${TOMCAT_HOME}
+ propertyExpansion( props );
+
+ // sets the JSPWiki working directory (jspwiki.workDir)
+ setWorkDir( context, props );
+
// add system properties beginning with jspwiki...
final Map< String, String > sysprops = collectPropertiesFrom(
System.getProperties().entrySet().stream()
.collect( Collectors.toMap( Object::toString, Object::toString ) ) );
@@ -143,15 +152,6 @@ public final class PropertyReader {
// finally, expand the variables (new in 2.5)
expandVars( props );
-
-
- // now load the cascade (new in 2.5)
- loadWebAppPropsCascade( context, props );
-
- // sets the JSPWiki working directory (jspwiki.workDir)
- setWorkDir( context, props );
-
-
return props;
} catch( final Exception e ) {
@@ -170,24 +170,24 @@ public final class PropertyReader {
/**
* Figure out where our properties lie.
- *
+ *
* @param context servlet context
* @param propertyFile property file
- * @return inputstream holding the properties file
+ * @return InputStream holding the properties file
* @throws FileNotFoundException properties file not found
*/
- static InputStream loadCustomPropertiesFile( final ServletContext
context, final String propertyFile ) throws IOException {
+ static InputStream loadCustomPropertiesFile( final ServletContext context,
final String propertyFile ) throws IOException {
final InputStream propertyStream;
- if( propertyFile == null ) {
- LOG.debug( "No " + PARAM_CUSTOMCONFIG + " defined for this
context, looking for custom properties file with default name of: " +
CUSTOM_JSPWIKI_CONFIG );
- // Use the custom property file at the default location
- propertyStream = locateClassPathResource(context,
CUSTOM_JSPWIKI_CONFIG);
- } else {
- LOG.debug( PARAM_CUSTOMCONFIG + " defined, using " +
propertyFile + " as the custom properties file." );
+ if( propertyFile == null ) {
+ LOG.debug( "No " + PARAM_CUSTOMCONFIG + " defined for this
context, looking for custom properties file with default name of: " +
CUSTOM_JSPWIKI_CONFIG );
+ // Use the custom property file at the default location
+ propertyStream = locateClassPathResource(context,
CUSTOM_JSPWIKI_CONFIG);
+ } else {
+ LOG.debug( PARAM_CUSTOMCONFIG + " defined, using " + propertyFile
+ " as the custom properties file." );
propertyStream = Files.newInputStream( new
File(propertyFile).toPath() );
- }
- return propertyStream;
- }
+ }
+ return propertyStream;
+ }
/**
@@ -202,9 +202,9 @@ public final class PropertyReader {
props.load( in );
}
} catch( final IOException e ) {
- LOG.error( "Unable to load default propertyfile '" +
DEFAULT_JSPWIKI_CONFIG + "'" + e.getMessage(), e );
+ LOG.error( "Unable to load default propertyfile '{}' {}",
DEFAULT_JSPWIKI_CONFIG, e.getMessage(), e );
}
-
+
return props;
}
@@ -236,8 +236,7 @@ public final class PropertyReader {
*/
private static String getInitParameter( final ServletContext context,
final String name ) {
final String value = context.getInitParameter( name );
- return value != null ? value
- : System.getProperty( name ) ;
+ return value != null ? value : System.getProperty( name ) ;
}
@@ -264,28 +263,65 @@ public final class PropertyReader {
}
try( final InputStream propertyStream =
Files.newInputStream(Paths.get(( propertyFile ) ))) {
- LOG.info( " Reading additional properties from " +
propertyFile + " and merge to cascade." );
+ LOG.info( " Reading additional properties from {} and merge to
cascade.", propertyFile );
final Properties additionalProps = new Properties();
additionalProps.load( propertyStream );
defaultProperties.putAll( additionalProps );
} catch( final Exception e ) {
- LOG.error( "JSPWiki: Unable to load and setup properties from
" + propertyFile + "." + e.getMessage() );
+ LOG.error( "JSPWiki: Unable to load and setup properties from
{}. {}", propertyFile, e.getMessage() );
}
}
}
/**
- * You define a property variable by using the prefix "var.x" as a
property. In property values you can then use the "$x" identifier
- * to use this variable.
+ * <p>Try to resolve properties whose value is something like {@code
${SOME_VALUE}} from a system property first and,
+ * if not found, from a system environment variable. If not found on
neither, the property value will remain as
+ * {@code ${SOME_VALUE}}, and no more expansions will be processed.</p>
*
- * For example, you could declare a base directory for all your files
like this and use it in all your other property definitions with
- * a "$basedir". Note that it does not matter if you define the variable
before its usage.
+ * <p>Several expansions per property is OK, but no we're not supporting
fancy things like recursion. Reference to
+ * other properties is achieved through {@link #expandVars(Properties)}.
More than one property expansion per entry
+ * is allowed.</p>
+ *
+ * @param properties properties to expand;
+ */
+ public static void propertyExpansion( final Properties properties ) {
+ final Enumeration< ? > propertyList = properties.propertyNames();
+ while( propertyList.hasMoreElements() ) {
+ final String propertyName = ( String )propertyList.nextElement();
+ String propertyValue = properties.getProperty( propertyName );
+ while( propertyValue.contains( "${" ) && propertyValue.contains(
"}" ) ) {
+ final int start = propertyValue.indexOf( "${" );
+ final int end = propertyValue.indexOf( "}", start );
+ if( start >= 0 && end >= 0 && end > start ) {
+ final String substring = propertyValue.substring( start,
end ).replace( "${", "" ).replace( "}", "" );
+ final String expansion = Objects.toString(
System.getProperty( substring ), System.getenv( substring ) );
+ if( expansion != null ) {
+ propertyValue = propertyValue.replace( "${" +
substring + "}", expansion );
+ properties.setProperty( propertyName, propertyValue );
+ } else {
+ LOG.warn( "{} referenced on {} ({}) but not found on
System props or env", substring, propertyName, propertyValue );
+ break;
+ }
+ } else {
+ // no more matches or value like foo}${bar
+ break;
+ }
+ }
+ }
+ }
+
+ /**
+ * <p>You define a property variable by using the prefix {@code var.x} as
a property. In property values you can then use the "$x" identifier
+ * to use this variable.</p>
+ *
+ * <p>For example, you could declare a base directory for all your files
like this and use it in all your other property definitions with
+ * a {@code $basedir}. Note that it does not matter if you define the
variable before its usage.
* <pre>
- * var.basedir = /p/mywiki;
+ * var.basedir = /p/mywiki; # var.basedir = ${TOMCAT_HOME} would also be
fine
* jspwiki.fileSystemProvider.pageDir = $basedir/www/
* jspwiki.basicAttachmentProvider.storageDir = $basedir/www/
* jspwiki.workDir = $basedir/wrk/
- * </pre>
+ * </pre></p>
*
* @param properties - properties to expand;
*/
@@ -295,33 +331,13 @@ public final class PropertyReader {
Enumeration< ? > propertyList = properties.propertyNames();
while( propertyList.hasMoreElements() ) {
final String propertyName = ( String )propertyList.nextElement();
- String propertyValue = properties.getProperty( propertyName );
+ final String propertyValue = properties.getProperty( propertyName
);
if ( propertyName.startsWith( PARAM_VAR_DECLARATION ) ) {
final String varName = propertyName.substring( 4 ).trim();
final String varValue = propertyValue.trim();
vars.put( varName, varValue );
}
- boolean subsitution=false;
- while ( propertyValue.contains( "${" ) && propertyValue.contains(
"}" ) ) {
- int start = propertyValue.indexOf( "${" );
- int end = propertyValue.indexOf( "}", start );
- if ( start == -1 || end == -1 ) break;
- if ( end > start ) {
- String substring = propertyValue.substring( start, end
).replace( "${", "" ).replace( "}", "" );
- if (System.getProperty( substring ) != null ) {
- propertyValue = propertyValue.replace( "${" +
substring + "}", System.getProperty( substring ) );
- subsitution = true;
- } else break;
- } else {
- //this would be a strange issue of a value like
- // foo}${bar
- break;
- }
- }
- if ( subsitution ) {
- properties.setProperty( propertyName, propertyValue );
- }
}
//now, substitute $ values in property values with vars...
diff --git
a/jspwiki-util/src/test/java/org/apache/wiki/util/PropertyReaderTest.java
b/jspwiki-util/src/test/java/org/apache/wiki/util/PropertyReaderTest.java
index c20e1feae..e45c908e0 100644
--- a/jspwiki-util/src/test/java/org/apache/wiki/util/PropertyReaderTest.java
+++ b/jspwiki-util/src/test/java/org/apache/wiki/util/PropertyReaderTest.java
@@ -36,10 +36,10 @@ import static org.mockito.Mockito.mock;
/**
* Unit test for PropertyReader.
*/
-public class PropertyReaderTest {
+class PropertyReaderTest {
@Test
- public void testLocateClassPathResource() {
+ void testLocateClassPathResource() {
Assertions.assertEquals( "/ini/jspwiki.properties",
PropertyReader.createResourceLocation( "ini", "jspwiki.properties" ) );
Assertions.assertEquals( "/ini/jspwiki.properties",
PropertyReader.createResourceLocation( null, "ini/jspwiki.properties" ) );
Assertions.assertEquals( "/ini/jspwiki.properties",
PropertyReader.createResourceLocation( null, "/ini/jspwiki.properties" ) );
@@ -51,7 +51,7 @@ public class PropertyReaderTest {
}
@Test
- public void testVariableExpansion() {
+ void testVariableExpansion() {
final Properties p = new Properties();
p.put( "var.basedir", "/p/mywiki" );
p.put( "jspwiki.fileSystemProvider.pageDir", "$basedir/www/" );
@@ -68,26 +68,9 @@ public class PropertyReaderTest {
Assertions.assertTrue( p.getProperty( "jspwiki.xyz" ).endsWith( "test
basedir" ) ); //don't touch this
Assertions.assertFalse( p.getProperty( "jspwiki.workDir" ).endsWith(
"$basedir/wrk/" ) );
}
- @Test
- public void testSystemPropertyInjection() {
- System.setProperty("FOO", "BAR");
- System.setProperty("TEST", "VAL");
- final Properties p = new Properties();
- p.put( "jspwiki.fileSystemProvider.pageDir", "${FOO}/www/" );
- p.put( "jspwiki.fileSystemProvider.workDir", "${FOO}/www/${TEST}" );
- p.put( "jspwiki.fileSystemProvider.badVal1", "${FOO/www/${TEST}" );
- p.put( "jspwiki.fileSystemProvider.badVal2", "}${FOO/www/${TEST}" );
- p.put( "jspwiki.fileSystemProvider.badVal3", "${NONEXISTANTPROP}" );
- PropertyReader.expandVars( p );
- Assertions.assertEquals( "BAR/www/", p.getProperty(
"jspwiki.fileSystemProvider.pageDir" ) );
- Assertions.assertEquals( "BAR/www/VAL", p.getProperty(
"jspwiki.fileSystemProvider.workDir" ) );
- Assertions.assertEquals( "${FOO/www/${TEST}", p.getProperty(
"jspwiki.fileSystemProvider.badVal1" ) );
- Assertions.assertEquals( "}${FOO/www/${TEST}", p.getProperty(
"jspwiki.fileSystemProvider.badVal2" ) );
- Assertions.assertEquals( "${NONEXISTANTPROP}", p.getProperty(
"jspwiki.fileSystemProvider.badVal3" ) );
- }
@Test
- public void testVariableExpansion2() {
+ void testVariableExpansion2() {
final Properties p = new Properties();
//this time, declare the var at the end... (should overwrite this one);
@@ -114,7 +97,7 @@ public class PropertyReaderTest {
}
@Test
- public void testMultipleVariableExpansion() {
+ void testMultipleVariableExpansion() {
final Properties p = new Properties();
//this time, declare the var at the end... (should overwrite this one);
@@ -133,7 +116,7 @@ public class PropertyReaderTest {
}
@Test
- public void testCollectPropertiesFrom() {
+ void testCollectPropertiesFrom() {
final Map< String, String > sut = new HashMap<>();
sut.put( "jspwiki_frontPage", "Main" );
sut.put( "secretEnv", "asdasd" );
@@ -145,7 +128,7 @@ public class PropertyReaderTest {
}
@Test
- public void testSetWorkDir() {
+ void testSetWorkDir() {
final Properties properties = new Properties();
final ServletContext servletContext = mock(ServletContext.class);
final File tmp = new File( "/tmp" );
@@ -171,4 +154,29 @@ public class PropertyReaderTest {
Assertions.assertEquals( System.getProperty( "java.io.tmpdir" ),
workDir );
}
+ @Test
+ void testSystemPropertyExpansion() {
+ try {
+ System.setProperty( "FOO", "BAR" );
+ System.setProperty( "TEST", "VAL" );
+ final Properties p = new Properties();
+ p.put( "jspwiki.fileSystemProvider.pageDir", "${FOO}/www/" );
+ p.put( "jspwiki.fileSystemProvider.workDir", "${FOO}/www/${TEST}"
);
+ p.put( "jspwiki.fileSystemProvider.badVal1", "${FOO/www/${TEST}" );
+ p.put( "jspwiki.fileSystemProvider.badVal2", "}${FOO/www/${TEST}"
);
+ p.put( "jspwiki.fileSystemProvider.badVal3", "${NONEXISTANTPROP}"
);
+ p.put( "jspwiki.fileSystemProvider.badVal4",
"${NONEXISTANTPROP}/${FOO}" );
+ PropertyReader.propertyExpansion( p );
+ Assertions.assertEquals( "BAR/www/", p.getProperty(
"jspwiki.fileSystemProvider.pageDir" ) );
+ Assertions.assertEquals( "BAR/www/VAL", p.getProperty(
"jspwiki.fileSystemProvider.workDir" ) );
+ Assertions.assertEquals( "${FOO/www/${TEST}", p.getProperty(
"jspwiki.fileSystemProvider.badVal1" ) );
+ Assertions.assertEquals( "}${FOO/www/${TEST}", p.getProperty(
"jspwiki.fileSystemProvider.badVal2" ) );
+ Assertions.assertEquals( "${NONEXISTANTPROP}", p.getProperty(
"jspwiki.fileSystemProvider.badVal3" ) );
+ Assertions.assertEquals( "${NONEXISTANTPROP}/${FOO}",
p.getProperty( "jspwiki.fileSystemProvider.badVal4" ) );
+ } finally {
+ System.setProperty( "FOO", "" );
+ System.setProperty( "TEST", "" );
+ }
+ }
+
}
