Bug 60012

Summary: Several log refactoring/improvement suggestions
Product: Tomcat 8 Reporter: Nemo Chen <chenfsd>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 8.5.x-trunk   
Target Milestone: ----   
Hardware: PC   
OS: All   

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