Bug 60012 - Several log refactoring/improvement suggestions
Summary: Several log refactoring/improvement suggestions
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.5.x-trunk
Hardware: PC All
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-18 01:00 UTC by Nemo Chen
Modified: 2016-08-19 07:52 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nemo Chen 2016-08-18 01:00:23 UTC
Method Invocation Replaced By Variable:

in file: apache-tomcat-8.5.4-src/java/org/apache/catalina/authenticator/AuthenticatorBase.java
line 441 - 448
principal = session.getPrincipal();
if (principal != null) {
    if (log.isDebugEnabled()) {
        log.debug("We have cached auth type " + session.getAuthType() +
                " for principal " + session.getPrincipal());
    }
    request.setAuthType(session.getAuthType());
    request.setUserPrincipal(principal);
}

"session.getPrincipal()" could be replaced by variable "principal"

in file: apache-tomcat-8.5.4-src/java/org/apache/catalina/realm/JNDIRealm.java
line 2693- 2696

String absoluteName = result.getName();
if (containerLog.isTraceEnabled())
   containerLog.trace("  search returned absolute name: " +
           result.getName());

"result.getName()" can be replaced with "absoluteName"

*Printing null in logs*
apache-tomcat-8.5.4-src/java/org/apache/catalina/storeconfig/StoreLoader.java

line 252 - 254
if ((is == null) || (error != null)) {
    log.error(error);
}

apache-tomcat-8.5.4-src/java/org/apache/catalina/startup/CatalinaProperties.java
line 110 -115
if ((is == null) || (error != null)) {
    // Do something
    log.warn("Failed to load catalina.properties", error);
    // That's fine - we have reasonable defaults.
    properties = new Properties();
}

in the above two scenario, there is a change "error == null", there should be a null check inside the logs.
Comment 1 Violeta Georgieva 2016-08-19 07:52:17 UTC
Hi,

Thanks for the report.
This has been fixed in
- 9.0.x for 9.0.0.M10 onwards and
- 8.5.x for 8.5.5 onwards

Regards,
Violeta