Inspired by 60161: Allow creating subcategories from the container lo…

…gger name. The configuration remains compatible, but some possibly logging intensive components get their own playground. Use it for the rewrite valve. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1764897 13f79535-47bb-0310-9956-ffa450edef68
apache · Oct 14, 2016 · 3aff80a · 3aff80a
1 parent 9863d1f
commit 3aff80a
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 47 deletions.
diff --git a/java/org/apache/catalina/Container.java b/java/org/apache/catalina/Container.java
@@ -122,6 +122,13 @@ public interface Container extends Lifecycle {
     public Log getLogger();
 
 
+    /**
+     * Return the logger name that the container will use.
+     * @return the abbreviated name of this container for logging messages
+     */
+    public String getLogName();
+
+
     /**
      * Obtain the JMX name for this container.
      *

diff --git a/java/org/apache/catalina/core/ContainerBase.java b/java/org/apache/catalina/core/ContainerBase.java
@@ -357,12 +357,40 @@ public Log getLogger() {
 
         if (logger != null)
             return (logger);
-        logger = LogFactory.getLog(logName());
+        logger = LogFactory.getLog(getLogName());
         return (logger);
 
     }
 
 
+    /**
+     * @return the abbreviated name of this container for logging messages
+     */
+    @Override
+    public String getLogName() {
+
+        if (logName != null) {
+            return logName;
+        }
+        String loggerName = null;
+        Container current = this;
+        while (current != null) {
+            String name = current.getName();
+            if ((name == null) || (name.equals(""))) {
+                name = "/";
+            } else if (name.startsWith("##")) {
+                name = "/" + name;
+            }
+            loggerName = "[" + name + "]"
+                + ((loggerName != null) ? ("." + loggerName) : "");
+            current = current.getParent();
+        }
+        logName = ContainerBase.class.getName() + "." + loggerName;
+        return logName;
+
+    }
+
+
     /**
      * Return the Cluster with which this Container is associated.  If there is
      * no associated Cluster, return the Cluster associated with our parent
@@ -1183,33 +1211,6 @@ public void fireContainerEvent(String type, Object data) {
     }
 
 
-    /**
-     * @return the abbreviated name of this container for logging messages
-     */
-    protected String logName() {
-
-        if (logName != null) {
-            return logName;
-        }
-        String loggerName = null;
-        Container current = this;
-        while (current != null) {
-            String name = current.getName();
-            if ((name == null) || (name.equals(""))) {
-                name = "/";
-            } else if (name.startsWith("##")) {
-                name = "/" + name;
-            }
-            loggerName = "[" + name + "]"
-                + ((loggerName != null) ? ("." + loggerName) : "");
-            current = current.getParent();
-        }
-        logName = ContainerBase.class.getName() + "." + loggerName;
-        return logName;
-
-    }
-
-
     // -------------------- JMX and Registration  --------------------
 
     @Override

diff --git a/java/org/apache/catalina/startup/FailedContext.java b/java/org/apache/catalina/startup/FailedContext.java
@@ -240,6 +240,9 @@ public void setLoader(Loader loader) { /* NO-OP */ }
     @Override
     public Log getLogger() { return null; }
 
+    @Override
+    public String getLogName() { return null; }
+
     @Override
     public Manager getManager() { return null; }
     @Override

diff --git a/java/org/apache/catalina/valves/rewrite/RewriteValve.java b/java/org/apache/catalina/valves/rewrite/RewriteValve.java
@@ -47,6 +47,7 @@
 import org.apache.catalina.connector.Response;
 import org.apache.catalina.util.URLEncoder;
 import org.apache.catalina.valves.ValveBase;
+import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.buf.CharChunk;
 import org.apache.tomcat.util.buf.MessageBytes;
 import org.apache.tomcat.util.buf.UriUtil;
@@ -143,6 +144,14 @@ public void setEnabled(boolean enabled) {
         this.enabled = enabled;
     }
 
+
+    @Override
+    protected void initInternal() throws LifecycleException {
+        super.initInternal();
+        containerLog = LogFactory.getLog(getContainer().getLogName() + ".rewrite");
+    }
+
+
     @Override
     protected synchronized void startInternal() throws LifecycleException {
 
@@ -155,11 +164,11 @@ protected synchronized void startInternal() throws LifecycleException {
             context = true;
             is = ((Context) getContainer()).getServletContext()
                 .getResourceAsStream("/WEB-INF/" + resourcePath);
-            if (container.getLogger().isDebugEnabled()) {
+            if (containerLog.isDebugEnabled()) {
                 if (is == null) {
-                    container.getLogger().debug("No configuration resource found: /WEB-INF/" + resourcePath);
+                    containerLog.debug("No configuration resource found: /WEB-INF/" + resourcePath);
                 } else {
-                    container.getLogger().debug("Read configuration from: /WEB-INF/" + resourcePath);
+                    containerLog.debug("Read configuration from: /WEB-INF/" + resourcePath);
                 }
             }
         } else if (getContainer() instanceof Host) {
@@ -170,21 +179,21 @@ protected synchronized void startInternal() throws LifecycleException {
                     // Use getResource and getResourceAsStream
                     is = getClass().getClassLoader()
                         .getResourceAsStream(resourceName);
-                    if (is != null && container.getLogger().isDebugEnabled()) {
+                    if (is != null && containerLog.isDebugEnabled()) {
-                        container.getLogger().debug("Read configuration from CL at " + resourceName);
+                        containerLog.debug("Read configuration from CL at " + resourceName);
                     }
                 } else {
-                    if (container.getLogger().isDebugEnabled()) {
+                    if (containerLog.isDebugEnabled()) {
-                        container.getLogger().debug("Read configuration from " + file.getAbsolutePath());
+                        containerLog.debug("Read configuration from " + file.getAbsolutePath());
                     }
                     is = new FileInputStream(file);
                 }
-                if ((is == null) && (container.getLogger().isDebugEnabled())) {
+                if ((is == null) && (containerLog.isDebugEnabled())) {
-                    container.getLogger().debug("No configuration resource found: " + resourceName +
+                    containerLog.debug("No configuration resource found: " + resourceName +
                             " in " + getConfigBase() + " or in the classloader");
                 }
             } catch (Exception e) {
-                container.getLogger().error("Error opening configuration", e);
+                containerLog.error("Error opening configuration", e);
             }
         }
 
@@ -197,19 +206,22 @@ protected synchronized void startInternal() throws LifecycleException {
                 BufferedReader reader = new BufferedReader(isr)) {
             parse(reader);
         } catch (IOException ioe) {
-            container.getLogger().error("Error closing configuration", ioe);
+            containerLog.error("Error closing configuration", ioe);
         } finally {
             try {
                 is.close();
             } catch (IOException e) {
-                container.getLogger().error("Error closing configuration", e);
+                containerLog.error("Error closing configuration", e);
             }
         }
 
     }
 
     public void setConfiguration(String configuration)
         throws Exception {
+        if (containerLog == null) {
+            containerLog = LogFactory.getLog(getContainer().getLogName() + ".rewrite");
+        }
         maps.clear();
         parse(new BufferedReader(new StringReader(configuration)));
     }
@@ -238,8 +250,8 @@ protected void parse(BufferedReader reader) throws LifecycleException {
                 Object result = parse(line);
                 if (result instanceof RewriteRule) {
                     RewriteRule rule = (RewriteRule) result;
-                    if (container.getLogger().isDebugEnabled()) {
+                    if (containerLog.isDebugEnabled()) {
-                        container.getLogger().debug("Add rule with pattern " + rule.getPatternString()
+                        containerLog.debug("Add rule with pattern " + rule.getPatternString()
                                 + " and substitution " + rule.getSubstitutionString());
                     }
                     for (int i = (conditions.size() - 1); i > 0; i--) {
@@ -248,9 +260,9 @@ protected void parse(BufferedReader reader) throws LifecycleException {
                         }
                     }
                     for (int i = 0; i < conditions.size(); i++) {
-                        if (container.getLogger().isDebugEnabled()) {
+                        if (containerLog.isDebugEnabled()) {
                             RewriteCond cond = conditions.get(i);
-                            container.getLogger().debug("Add condition " + cond.getCondPattern()
+                            containerLog.debug("Add condition " + cond.getCondPattern()
                                     + " test " + cond.getTestString() + " to rule with pattern "
                                     + rule.getPatternString() + " and substitution "
                                     + rule.getSubstitutionString() + (cond.isOrnext() ? " [OR]" : "")
@@ -271,7 +283,7 @@ protected void parse(BufferedReader reader) throws LifecycleException {
                     }
                 }
             } catch (IOException e) {
-                container.getLogger().error("Error reading configuration", e);
+                containerLog.error("Error reading configuration", e);
             }
         }
         this.rules = rules.toArray(new RewriteRule[0]);
@@ -338,8 +350,8 @@ public void invoke(Request request, Response response)
                 CharSequence test = (rule.isHost()) ? host : urlDecoded;
                 CharSequence newtest = rule.evaluate(test, resolver);
                 if (newtest != null && !test.equals(newtest.toString())) {
-                    if (container.getLogger().isDebugEnabled()) {
+                    if (containerLog.isDebugEnabled()) {
-                        container.getLogger().debug("Rewrote " + test + " as " + newtest
+                        containerLog.debug("Rewrote " + test + " as " + newtest
                                 + " with rule pattern " + rule.getPatternString());
                     }
                     if (rule.isHost()) {

diff --git a/test/org/apache/tomcat/unittest/TesterContext.java b/test/org/apache/tomcat/unittest/TesterContext.java
@@ -115,6 +115,11 @@ public Log getLogger() {
         return log;
     }
 
+    @Override
+    public String getLogName() {
+        return null;
+    }
+
     @Override
     public ObjectName getObjectName() {
         return null;

diff --git a/test/org/apache/tomcat/unittest/TesterHost.java b/test/org/apache/tomcat/unittest/TesterHost.java
@@ -44,6 +44,11 @@ public Log getLogger() {
         return null;
     }
 
+    @Override
+    public String getLogName() {
+        return null;
+    }
+
     @Override
     public ObjectName getObjectName() {
         return null;

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -72,6 +72,10 @@
         When calling <code>getResourceAsStream()</code> on a directory, ensure
         that <code>null</code> is returned. (markt)
       </fix>
+      <fix>
+        <bug>60161</bug>: Allow creating subcategories of the container logger,
+        and use it for the rewrite valve. (remm)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">