Merge pull request #3095 from mwoodiupui/3094

Improve IPv6 support

dspace-api/src/main/java/org/dspace/statistics/SolrLoggerServiceImpl.java

@@ -32,6 +32,7 @@ import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import javax.servlet.http.HttpServletRequest;
@@ -95,7 +96,6 @@ import org.dspace.eperson.Group;
 import org.dspace.service.ClientInfoService;
 import org.dspace.services.ConfigurationService;
 import org.dspace.statistics.service.SolrLoggerService;
-import org.dspace.statistics.util.DnsLookup;
 import org.dspace.statistics.util.LocationUtils;
 import org.dspace.statistics.util.SpiderDetector;
 import org.dspace.usage.UsageWorkflowEvent;
@@ -244,8 +244,9 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
         } catch (RuntimeException re) {
             throw re;
         } catch (Exception e) {
+            String email = null == currentUser ? "[anonymous]" : currentUser.getEmail();
             log.error("Error saving VIEW event to Solr for DSpaceObject {} by EPerson {}",
-                      dspaceObject.getID(), currentUser.getEmail(), e);
+                      dspaceObject.getID(), email, e);
         }
     }
 
@@ -326,14 +327,18 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
                 doc1.addField("referrer", request.getHeader("referer"));
             }
 
+            InetAddress ipAddress = null;
             try {
-                String dns = configurationService.getProperty("anonymize_statistics.dns_mask", "anonymized");
+                String dns;
                 if (!configurationService.getBooleanProperty("anonymize_statistics.anonymize_on_log", false)) {
-                    dns = DnsLookup.reverseDns(ip);
+                    ipAddress = InetAddress.getByName(ip);
+                    dns = ipAddress.getHostName();
+                } else {
+                    dns = configurationService.getProperty("anonymize_statistics.dns_mask", "anonymized");
                 }
-                doc1.addField("dns", dns.toLowerCase());
+                doc1.addField("dns", dns.toLowerCase(Locale.ROOT));
-            } catch (Exception e) {
+            } catch (UnknownHostException e) {
-                log.info("Failed DNS Lookup for IP:" + ip);
+                log.info("Failed DNS Lookup for IP:  {}", ip);
                 log.debug(e.getMessage(), e);
             }
             if (request.getHeader("User-Agent") != null) {
@@ -342,9 +347,8 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
             doc1.addField("isBot", isSpiderBot);
             // Save the location information if valid, save the event without
             // location information if not valid
-            if (locationService != null) {
+            if (locationService != null && ipAddress != null) {
                 try {
-                    InetAddress ipAddress = InetAddress.getByName(ip);
                     CityResponse location = locationService.city(ipAddress);
                     String countryCode = location.getCountry().getIsoCode();
                     double latitude = location.getLocation().getLatitude();
@@ -358,16 +362,17 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
                             doc1.addField("continent", LocationUtils
                                 .getContinentCode(countryCode));
                         } catch (Exception e) {
-                            System.out
+                            log.warn("Failed to load country/continent table: {}", countryCode);
-                                .println("COUNTRY ERROR: " + countryCode);
                         }
                         doc1.addField("countryCode", countryCode);
                         doc1.addField("city", location.getCity().getName());
                         doc1.addField("latitude", latitude);
                         doc1.addField("longitude", longitude);
                     }
-                } catch (IOException | GeoIp2Exception e) {
+                } catch (IOException e) {
-                    log.error("Unable to get location of request: {}", e.getMessage());
+                    log.warn("GeoIP lookup failed.", e);
+                } catch (GeoIp2Exception e) {
+                    log.info("Unable to get location of request: {}", e.getMessage());
                 }
             }
         }
@@ -408,14 +413,18 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
             doc1.addField("ip", ip);
         }
 
+        InetAddress ipAddress = null;
         try {
-            String dns = configurationService.getProperty("anonymize_statistics.dns_mask", "anonymized");
+            String dns;
             if (!configurationService.getBooleanProperty("anonymize_statistics.anonymize_on_log", false)) {
-                dns = DnsLookup.reverseDns(ip);
+                ipAddress = InetAddress.getByName(ip);
+                dns = ipAddress.getHostName();
+            } else {
+                dns = configurationService.getProperty("anonymize_statistics.dns_mask", "anonymized");
             }
-            doc1.addField("dns", dns.toLowerCase());
+            doc1.addField("dns", dns.toLowerCase(Locale.ROOT));
-        } catch (Exception e) {
+        } catch (UnknownHostException e) {
-            log.info("Failed DNS Lookup for IP:" + ip);
+            log.info("Failed DNS Lookup for IP:  {}", ip);
             log.debug(e.getMessage(), e);
         }
         if (userAgent != null) {
@@ -426,7 +435,6 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
         // location information if not valid
         if (locationService != null) {
             try {
-                InetAddress ipAddress = InetAddress.getByName(ip);
                 CityResponse location = locationService.city(ipAddress);
                 String countryCode = location.getCountry().getIsoCode();
                 double latitude = location.getLocation().getLatitude();
@@ -448,8 +456,10 @@ public class SolrLoggerServiceImpl implements SolrLoggerService, InitializingBea
                     doc1.addField("latitude", latitude);
                     doc1.addField("longitude", longitude);
                 }
-            } catch (GeoIp2Exception | IOException e) {
+            } catch (IOException e) {
-                log.error("Unable to get location of request:  {}", e.getMessage());
+                log.warn("GeoIP lookup failed.", e);
+            } catch (GeoIp2Exception e) {
+                log.info("Unable to get location of request: {}", e.getMessage());
             }
         }
 

dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java

@@ -12,6 +12,9 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
 /**
  * A Spare v4 IPTable implementation that uses nested HashMaps
  * to optimize IP address matching over ranges of IP addresses.
@@ -19,13 +22,23 @@ import java.util.Set;
  * @author mdiggory at atmire.com
  */
 public class IPTable {
+    private static final Logger log = LogManager.getLogger(IPTable.class);
 
     /* A lookup tree for IP addresses and SubnetRanges */
-    private Map<String, Map<String, Map<String, Set<String>>>> map =
+    private final Map<String, Map<String, Map<String, Set<String>>>> map
-        new HashMap<String, Map<String, Map<String, Set<String>>>>();
+            = new HashMap<>();
 
     /**
-     * Can be full v4 IP, subnet or range string
+     * Can be full v4 IP, subnet or range string.
+     * <ul>
+     *   <li>A full address is a complete dotted-quad:  {@code "1.2.3.4".}
+     *   <li>A subnet is a dotted-triplet:  {@code "1.2.3"}.  It means an entire
+     *       Class C subnet:  "1.2.3.0-1.2.3.255".
+     *   <li>A range is two dotted-quad addresses separated by hyphen:
+     *       {@code "1.2.3.4-1.2.3.14"}.  Only the final octet may be different.
+     * </ul>
+     *
+     * Any attempt at CIDR notation is ignored.
      *
      * @param ip IP address(es)
      * @throws IPFormatException Exception Class to deal with IPFormat errors.
@@ -59,7 +72,6 @@ public class IPTable {
 
             if (subnets.length < 3) {
                 throw new IPFormatException(ip + " - require at least three subnet places (255.255.255.0");
-
             }
 
             start = subnets;
@@ -67,27 +79,24 @@ public class IPTable {
         }
 
         if (start.length >= 3) {
-
-
             Map<String, Map<String, Set<String>>> first = map.get(start[0]);
 
             if (first == null) {
-                first = new HashMap<String, Map<String, Set<String>>>();
+                first = new HashMap<>();
                 map.put(start[0], first);
             }
 
             Map<String, Set<String>> second = first.get(start[1]);
 
-
             if (second == null) {
-                second = new HashMap<String, Set<String>>();
+                second = new HashMap<>();
                 first.put(start[1], second);
             }
 
             Set<String> third = second.get(start[2]);
 
             if (third == null) {
-                third = new HashSet<String>();
+                third = new HashSet<>();
                 second.put(start[2], third);
             }
 
@@ -115,14 +124,22 @@ public class IPTable {
      * Check whether a given address is contained in this netblock.
      *
      * @param ip the address to be tested
-     * @return true if {@code ip} is within this table's limits
+     * @return true if {@code ip} is within this table's limits.  Returns false
+     *         if {@link ip} looks like an IPv6 address.
      * @throws IPFormatException Exception Class to deal with IPFormat errors.
      */
     public boolean contains(String ip) throws IPFormatException {
 
         String[] subnets = ip.split("\\.");
 
-        if (subnets.length != 4) {
+        // Does it look like IPv6?
+        if (subnets.length > 4 || ip.contains("::")) {
+            log.warn("Address {} assumed not to match.  IPv6 is not implemented.", ip);
+            return false;
+        }
+
+        // Does it look like a subnet?
+        if (subnets.length < 4) {
             throw new IPFormatException("needs to be a single IP address");
         }
 
@@ -154,7 +171,7 @@ public class IPTable {
      * @return this table's content as a Set
      */
     public Set<String> toSet() {
-        HashSet<String> set = new HashSet<String>();
+        HashSet<String> set = new HashSet<>();
 
         for (Map.Entry<String, Map<String, Map<String, Set<String>>>> first : map.entrySet()) {
             String firstString = first.getKey();

dspace-api/src/test/java/org/dspace/statistics/util/IPTableTest.java

@@ -0,0 +1,100 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.statistics.util;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.dspace.statistics.util.IPTable.IPFormatException;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Ignore;
+import org.junit.Test;
+
+/**
+ *
+ * @author mwood
+ */
+public class IPTableTest {
+    private static final String LOCALHOST = "127.0.0.1";
+
+    public IPTableTest() {
+    }
+
+    @BeforeClass
+    public static void setUpClass() {
+    }
+
+    @AfterClass
+    public static void tearDownClass() {
+    }
+
+    @Before
+    public void setUp() {
+    }
+
+    @After
+    public void tearDown() {
+    }
+
+    /**
+     * Test of add method, of class IPTable.
+     * @throws java.lang.Exception passed through.
+     */
+    @Ignore
+    @Test
+    public void testAdd() throws Exception {
+    }
+
+    /**
+     * Test of contains method, of class IPTable.
+     * @throws java.lang.Exception passed through.
+     */
+    @Test
+    public void testContains()
+            throws Exception {
+        IPTable instance = new IPTable();
+        instance.add(LOCALHOST);
+        boolean contains;
+
+        contains = instance.contains(LOCALHOST);
+        assertTrue("Address that was add()ed should match", contains);
+
+        contains = instance.contains("192.168.1.1");
+        assertFalse("Address that was not add()ed should not match", contains);
+
+        contains = instance.contains("fec0:0:0:1::2");
+        assertFalse("IPv6 address should not match anything.", contains);
+    }
+
+    /**
+     * Test of contains method when presented with an invalid address.
+     * @throws Exception passed through.
+     */
+    @Test(expected = IPFormatException.class)
+    public void testContainsBadFormat()
+            throws Exception {
+        IPTable instance = new IPTable();
+        instance.add(LOCALHOST);
+        boolean contains;
+
+        // This should throw an IPFormatException.
+        contains = instance.contains("axolotl");
+        assertFalse("Nonsense string should raise an exception.", contains);
+    }
+
+    /**
+     * Test of toSet method, of class IPTable.
+     */
+    @Ignore
+    @Test
+    public void testToSet() {
+    }
+}