You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/07/30 21:40:03 UTC

[tomcat] branch 8.5.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=51497

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/8.5.x by this push:
     new 3b17b66  Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=51497
3b17b66 is described below

commit 3b17b66656c6dcae8844bd2ff18f130e0283db2b
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jul 30 22:37:56 2019 +0100

    Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=51497
    
    Add an option to use canonical IPv6 representation in the access logs.
---
 .../catalina/security/SecurityClassLoad.java       |  6 --
 .../catalina/valves/AbstractAccessLogValve.java    | 80 ++++++++++++++++------
 .../catalina/valves/ExtendedAccessLogValve.java    |  2 +-
 webapps/docs/changelog.xml                         |  5 ++
 webapps/docs/config/valve.xml                      | 10 +++
 5 files changed, 76 insertions(+), 27 deletions(-)

diff --git a/java/org/apache/catalina/security/SecurityClassLoad.java b/java/org/apache/catalina/security/SecurityClassLoad.java
index 1bb6ecd..b6e2be7 100644
--- a/java/org/apache/catalina/security/SecurityClassLoad.java
+++ b/java/org/apache/catalina/security/SecurityClassLoad.java
@@ -43,7 +43,6 @@ public final class SecurityClassLoad {
         loadServletsPackage(loader);
         loadSessionPackage(loader);
         loadUtilPackage(loader);
-        loadValvesPackage(loader);
         loadJavaxPackage(loader);
         loadConnectorPackage(loader);
         loadTomcatPackage(loader);
@@ -109,11 +108,6 @@ public final class SecurityClassLoad {
     }
 
 
-    private static final void loadValvesPackage(ClassLoader loader) throws Exception {
-        final String basePackage = "org.apache.catalina.valves.";
-        loadAnonymousInnerClasses(loader, basePackage + "AbstractAccessLogValve");
-    }
-
     private static final void loadCoyotePackage(ClassLoader loader) throws Exception {
         final String basePackage = "org.apache.coyote.";
         loader.loadClass(basePackage + "http11.Constants");
diff --git a/java/org/apache/catalina/valves/AbstractAccessLogValve.java b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
index 950dbad..9f700e3 100644
--- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
+++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
@@ -28,6 +28,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.TimeZone;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -51,6 +52,7 @@ import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.apache.tomcat.util.net.IPv6Utils;
 
 
 /**
@@ -173,6 +175,11 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
      */
     protected boolean enabled = true;
 
+     /**
+     * Use IPv6 canonical representation format as defined by RFC 5952.
+     */
+    private boolean ipv6Canonical = false;
+
     /**
      * The pattern used to format our access log lines.
      */
@@ -344,7 +351,7 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
         private final Locale cacheDefaultLocale;
         private final DateFormatCache parent;
         protected final Cache cLFCache;
-        private final HashMap<String, Cache> formatCache = new HashMap<>();
+        private final Map<String, Cache> formatCache = new HashMap<>();
 
         protected DateFormatCache(int size, Locale loc, DateFormatCache parent) {
             cacheSize = size;
@@ -489,6 +496,16 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
     }
 
 
+    public boolean getIpv6Canonical() {
+        return ipv6Canonical;
+    }
+
+
+    public void setIpv6Canonical(boolean ipv6Canonical) {
+        this.ipv6Canonical = ipv6Canonical;
+    }
+
+
     /**
      * {@inheritDoc}
      * Default is <code>false</code>.
@@ -498,6 +515,7 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
         this.requestAttributesEnabled = requestAttributesEnabled;
     }
 
+
     /**
      * {@inheritDoc}
      */
@@ -803,9 +821,9 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
      */
     protected static class LocalAddrElement implements AccessLogElement {
 
-        private static final String LOCAL_ADDR_VALUE;
+        private final String localAddrValue;
 
-        static {
+        public LocalAddrElement(boolean ipv6Canonical) {
             String init;
             try {
                 init = InetAddress.getLocalHost().getHostAddress();
@@ -813,13 +831,18 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
                 ExceptionUtils.handleThrowable(e);
                 init = "127.0.0.1";
             }
-            LOCAL_ADDR_VALUE = init;
+
+            if (ipv6Canonical) {
+                localAddrValue = IPv6Utils.canonize(init);
+            } else {
+                localAddrValue = init;
+            }
         }
 
         @Override
         public void addElement(CharArrayWriter buf, Date date, Request request,
                 Response response, long time) {
-            buf.append(LOCAL_ADDR_VALUE);
+            buf.append(localAddrValue);
         }
     }
 
@@ -830,16 +853,22 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
         @Override
         public void addElement(CharArrayWriter buf, Date date, Request request,
                 Response response, long time) {
+            String value = null;
             if (requestAttributesEnabled) {
                 Object addr = request.getAttribute(REMOTE_ADDR_ATTRIBUTE);
                 if (addr == null) {
-                    buf.append(request.getRemoteAddr());
+                    value = request.getRemoteAddr();
                 } else {
-                    buf.append(addr.toString());
+                    value = addr.toString();
                 }
             } else {
-                buf.append(request.getRemoteAddr());
+                value = request.getRemoteAddr();
             }
+
+            if (ipv6Canonical) {
+                value = IPv6Utils.canonize(value);
+            }
+            buf.append(value);
         }
     }
 
@@ -863,6 +892,10 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
             if (value == null || value.length() == 0) {
                 value = "-";
             }
+
+            if (ipv6Canonical) {
+                value = IPv6Utils.canonize(value);
+            }
             buf.append(value);
         }
     }
@@ -1046,17 +1079,22 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
             if (usesBegin) {
                 timestamp -= time;
             }
-            switch (type) {
-            case CLF:
+            /*  Implementation note: This is deliberately not implemented using
+             *  switch. If a switch is used the compiler (at least the Oracle
+             *  one) will use a synthetic class to implement the switch. The
+             *  problem is that this class needs to be pre-loaded when using a
+             *  SecurityManager and the name of that class will depend on any
+             *  anonymous inner classes and any other synthetic classes. As such
+             *  the name is not constant and keeping the pre-loading up to date
+             *  as the name changes is error prone.
+             */
+            if (type == FormatType.CLF) {
                 buf.append(localDateCache.get().getFormat(timestamp));
-                break;
-            case SEC:
+            } else if (type == FormatType.SEC) {
                 buf.append(Long.toString(timestamp / 1000));
-                break;
-            case MSEC:
+            } else if (type == FormatType.MSEC) {
                 buf.append(Long.toString(timestamp));
-                break;
-            case MSEC_FRAC:
+            } else if (type == FormatType.MSEC_FRAC) {
                 frac = timestamp % 1000;
                 if (frac < 100) {
                     if (frac < 10) {
@@ -1067,8 +1105,8 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
                     }
                 }
                 buf.append(Long.toString(frac));
-                break;
-            case SDF:
+            } else {
+                // FormatType.SDF
                 String temp = localDateCache.get().getFormat(format, locale, timestamp);
                 if (usesMsecs) {
                     frac = timestamp % 1000;
@@ -1086,7 +1124,6 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
                     temp = temp.replace(msecPattern, Long.toString(frac));
                 }
                 buf.append(temp);
-                break;
             }
         }
     }
@@ -1371,6 +1408,9 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
                 value = "-";
             }
 
+            if (ipv6Canonical) {
+                value = IPv6Utils.canonize(value);
+            }
             buf.append(value);
         }
     }
@@ -1668,7 +1708,7 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
         case 'a':
             return new RemoteAddrElement();
         case 'A':
-            return new LocalAddrElement();
+            return new LocalAddrElement(ipv6Canonical);
         case 'b':
             return new ByteSentElement(true);
         case 'B':
diff --git a/java/org/apache/catalina/valves/ExtendedAccessLogValve.java b/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
index c3b7008..43f5fa7 100644
--- a/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
+++ b/java/org/apache/catalina/valves/ExtendedAccessLogValve.java
@@ -604,7 +604,7 @@ public class ExtendedAccessLogValve extends AccessLogValve {
         } else if ("s".equals(token)) {
             String nextToken = tokenizer.getToken();
             if ("ip".equals(nextToken)) {
-                return new LocalAddrElement();
+                return new LocalAddrElement(getIpv6Canonical());
             } else if ("dns".equals(nextToken)) {
                 return new AccessLogElement() {
                     @Override
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 23e137d..12a9077 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -48,6 +48,11 @@
   <subsection name="Catalina">
     <changelog>
       <add>
+        <bug>51497</bug>: Add an option, <code>ipv6Canonical</code>, to the
+        <code>AccessLogValve</code> that causes IPv6 addresses to be output in
+        canonical form defined by RFC 5952. (ognjen/markt)
+      </add>
+      <add>
         <bug>57665</bug>: Add support for the <code>X-Forwarded-Host</code>
         header to the <code>RemoteIpFilter</code> and <code>RemotepValve</code>.
         (markt)
diff --git a/webapps/docs/config/valve.xml b/webapps/docs/config/valve.xml
index 048c77f..b8bbc42 100644
--- a/webapps/docs/config/valve.xml
+++ b/webapps/docs/config/valve.xml
@@ -181,6 +181,16 @@
         </p>
       </attribute>
 
+      <attribute name="ipv6Canonical" required="false">
+        <p>Flag to determine if IPv6 addresses should be represented in canonical
+           representation format as defined by RFC 5952. If set to <code>true</code>,
+           then IPv6 addresses will be written in canonical format (e.g.
+           <code>2001:db8::1:0:0:1</code>, <code>::1</code>), otherwise it will be
+           represented in full form (e.g. <code>2001:db8:0:0:1:0:0:1</code>,
+           <code>0:0:0:0:0:0:0:1</code>). Default value: <code>false</code>
+        </p>
+      </attribute>
+
       <attribute name="locale" required="false">
         <p>The locale used to format timestamps in the access log
            lines. Any timestamps configured using an


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org