Skip to content

Issue #839 JavaLogFactory ConcMod #840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 27, 2024
Merged

Issue #839 JavaLogFactory ConcMod #840

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6f7c45
Issue #839 JavaLogFactory ConcMod
jeremiahjstacey Apr 26, 2024
ae8a7c3
JavaLogFactory Cleanup
jeremiahjstacey May 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions configuration/esapi/esapi-java-logging.properties
View file
Open in desktop

This file was deleted.

59 changes: 20 additions & 39 deletions src/main/java/org/owasp/esapi/logging/java/JavaLogFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,16 @@
*/
package org.owasp.esapi.logging.java;

import static org.owasp.esapi.PropNames.APPLICATION_NAME;
import static org.owasp.esapi.PropNames.LOG_APPLICATION_NAME;
import static org.owasp.esapi.PropNames.LOG_CLIENT_INFO;
import static org.owasp.esapi.PropNames.LOG_ENCODING_REQUIRED;
import static org.owasp.esapi.PropNames.LOG_SERVER_IP;
import static org.owasp.esapi.PropNames.LOG_USER_INFO;

import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
Expand All @@ -25,7 +33,6 @@
import org.owasp.esapi.ESAPI;
import org.owasp.esapi.LogFactory;
import org.owasp.esapi.Logger;
import org.owasp.esapi.PropNames;
import org.owasp.esapi.codecs.HTMLEntityCodec;
import org.owasp.esapi.errors.ConfigurationException;
import org.owasp.esapi.logging.appender.LogAppender;
Expand All @@ -35,13 +42,6 @@
import org.owasp.esapi.logging.cleaning.LogScrubber;
import org.owasp.esapi.logging.cleaning.NewlineLogScrubber;

import static org.owasp.esapi.PropNames.LOG_ENCODING_REQUIRED;
import static org.owasp.esapi.PropNames.LOG_USER_INFO;
import static org.owasp.esapi.PropNames.LOG_CLIENT_INFO;
import static org.owasp.esapi.PropNames.LOG_APPLICATION_NAME;
import static org.owasp.esapi.PropNames.APPLICATION_NAME;
import static org.owasp.esapi.PropNames.LOG_SERVER_IP;

/**
* LogFactory implementation which creates JAVA supporting Loggers.
* <br><br>
Expand Down Expand Up @@ -93,43 +93,24 @@ public class JavaLogFactory implements LogFactory {

LOG_BRIDGE = new JavaLogBridgeImpl(JAVA_LOG_APPENDER, JAVA_LOG_SCRUBBER, levelLookup);

readLoggerConfiguration(LogManager.getLogManager());
}

/**
* Attempts to load the expected property file path into the provided LogManager reference.
* @param logManager LogManager which is being configured.
*/
/*package*/ static void readLoggerConfiguration(LogManager logManager) {
if (System.getProperties().keySet().stream().anyMatch(propKey ->
"java.util.logging.config.class".equals(propKey) || "java.util.logging.config.file".equals(propKey))) {
// LogManager has external configuration. Do not load ESAPI defaults.
// See javadoc for the LogManager class for more information on properties.
boolean isStartupSysoutDisabled = Boolean.valueOf(System.getProperty(PropNames.DISCARD_LOGSPECIAL, Boolean.FALSE.toString()));
if (!isStartupSysoutDisabled) {
String logManagerPreferredMsg = String.format("[ESAPI-STARTUP] ESAPI JavaLogFactory Configuration will not be applied. "
+ "java.util.LogManager configuration Detected. "
+ "{\"java.util.logging.config.class\":\"%s\",\"java.util.logging.config.file\":\"%s\"}",
System.getProperty("java.util.logging.config.class"), System.getProperty("java.util.logging.config.file"));

System.out.println(logManagerPreferredMsg);
// ::SAMPLE OUTPUT::
//[ESAPI-STARTUP] ESAPI JavaLogFactory Configuration will not be applied. java.util.LogManager configuration Detected.{"java.util.logging.config.class":"some.defined.value","java.util.logging.config.file":"null"}
}

return;
}
/*
* This will load the logging properties file to control the format of the output for Java logs.
* esapi-java-logging.properties file may lead to confusing logging behavior
* by overriding desired configurations provided through Java's LogManager class.
*
* Verify the file is not present and fail if found to enforce understanding of
* the configuration method.
*/
try (InputStream stream = JavaLogFactory.class.getClassLoader().
getResourceAsStream("esapi-java-logging.properties")) {
if (stream == null) {
throw new ConfigurationException("Unable to locate resource: esapi-java-logging.properties");
if (stream != null) {
throw new ConfigurationException("esapi-java-logging.properties is no longer supported. See https://github.com/ESAPI/esapi-java-legacy/wiki/Configuring-the-JavaLogFactory for information on corrective actions.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make more sense to define this exception message as final String and the reference it both places. That way, if we every decide to change the error message, we won't forget to change one of the instances. Otherwise, I think this is great and hopefully it will head off questions from those who seem to have a habit of never reading the release notes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update applied

}
logManager.readConfiguration(stream);

} catch (IOException ioe) {
throw new ConfigurationException("Failed to load esapi-java-logging.properties.", ioe);
// This is a little strange, I know.
// If the IOException is thrown, then the file actually exists but is malformatted or has some other issue.
// The file should not exist at all, so use the same message as above but include the original exception in the log as well.
throw new ConfigurationException("esapi-java-logging.properties is no longer supported. See https://github.com/ESAPI/esapi-java-legacy/wiki/Configuring-the-JavaLogFactory for information on corrective actions.", ioe);
}
}

Expand Down
82 changes: 0 additions & 82 deletions src/test/java/org/owasp/esapi/logging/java/JavaLogFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,8 @@
*/
package org.owasp.esapi.logging.java;

import java.io.IOException;
import java.io.InputStream;
import java.util.List;
import java.util.logging.LogManager;

import org.hamcrest.CustomMatcher;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -28,7 +24,6 @@
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.owasp.esapi.Logger;
import org.owasp.esapi.errors.ConfigurationException;
import org.owasp.esapi.logging.appender.LogAppender;
import org.owasp.esapi.logging.appender.LogPrefixAppender;
import org.owasp.esapi.logging.cleaning.CodecLogScrubber;
Expand All @@ -48,83 +43,6 @@ public class JavaLogFactoryTest {
@Rule
public ExpectedException exEx = ExpectedException.none();

@Test
public void testLogManagerConfigurationAsClass() throws Exception {
String propKey = "java.util.logging.config.class";
//If defined, grab the value; otherwise, set to a known value to allow for prop to be cleared.
String sysDefault = System.getProperties().stringPropertyNames().contains(propKey) ? System.getProperty(propKey) : testName.getMethodName();

System.setProperty(propKey, "some.defined.value");
LogManager testLogManager = new LogManager() {
@Override
public void readConfiguration(InputStream ins) throws IOException, SecurityException {
throw new IOException(testName.getMethodName());
}
};

try {
// This would throw an IOException if the LogManager was not being respected since no esapi-java-logging file is specified
JavaLogFactory.readLoggerConfiguration(testLogManager);
} finally {
//Restore original prop values
if (testName.getMethodName().equals(sysDefault))
System.clearProperty(propKey);
else {
System.setProperty(propKey, sysDefault);
}
}
}

@Test
public void testLogManagerConfigurationAsFile() throws Exception {
String propKey = "java.util.logging.config.file";
//If defined, grab the value; otherwise, set to a known value to allow for prop to be cleared.
String sysDefault = System.getProperties().stringPropertyNames().contains(propKey) ? System.getProperty(propKey) : testName.getMethodName();

System.setProperty(propKey, "some.defined.value");
LogManager testLogManager = new LogManager() {
@Override
public void readConfiguration(InputStream ins) throws IOException, SecurityException {
throw new IOException(testName.getMethodName());
}
};

try {
// This would throw an IOException if the LogManager was not being respected since no esapi-java-logging file is specified
JavaLogFactory.readLoggerConfiguration(testLogManager);
} finally {
//Restore original prop values
if (testName.getMethodName().equals(sysDefault)) {
System.clearProperty(propKey);
} else {
System.setProperty(propKey, sysDefault);
}
}
}
@Test
public void testConfigurationExceptionOnMissingConfiguration() throws Exception {
final IOException originException = new IOException(testName.getMethodName());

LogManager testLogManager = new LogManager() {
@Override
public void readConfiguration(InputStream ins) throws IOException, SecurityException {
throw originException;
}
};

exEx.expectMessage("Failed to load esapi-java-logging.properties");
exEx.expect(ConfigurationException.class);

exEx.expectCause(new CustomMatcher<Throwable>("Check for IOException") {
@Override
public boolean matches(Object item) {
return item instanceof IOException;
}
});

JavaLogFactory.readLoggerConfiguration(testLogManager);
}

@Test
public void testCreateLoggerByString() {
Logger logger = new JavaLogFactory().getLogger("test");
Expand Down
6 changes: 0 additions & 6 deletions src/test/resources/esapi-java-logging.properties
View file
Open in desktop

This file was deleted.