Preserve Log4j 1 lookups when converting a properties configuration

[kalayciburak]

Log4j1ConfigurationConverter shares Log4j1ConfigurationParser with the runtime factory. getProperty calls OptionConverter.substVars, which resolves each ${...} against the current JVM before the value is written. A converted file therefore captures host system properties (user.home, paths, anything set with -D) and loses the indirection.

The converter now rewrites every ${name} to ${sys:name} instead of resolving it. Log4j 1 reads system properties before the file, and ${sys:name} falls back to a configuration <Property> when -Dname is not set.

• file-defined names are still emitted as <Property>
• references, including nested names inside a property value, become ${sys:name}

Log4j1ConfigurationFactory still uses the default parser and resolves variables.

Fixes #4348

Checklist

• Base your changes on 2.x branch if you are targeting Log4j 2; use main otherwise
• Non-trivial changes contain an entry file in the src/changelog/.2.x.x directory
• Tests are provided

./mvnw verify was not run.

Tests

Executed with JAVA_HOME set to Java 17:

• ./mvnw -pl log4j-1.2-api -am -Dtest=Log4j1ConfigurationConverterLookupTest -Dsurefire.failIfNoSpecifiedTests=false test - 4 tests, 0 failures (includes the threshold lookup)
• ./mvnw -pl log4j-1.2-api spotless:check - clean

[ramanathan1504]

The converter now writes ${sys:user.home} instead of the home directory of the machine that ran it, and the new test fails on 2.x without the change.
Can translateLookups in Log4j1ConfigurationParser turn every ${name} into ${sys:name}? Log4j 1 reads system properties before the file, and ${sys:app.dir} falls back to the <Property> when -Dapp.dir is not set, so isDefinedProperty can go. The log4j.threshold branch for a value with ${ also needs a test in Log4j1ConfigurationConverterLookupTest.

[ramanathan1504]

Log4j 1 checks system properties before the file. ${sys:app.dir} does the same and falls back to the <Property>, while ${app.dir} ignores -Dapp.dir.

Suggested change

	if (isDefinedProperty(key)) {
	translated.append("${").append(key).append('}');
	} else {
	translated.append("${sys:").append(key).append('}');
	}
	translated.append("${sys:").append(key).append('}');

[ramanathan1504]

With every name mapped to ${sys:...}, this method has no caller.

Suggested change

	private boolean isDefinedProperty(final String key) {
	return !key.startsWith("log4j.")
	&& !key.equals(ROOTCATEGORY)
	&& !key.equals(ROOTLOGGER)
	&& properties.getProperty(key) != null;
	}

[ramanathan1504]

This assert fails after the two changes above. The file property now reads through ${sys:app.dir}.

Suggested change

	assertTrue(xml.contains("${app.dir}/app.log"), xml);
	assertTrue(xml.contains("${sys:app.dir}/app.log"), xml);

[ramanathan1504]

Nothing covers the threshold branch for a value with ${. This test fails if that branch is removed.

Suggested change

	}
	}
	@Test
	public void thresholdLookupIsKept() throws Exception {
	final String xml = convert("log4j.rootLogger=INFO, FILE\n"
	+ "log4j.threshold=${lvl}\n"
	+ "log4j.appender.FILE=org.apache.log4j.FileAppender\n"
	+ "log4j.appender.FILE.File=app.log\n");
	assertTrue(xml.contains("level=\"${sys:lvl}\""), xml);
	}

[kalayciburak]

yes, every name is ${sys:name} now. dropped isDefinedProperty and added the threshold test.