Extend "version" parameter to support lists and ranges.task-6947

Changes the current behavior of the "version" parameter by moving away
from string prefix matching to actually parsing provided (partial)
versions. As a result, for example, "version=0.3.2.1" doesn't
magically include versions 0.3.2.10 to 0.3.2.19, 0.3.2.100 to
0.3.2.199, etc. anymore. Without this change, version ranges would
have become just too confusing. The downside is that this change
requires a major version bump.

Implements #6947.

8 files changed, 235 insertions, 78 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 00a0c6a..f0b3a9c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,9 @@
+# Changes in version 7.0-1.1?.0 - 2018-0?-??
+
+ * Medium changes
+   - Extend "version" parameter to support lists and ranges.
+
+
 # Changes in version 6.2-1.16.0 - 2018-07-??
 
  * Medium changes
diff --git a/src/main/java/org/torproject/onionoo/server/NodeIndex.java b/src/main/java/org/torproject/onionoo/server/NodeIndex.java
index ed9ec44..7ec86b2 100644
--- a/src/main/java/org/torproject/onionoo/server/NodeIndex.java
+++ b/src/main/java/org/torproject/onionoo/server/NodeIndex.java
@@ -4,6 +4,7 @@
 package org.torproject.onionoo.server;
 
 import org.torproject.onionoo.docs.SummaryDocument;
+import org.torproject.onionoo.updater.TorVersion;
 
 import java.text.SimpleDateFormat;
 import java.util.Map;
@@ -170,23 +171,24 @@ class NodeIndex {
     return bridgesByLastSeenDays;
   }
 
-  private Map<String, Set<String>> relaysByVersion;
+  private Map<TorVersion, Set<String>> relaysByVersion;
 
-  public void setRelaysByVersion(Map<String, Set<String>> relaysByVersion) {
+  public void setRelaysByVersion(Map<TorVersion, Set<String>> relaysByVersion) {
     this.relaysByVersion = relaysByVersion;
   }
 
-  public Map<String, Set<String>> getRelaysByVersion() {
+  public Map<TorVersion, Set<String>> getRelaysByVersion() {
     return this.relaysByVersion;
   }
 
-  private Map<String, Set<String>> bridgesByVersion;
+  private Map<TorVersion, Set<String>> bridgesByVersion;
 
-  public void setBridgesByVersion(Map<String, Set<String>> bridgesByVersion) {
+  public void setBridgesByVersion(Map<TorVersion,
+      Set<String>> bridgesByVersion) {
     this.bridgesByVersion = bridgesByVersion;
   }
 
-  public Map<String, Set<String>> getBridgesByVersion() {
+  public Map<TorVersion, Set<String>> getBridgesByVersion() {
     return this.bridgesByVersion;
   }
 
diff --git a/src/main/java/org/torproject/onionoo/server/NodeIndexer.java b/src/main/java/org/torproject/onionoo/server/NodeIndexer.java
index 1f1d279..46d1dd6 100644
--- a/src/main/java/org/torproject/onionoo/server/NodeIndexer.java
+++ b/src/main/java/org/torproject/onionoo/server/NodeIndexer.java
@@ -10,6 +10,7 @@ import org.torproject.onionoo.docs.DocumentStore;
 import org.torproject.onionoo.docs.DocumentStoreFactory;
 import org.torproject.onionoo.docs.SummaryDocument;
 import org.torproject.onionoo.docs.UpdateStatus;
+import org.torproject.onionoo.updater.TorVersion;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -155,8 +156,8 @@ public class NodeIndexer implements ServletContextListener, Runnable {
     Map<String, Set<String>> newBridgesByFlag = new HashMap<>();
     Map<String, Set<String>> newRelaysByContact = new HashMap<>();
     Map<String, Set<String>> newRelaysByFamily = new HashMap<>();
-    Map<String, Set<String>> newRelaysByVersion = new HashMap<>();
-    Map<String, Set<String>> newBridgesByVersion = new HashMap<>();
+    Map<TorVersion, Set<String>> newRelaysByVersion = new HashMap<>();
+    Map<TorVersion, Set<String>> newBridgesByVersion = new HashMap<>();
     Map<String, Set<String>> newRelaysByOperatingSystem = new HashMap<>();
     Map<String, Set<String>> newBridgesByOperatingSystem = new HashMap<>();
     Map<String, Set<String>> newRelaysByHostName = new HashMap<>();
@@ -275,10 +276,10 @@ public class NodeIndexer implements ServletContextListener, Runnable {
       }
       newRelaysByContact.get(contact).add(fingerprint);
       newRelaysByContact.get(contact).add(hashedFingerprint);
-      String version = entry.getVersion();
+      TorVersion version = TorVersion.of(entry.getVersion());
       if (null != version) {
         if (!newRelaysByVersion.containsKey(version)) {
-          newRelaysByVersion.put(version, new HashSet<String>());
+          newRelaysByVersion.put(version, new HashSet<>());
         }
         newRelaysByVersion.get(version).add(fingerprint);
         newRelaysByVersion.get(version).add(hashedFingerprint);
@@ -369,7 +370,7 @@ public class NodeIndexer implements ServletContextListener, Runnable {
           hashedFingerprint);
       newBridgesByLastSeenDays.get(daysSinceLastSeen).add(
           hashedHashedFingerprint);
-      String version = entry.getVersion();
+      TorVersion version = TorVersion.of(entry.getVersion());
       if (null != version) {
         if (!newBridgesByVersion.containsKey(version)) {
           newBridgesByVersion.put(version, new HashSet<>());
diff --git a/src/main/java/org/torproject/onionoo/server/RequestHandler.java b/src/main/java/org/torproject/onionoo/server/RequestHandler.java
index 344178b..3b74c45 100644
--- a/src/main/java/org/torproject/onionoo/server/RequestHandler.java
+++ b/src/main/java/org/torproject/onionoo/server/RequestHandler.java
@@ -6,6 +6,7 @@ package org.torproject.onionoo.server;
 import org.torproject.onionoo.docs.DocumentStore;
 import org.torproject.onionoo.docs.DocumentStoreFactory;
 import org.torproject.onionoo.docs.SummaryDocument;
+import org.torproject.onionoo.updater.TorVersion;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -91,9 +92,9 @@ public class RequestHandler {
     System.arraycopy(contact, 0, this.contact, 0, contact.length);
   }
 
-  private String version;
+  private List<TorVersion[]> version;
 
-  public void setVersion(String version) {
+  public void setVersion(List<TorVersion[]> version) {
     this.version = version;
   }
 
@@ -551,18 +552,30 @@ public class RequestHandler {
       return;
     }
     Set<String> keepRelays = new HashSet<>();
-    for (Map.Entry<String, Set<String>> e
+    for (Map.Entry<TorVersion, Set<String>> e
         : this.nodeIndex.getRelaysByVersion().entrySet()) {
-      if (e.getKey().startsWith(this.version)) {
-        keepRelays.addAll(e.getValue());
+      for (TorVersion[] versionRange : this.version) {
+        if ((null == versionRange[0]
+            || e.getKey().compareTo(versionRange[0]) >= 0)
+            && (null == versionRange[1]
+            || e.getKey().compareTo(versionRange[1]) <= 0
+            || e.getKey().matchingPrefix(versionRange[1]))) {
+          keepRelays.addAll(e.getValue());
+        }
       }
     }
     this.filteredRelays.keySet().retainAll(keepRelays);
     Set<String> keepBridges = new HashSet<>();
-    for (Map.Entry<String, Set<String>> e
+    for (Map.Entry<TorVersion, Set<String>> e
         : this.nodeIndex.getBridgesByVersion().entrySet()) {
-      if (e.getKey().startsWith(this.version)) {
-        keepBridges.addAll(e.getValue());
+      for (TorVersion[] versionRange : this.version) {
+        if ((null == versionRange[0]
+            || e.getKey().compareTo(versionRange[0]) >= 0)
+            && (null == versionRange[1]
+            || e.getKey().compareTo(versionRange[1]) <= 0
+            || e.getKey().matchingPrefix(versionRange[1]))) {
+          keepBridges.addAll(e.getValue());
+        }
       }
     }
     this.filteredBridges.keySet().retainAll(keepBridges);
diff --git a/src/main/java/org/torproject/onionoo/server/ResourceServlet.java b/src/main/java/org/torproject/onionoo/server/ResourceServlet.java
index 534dc0c..e095415 100644
--- a/src/main/java/org/torproject/onionoo/server/ResourceServlet.java
+++ b/src/main/java/org/torproject/onionoo/server/ResourceServlet.java
@@ -3,6 +3,8 @@
 
 package org.torproject.onionoo.server;
 
+import org.torproject.onionoo.updater.TorVersion;
+
 import org.apache.commons.lang3.StringUtils;
 
 import java.io.IOException;
@@ -283,7 +285,7 @@ public class ResourceServlet extends HttpServlet {
       rh.setContact(contactParts);
     }
     if (parameterMap.containsKey("version")) {
-      String versionParameter = this.parseVersionParameter(
+      List<TorVersion[]> versionParameter = this.parseVersionParameter(
           parameterMap.get("version"));
       if (null == versionParameter) {
         response.sendError(HttpServletResponse.SC_BAD_REQUEST);
@@ -361,7 +363,7 @@ public class ResourceServlet extends HttpServlet {
       String[] familyParameter = this.parseFingerprintParameter(
           parameterMap.get("family"));
       if (null == familyParameter || 1 != familyParameter.length) {
-          response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+        response.sendError(HttpServletResponse.SC_BAD_REQUEST);
         return;
       }
       rh.setFamily(familyParameter[0]);
@@ -599,14 +601,35 @@ public class ResourceServlet extends HttpServlet {
   }
 
   private static Pattern versionParameterPattern =
-      Pattern.compile("^[0-9a-zA-Z\\.-]+$");
+      Pattern.compile("^[0-9a-zA-Z,\\.-]+$");
 
-  private String parseVersionParameter(String parameter) {
+  private List<TorVersion[]> parseVersionParameter(String parameter) {
     if (!versionParameterPattern.matcher(parameter).matches()) {
       /* Version contains illegal character(s). */
       return null;
     }
-    return parameter;
+    List<TorVersion[]> result = new ArrayList<>();
+    for (String listElement : parameter.split(",")) {
+      TorVersion fromVersion;
+      TorVersion toVersion;
+      if (listElement.contains("..")) {
+        fromVersion = TorVersion.of(
+            listElement.substring(0, listElement.lastIndexOf("..")));
+        toVersion = TorVersion.of(
+            listElement.substring(listElement.lastIndexOf("..") + 2));
+      } else {
+        fromVersion = toVersion = TorVersion.of(listElement);
+      }
+      if (null == fromVersion && null == toVersion) {
+        return null;
+      }
+      if (null != fromVersion && null != toVersion
+          && fromVersion.compareTo(toVersion) > 0) {
+        return null;
+      }
+      result.add(new TorVersion[] { fromVersion, toVersion });
+    }
+    return result;
   }
 
   private String parseOperatingSystemParameter(String parameter) {
diff --git a/src/main/java/org/torproject/onionoo/updater/TorVersion.java b/src/main/java/org/torproject/onionoo/updater/TorVersion.java
index d8e1683..e8bdd0f 100644
--- a/src/main/java/org/torproject/onionoo/updater/TorVersion.java
+++ b/src/main/java/org/torproject/onionoo/updater/TorVersion.java
@@ -3,7 +3,9 @@
 
 package org.torproject.onionoo.updater;
 
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.SortedSet;
 
@@ -15,16 +17,10 @@ import java.util.SortedSet;
  */
 public class TorVersion implements Comparable<TorVersion> {
 
-  private int majorVersion;
-
-  private int minorVersion;
-
-  private int microVersion;
+  private List<Integer> versionNumbers = new ArrayList<>();
 
   private String releaseSeries;
 
-  private Integer patchLevel = null;
-
   private String statusTag = null;
 
   private static Map<String, TorVersion> knownVersions = new HashMap<>();
@@ -41,21 +37,32 @@ public class TorVersion implements Comparable<TorVersion> {
     }
     if (!knownVersions.containsKey(versionString)) {
       TorVersion result = new TorVersion();
-      String[] components = versionString.split("-")[0].split("\\.");
+      boolean isValid = true;
       try {
-        result.majorVersion = Integer.parseInt(components[0]);
-        result.minorVersion = Integer.parseInt(components[1]);
-        result.microVersion = Integer.parseInt(components[2]);
-        result.releaseSeries = String.format("%d.%d.%d",
-            result.majorVersion, result.minorVersion, result.microVersion);
-        if (components.length == 4) {
-          result.patchLevel = Integer.parseInt(components[3]);
-          if (versionString.contains("-")) {
-            result.statusTag = versionString.split("-", 2)[1].split(" ")[0];
+        String[] components = versionString.split("-")[0].split("\\.", -1);
+        for (int position = 0; position < 4 && position < components.length;
+             position++) {
+          if (!components[position].isEmpty()) {
+            result.versionNumbers.add(Integer.parseInt(components[position]));
+          } else if (0 == position || position < components.length - 1) {
+            /* Version cannot start with a blank, nor can it contain a blank in
+             * between two dots. */
+            isValid = false;
           }
         }
+        if (result.versionNumbers.size() >= 3) {
+          result.releaseSeries = String.format("%d.%d.%d",
+              result.versionNumbers.get(0), result.versionNumbers.get(1),
+              result.versionNumbers.get(2));
+        }
+        if (versionString.contains("-")) {
+          result.statusTag = versionString.split("-", 2)[1].split(" ")[0];
+        }
       } catch (ArrayIndexOutOfBoundsException
           | NumberFormatException exception) {
+        isValid = false;
+      }
+      if (!isValid) {
         result = null;
       }
       knownVersions.put(versionString, result);
@@ -69,27 +76,15 @@ public class TorVersion implements Comparable<TorVersion> {
       throw new NullPointerException();
     }
     int result;
-    if ((result = Integer.compare(this.majorVersion,
-        other.majorVersion)) != 0) {
-      return result;
-    }
-    if ((result = Integer.compare(this.minorVersion,
-        other.minorVersion)) != 0) {
-      return result;
+    for (int position = 0; position < this.versionNumbers.size()
+        && position < other.versionNumbers.size(); position++) {
+      if ((result = Integer.compare(this.versionNumbers.get(position),
+          other.versionNumbers.get(position))) != 0) {
+        return result;
+      }
     }
-    if ((result = Integer.compare(this.microVersion,
-        other.microVersion)) != 0) {
-      return result;
-    }
-    if (null == this.patchLevel && null == other.patchLevel) {
-      return 0;
-    } else if (null == patchLevel) {
-      return -1;
-    } else if (null == other.patchLevel) {
-      return 1;
-    } else if ((result = Integer.compare(this.patchLevel,
-        other.patchLevel)) != 0) {
-      return result;
+    if (this.versionNumbers.size() != other.versionNumbers.size()) {
+      return this.versionNumbers.size() < other.versionNumbers.size() ? -1 : 1;
     }
     if (null == this.statusTag && null == other.statusTag) {
       return 0;
@@ -108,20 +103,64 @@ public class TorVersion implements Comparable<TorVersion> {
         && this.compareTo((TorVersion) other) == 0;
   }
 
+  /** Return whether prefixes of this version and another version match.
+   *
+   * <p>Two versions A and B have the same prefix if A starts with B, B starts
+   * with A, or A and B are the same.</p>
+   */
+  public boolean matchingPrefix(TorVersion other) {
+    if (null == other) {
+      throw new NullPointerException();
+    }
+    for (int position = 0; position < this.versionNumbers.size()
+        && position < other.versionNumbers.size(); position++) {
+      if ((Integer.compare(this.versionNumbers.get(position),
+          other.versionNumbers.get(position))) != 0) {
+        return false;
+      }
+    }
+    if (null != this.statusTag && null != other.statusTag) {
+      return this.statusTag.equals(other.statusTag);
+    }
+    return true;
+  }
+
   @Override
   public int hashCode() {
-    return 2 * Integer.hashCode(this.majorVersion)
-        + 3 * Integer.hashCode(this.minorVersion)
-        + 5 * Integer.hashCode(this.microVersion)
-        + 7 * (null == this.patchLevel ? 0 : this.patchLevel)
-        + 11 * (null == this.statusTag ? 0 : this.statusTag.hashCode());
+    int result = 0;
+    for (int position = 0; position < this.versionNumbers.size(); position++) {
+      result += (2 * position + 1)
+          * Integer.hashCode(this.versionNumbers.get(position));
+    }
+    if (null != this.statusTag) {
+      result += 11 * this.statusTag.hashCode();
+    }
+    return result;
+  }
+
+  @Override
+  public String toString() {
+    StringBuilder sb = new StringBuilder();
+    for (int position = 0; position < this.versionNumbers.size(); position++) {
+      if (position > 0) {
+        sb.append('.');
+      }
+      sb.append(this.versionNumbers.get(position));
+    }
+    if (null != this.statusTag) {
+      sb.append('-').append(this.statusTag);
+    }
+    return sb.toString();
   }
 
   /** Determine the version status of this tor version in the context of the
    * given recommended tor versions. */
   public TorVersionStatus determineVersionStatus(
       SortedSet<TorVersion> recommendedVersions) {
-    if (recommendedVersions.contains(this)) {
+    if (null == this.releaseSeries) {
+      /* Only consider full versions, not partial versions. */
+      return TorVersionStatus.UNRECOMMENDED;
+    } else if (recommendedVersions.contains(this)) {
       return TorVersionStatus.RECOMMENDED;
     } else if (this.compareTo(recommendedVersions.last()) > 0) {
       return TorVersionStatus.EXPERIMENTAL;
diff --git a/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java b/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java
index 8100c00..f35c392 100644
--- a/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java
+++ b/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java
@@ -291,19 +291,19 @@ public class ResourceServletTest {
     this.runTest(request);
     assertNotNull("Summary document is null, status code is "
         + this.response.errorStatusCode, this.summaryDocument);
-    assertEquals(expectedRelaysNumber,
+    assertEquals("Unexpected number of relays.", expectedRelaysNumber,
         this.summaryDocument.relays.length);
     if (expectedRelaysNicknames != null) {
       for (int i = 0; i < expectedRelaysNumber; i++) {
-        assertEquals(expectedRelaysNicknames[i],
+        assertEquals("Unexpected relay nickname.", expectedRelaysNicknames[i],
             this.summaryDocument.relays[i].n);
       }
     }
-    assertEquals(expectedBridgesNumber,
+    assertEquals("Unexpected number of bridges.", expectedBridgesNumber,
         this.summaryDocument.bridges.length);
     if (expectedBridgesNicknames != null) {
       for (int i = 0; i < expectedBridgesNumber; i++) {
-        assertEquals(expectedBridgesNicknames[i],
+        assertEquals("Unexpected bridge nickname.", expectedBridgesNicknames[i],
             this.summaryDocument.bridges[i].n);
       }
     }
@@ -1621,8 +1621,7 @@ public class ResourceServletTest {
 
   @Test
   public void testVersionBlaBlaBla() {
-    this.assertSummaryDocument("/summary?version=bla-bla-bla", 0, null, 0,
-        null);
+    this.assertErrorStatusCode("/summary?version=bla-bla-bla", 400);
   }
 
   @Test
@@ -1642,8 +1641,9 @@ public class ResourceServletTest {
 
   @Test
   public void testVersion0232() {
-    /* This is correct when comparing strings. */
-    this.assertSummaryDocument("/summary?version=0.2.3.2", 2, null, 0, null);
+    /* This is only correct when comparing strings, not when comparing parsed
+     * version numbers. */
+    this.assertSummaryDocument("/summary?version=0.2.3.2", 0, null, 0, null);
   }
 
   @Test
@@ -1653,11 +1653,76 @@ public class ResourceServletTest {
   }
 
   @Test
-  public void testVersionStart() {
-    /* This is also correct when comparing strings. */
+  public void testVersionStar() {
     this.assertErrorStatusCode("/summary?version=*", 400);
   }
 
+  @Test
+  public void testVersionRangeTo() {
+    this.assertSummaryDocument("/summary?version=..0.2.3.24", 1, null, 1, null);
+  }
+
+  @Test
+  public void testVersionRangeFrom() {
+    this.assertSummaryDocument("/summary?version=0.2.3.25..", 1, null, 1, null);
+  }
+
+  @Test
+  public void testVersionRangeFromTo() {
+    this.assertSummaryDocument("/summary?version=0.2.3.24..0.2.3.25", 2, null,
+        0, null);
+  }
+
+  @Test
+  public void testVersionRangeFromToExchanged() {
+    this.assertErrorStatusCode("/summary?version=0.2.3.25..0.2.3.24", 400);
+  }
+
+  @Test
+  public void testVersionTwoSingles() {
+    this.assertSummaryDocument("/summary?version=0.2.2.39,0.2.3.24", 1, null, 1,
+        null);
+  }
+
+  @Test
+  public void testVersionTwoOtherSingles() {
+    this.assertSummaryDocument("/summary?version=0.2.2.39,0.2.4.4", 0, null, 2,
+        null);
+  }
+
+  @Test
+  public void testVersionSingleAndRange() {
+    this.assertSummaryDocument("/summary?version=0.2.2.39,0.2.4..", 0, null, 2,
+        null);
+  }
+
+  @Test
+  public void testVersion0AndLater() {
+    this.assertSummaryDocument("/summary?version=0..", 2, null, 2, null);
+  }
+
+  @Test
+  public void testVersionJustTwoDots() {
+    /* Need at least a start or an end. */
+    this.assertErrorStatusCode("/summary?version=..", 400);
+  }
+
+  @Test
+  public void testVersion0ThreeDots() {
+    /* Parses as "all versions starting at 0.". */
+    this.assertSummaryDocument("/summary?version=0...", 2, null, 2, null);
+  }
+
+  @Test
+  public void testVersion0FourDots() {
+    this.assertErrorStatusCode("/summary?version=0....", 400);
+  }
+
+  @Test
+  public void testVersion1AndEarlier() {
+    this.assertSummaryDocument("/summary?version=..1", 2, null, 2, null);
+  }
+
   @Test(timeout = 100)
   public void testOperatingSystemLinux() {
     this.assertSummaryDocument(
diff --git a/src/test/java/org/torproject/onionoo/updater/TorVersionTest.java b/src/test/java/org/torproject/onionoo/updater/TorVersionTest.java
index b6a9764..e6f4e64 100644
--- a/src/test/java/org/torproject/onionoo/updater/TorVersionTest.java
+++ b/src/test/java/org/torproject/onionoo/updater/TorVersionTest.java
@@ -89,7 +89,15 @@ public class TorVersionTest {
           { "0.2.5.16", "0.2.5.17", false, false, -1 },
           { "0.3.3.1-alpha", "0.3.3.1-alpha", true, true, 0 },
           { "0.1.2.3", "00.01.02.03", true, true, 0 },
-          { "0.1.2.3-alpha", "00.01.02.03-aallpphhaa", false, false, 1 }
+          { "0.1.2.3-alpha", "00.01.02.03-aallpphhaa", false, false, 1 },
+          { "0", "0.1.2.3", false, false, -1 },
+          { "0.", "0.1.2.3", false, false, -1 },
+          { "0.1", "0.1.2.3", false, false, -1 },
+          { "0.1.", "0.1.2.3", false, false, -1 },
+          { "0.1.2", "0.1.2.3", false, false, -1 },
+          { "0.1.2.", "0.1.2.3", false, false, -1 },
+          { "0.2", "0.1.2.3", false, false, 1 },
+
       });
     }