commit f0f57879d6074a979c33a46873ad5d9e60c6d72e Author: Karsten Loesing karsten.loesing@gmx.net Date: Sun Jul 29 20:01:49 2018 +0200
Extend "version" parameter to support lists and ranges.
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. --- CHANGELOG.md | 6 + .../org/torproject/onionoo/server/NodeIndex.java | 14 ++- .../org/torproject/onionoo/server/NodeIndexer.java | 9 +- .../torproject/onionoo/server/RequestHandler.java | 29 +++-- .../torproject/onionoo/server/ResourceServlet.java | 31 ++++- .../org/torproject/onionoo/updater/TorVersion.java | 125 ++++++++++++++------- .../onionoo/server/ResourceServletTest.java | 85 ++++++++++++-- .../torproject/onionoo/updater/TorVersionTest.java | 10 +- 8 files changed, 233 insertions(+), 76 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md index 4937eb3..243c5af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# Changes in version 7.0-1.18.0 - 2018-09-?? + + * Medium changes + - Extend "version" parameter to support lists and ranges. + + # Changes in version 6.2-1.17.1 - 2018-08-17
* Minor changes diff --git a/src/main/java/org/torproject/onionoo/server/NodeIndex.java b/src/main/java/org/torproject/onionoo/server/NodeIndex.java index d8b983a..0f38cd3 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; @@ -180,23 +181,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 80418aa..1a13b99 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<>(); @@ -264,7 +265,7 @@ public class NodeIndexer implements ServletContextListener, Runnable { newRelaysByContact.putIfAbsent(contact, new HashSet<>()); newRelaysByContact.get(contact).add(fingerprint); newRelaysByContact.get(contact).add(hashedFingerprint); - String version = entry.getVersion(); + TorVersion version = TorVersion.of(entry.getVersion()); if (null != version) { newRelaysByVersion.putIfAbsent(version, new HashSet<>()); newRelaysByVersion.get(version).add(fingerprint); @@ -346,7 +347,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) { newBridgesByVersion.putIfAbsent(version, new HashSet<>()); newBridgesByVersion.get(version).add(hashedFingerprint); diff --git a/src/main/java/org/torproject/onionoo/server/RequestHandler.java b/src/main/java/org/torproject/onionoo/server/RequestHandler.java index d1e9cce..e3c94f9 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; @@ -96,9 +97,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; }
@@ -572,18 +573,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 7913d22..22077ae 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; @@ -292,7 +294,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); @@ -630,14 +632,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 6440283..3fb0ef0 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 a6042b1..921d6e4 100644 --- a/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java +++ b/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java @@ -293,19 +293,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); } } @@ -1684,8 +1684,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 @@ -1705,8 +1704,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 @@ -1716,11 +1716,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 }, + }); }