Repository navigation
Issue #839 JavaLogFactory ConcMod - #840
kwwall merged 2 commits into
Conversation
Sorry, something went wrong.
Removing support for esapi-java-logging.properties file from baseline. ConfigurationException is thrown if file is found on the path at runtime. Exception message links to a wiki page with instructions on how to configure the application instance.
|
@jeremiahjstacey - Do you think it's useful to mention the previous defaults that were in the (now removed -- after this PR is merged) esapi-java-logging.properties file that used to be in the ESAPI configuration jar? I thought we may get questions about what properties used to be there for JUL and thought we could drop those into the new wiki page (https://github.com/ESAPI/esapi-java-legacy/wiki/Configuring-the-JavaLogFactory) that you wrote. There's only these 5: handlers= java.util.logging.ConsoleHandler .level= INFO java.util.logging.ConsoleHandler.level = INFO java.util.logging.ConsoleHandler.formatter = java.util.logging.SimpleFormatter java.util.logging.SimpleFormatter.format=[%1$tF %1$tT] [%3$-7s] %5$s %n I can add it if you think it would be useful. I just don't want that to come up as a question and the JUL LogManager Javadoc obviously isn't going to answer that. Also, when setting java.util.logging.config.file, does it take an actual file path or a URI? Sometimes the allow URIs so one can fetch it remotely. I've not looked at their code, but just because it says '.file' doesn't give me high confidence that's how it's implemented. The LogManager Javadoc is a bit skimpy in that regard. E.g., I was wondering if one needed to set it something like: -Djava.util.logging.config.file=file:///path/to/configuration.properties |
Sorry, something went wrong.
kwwall
left a comment
There was a problem hiding this comment.
@jeremiahjstacey - Some minor suggestions is all, but I'm okay with this PR as it is.
@xeno6696 - I may try doing a release over Memorial Day weekend (no promise though), so if I will give you until Sat around noon if you want to review this. Ideally, just leave an 'approved' as early as possible so I know you're good with it.
Sorry, something went wrong.
| 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."); |
There was a problem hiding this comment.
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.
Sorry, something went wrong.
There was a problem hiding this comment.
Update applied
Sorry, something went wrong.
Removing unused imports. Consolidating String duplication to a class constant.
Removing support for esapi-java-logging.properties file from baseline.
ConfigurationException is thrown if file is found on the path at runtime. Exception message links to a wiki page with instructions on how to configure the application instance.
Closes #839
Also note the wiki page has been created.
https://github.com/ESAPI/esapi-java-legacy/wiki/Configuring-the-JavaLogFactory