commit 7593af19c332ffe6d33bc429a3d3234dd3cee3e1 Author: Karsten Loesing karsten.loesing@gmx.net Date: Tue Jan 10 10:48:54 2012 +0100

Add severities to warning messages.

Implements #4878. --- src/org/torproject/doctor/Checker.java | 78 ++++------------ src/org/torproject/doctor/Main.java | 3 +- src/org/torproject/doctor/StatusFileReport.java | 114 +++++++++++++--------- 3 files changed, 86 insertions(+), 109 deletions(-)

diff --git a/src/org/torproject/doctor/Checker.java b/src/org/torproject/doctor/Checker.java index dc1ea46..2df8f54 100644 --- a/src/org/torproject/doctor/Checker.java +++ b/src/org/torproject/doctor/Checker.java @@ -12,9 +12,9 @@ import org.torproject.descriptor.*; public class Checker {

/* Warning messages consisting of type and details. */ - private SortedMap<Warning, String> warnings = - new TreeMap<Warning, String>(); - public SortedMap<Warning, String> getWarnings() { + private SortedMap<Warning, SortedSet<String>> warnings = + new TreeMap<Warning, SortedSet<String>>(); + public SortedMap<Warning, SortedSet<String>> getWarnings() { return this.warnings; }

@@ -44,7 +44,7 @@ public class Checker { this.checkBandwidthScanners(); } } else { - this.warnings.put(Warning.NoConsensusKnown, ""); + this.warnings.put(Warning.NoConsensusKnown, new TreeSet<String>()); } }

@@ -93,12 +93,8 @@ public class Checker { + "moria1,dizum").split(","))); missingConsensuses.removeAll(this.downloadedConsensuses.keySet()); if (!missingConsensuses.isEmpty()) { - StringBuilder sb = new StringBuilder(); - for (String nickname : missingConsensuses) { - sb.append(", " + nickname); - } this.warnings.put(Warning.ConsensusDownloadTimeout, - sb.toString().substring(2)); + missingConsensuses); } }

@@ -115,12 +111,7 @@ public class Checker { } } if (!nonFresh.isEmpty()) { - StringBuilder sb = new StringBuilder(); - for (String nickname : nonFresh) { - sb.append(", " + nickname); - } - this.warnings.put(Warning.ConsensusNotFresh, - sb.toString().substring(2)); + this.warnings.put(Warning.ConsensusNotFresh, nonFresh); } }

@@ -142,12 +133,7 @@ public class Checker { } } if (!missingVotes.isEmpty()) { - StringBuilder sb = new StringBuilder(); - for (String nickname : missingVotes) { - sb.append(", " + nickname); - } - this.warnings.put(Warning.ConsensusMissingVotes, - sb.toString().substring(2)); + this.warnings.put(Warning.ConsensusMissingVotes, missingVotes); } }

@@ -166,12 +152,8 @@ public class Checker { } } if (!missingSignatures.isEmpty()) { - StringBuilder sb = new StringBuilder(); - for (String nickname : missingSignatures) { - sb.append(", " + nickname); - } this.warnings.put(Warning.ConsensusMissingSignatures, - sb.toString().substring(2)); + missingSignatures); } }

@@ -192,12 +174,7 @@ public class Checker { } } if (!dirs.isEmpty()) { - StringBuilder sb = new StringBuilder(); - for (String dir : dirs) { - sb.append(", " + dir); - } - this.warnings.put(Warning.ConsensusMethodNotSupported, - sb.toString().substring(2)); + this.warnings.put(Warning.ConsensusMethodNotSupported, dirs); } }

@@ -229,20 +206,12 @@ public class Checker { } } if (!unrecommendedServerVersions.isEmpty()) { - StringBuilder sb = new StringBuilder(); - for (String dir : unrecommendedServerVersions) { - sb.append(", " + dir); - } this.warnings.put(Warning.DifferentRecommendedServerVersions, - sb.toString().substring(2)); + unrecommendedServerVersions); } if (!unrecommendedClientVersions.isEmpty()) { - StringBuilder sb = new StringBuilder(); - for (String dir : unrecommendedClientVersions) { - sb.append(", " + dir); - } this.warnings.put(Warning.DifferentRecommendedClientVersions, - sb.toString().substring(2)); + unrecommendedClientVersions); } }

@@ -282,12 +251,8 @@ public class Checker { } } if (!conflicts.isEmpty()) { - StringBuilder sb = new StringBuilder(); - for (String dir : conflicts) { - sb.append(", " + dir); - } this.warnings.put(Warning.ConflictingOrInvalidConsensusParams, - sb.toString().substring(2)); + conflicts); } }

@@ -335,15 +300,15 @@ public class Checker {

private void warnAboutExpiringCertificates(Warning warningType, SortedMap<String, String> expiringCertificates) { + SortedSet<String> details = new TreeSet<String>(); StringBuilder sb = new StringBuilder(); for (Map.Entry<String, String> e : expiringCertificates.entrySet()) { String dir = e.getKey(); String timestamp = e.getValue(); - sb.append(", " + dir + " " + timestamp); + details.add(dir + " " + timestamp); } - String details = sb.toString().substring(2); - this.warnings.put(warningType, sb.toString().substring(2)); + this.warnings.put(warningType, details); }

/* Check if any votes are missing. */ @@ -357,12 +322,7 @@ public class Checker { missingVotes.remove(vote.getNickname()); } if (!missingVotes.isEmpty()) { - StringBuilder sb = new StringBuilder(); - for (String missingDir : missingVotes) { - sb.append(", " + missingDir); - } - this.warnings.put(Warning.VotesMissing, - sb.toString().substring(2)); + this.warnings.put(Warning.VotesMissing, missingVotes); } }

@@ -383,12 +343,8 @@ public class Checker { } } if (!missingBandwidthScanners.isEmpty()) { - StringBuilder sb = new StringBuilder(); - for (String dir : missingBandwidthScanners) { - sb.append(", " + dir); - } this.warnings.put(Warning.BandwidthScannerResultsMissing, - sb.toString().substring(2)); + missingBandwidthScanners); } } } diff --git a/src/org/torproject/doctor/Main.java b/src/org/torproject/doctor/Main.java index f723e0b..b6827a3 100644 --- a/src/org/torproject/doctor/Main.java +++ b/src/org/torproject/doctor/Main.java @@ -21,7 +21,8 @@ public class Main { StatusFileReport statusFile = new StatusFileReport(); Checker checker = new Checker(); checker.processDownloadedConsensuses(downloads); - SortedMap<Warning, String> warnings = checker.getWarnings(); + SortedMap<Warning, SortedSet<String>> warnings = + checker.getWarnings(); statusFile.processWarnings(warnings); statusFile.writeReport();

diff --git a/src/org/torproject/doctor/StatusFileReport.java b/src/org/torproject/doctor/StatusFileReport.java index 994704a..8e78571 100644 --- a/src/org/torproject/doctor/StatusFileReport.java +++ b/src/org/torproject/doctor/StatusFileReport.java @@ -20,8 +20,9 @@ public class StatusFileReport { }

/* Warnings obtained from checking the current consensus and votes. */ - private SortedMap<Warning, String> warnings; - public void processWarnings(SortedMap<Warning, String> warnings) { + private SortedMap<Warning, SortedSet<String>> warnings; + public void processWarnings(SortedMap<Warning, + SortedSet<String>> warnings) { this.warnings = warnings; }

@@ -74,78 +75,90 @@ public class StatusFileReport { private String allWarnings = null, newWarnings = null; private void prepareStatusFiles() { SortedMap<String, Long> warningStrings = new TreeMap<String, Long>(); - for (Map.Entry<Warning, String> e : this.warnings.entrySet()) { + for (Map.Entry<Warning, SortedSet<String>> e : + this.warnings.entrySet()) { Warning type = e.getKey(); - String details = e.getValue(); + SortedSet<String> details = e.getValue(); + StringBuilder sb = new StringBuilder(); + int written = 0; + for (String detail : details) { + sb.append((written++ > 0 ? ", " : "") + detail); + } + String detailsString = sb.toString(); switch (type) { case NoConsensusKnown: - warningStrings.put("No consensus known.", 0L); + warningStrings.put("ERROR: No consensus known.", 0L); break; case ConsensusDownloadTimeout: - warningStrings.put("The following directory authorities did " - + "not return a consensus within a timeout of 60 seconds: " - + details, 150L * 60L * 1000L); + warningStrings.put((details.size() > 3 ? "ERROR" : "WARNING") + + ": The following directory authorities did not return a " + + "consensus within a timeout of 60 seconds: " + + detailsString, 150L * 60L * 1000L); break; case ConsensusNotFresh: - warningStrings.put("The consensuses published by the following " - + "directory authorities are more than 1 hour old and " - + "therefore not fresh anymore: " + details, - 150L * 60L * 1000L); + warningStrings.put((details.size() > 3 ? "ERROR" : "WARNING") + + ": The consensuses published by the following directory " + + "authorities are more than 1 hour old and therefore not " + + "fresh anymore: " + detailsString, 150L * 60L * 1000L); break; case ConsensusMethodNotSupported: - warningStrings.put("The following directory authorities do not " - + "support the consensus method that the consensus uses: " - + details, 24L * 60L * 60L * 1000L); + warningStrings.put("WARNING: The following directory " + + "authorities do not support the consensus method that " + + "the consensus uses: " + detailsString, + 24L * 60L * 60L * 1000L); break; case DifferentRecommendedClientVersions: - warningStrings.put("The following directory authorities " - + "recommend other client versions than the consensus: " - + details, 150L * 60L * 1000L); + warningStrings.put("NOTICE: The following directory " + + "authorities recommend other client versions than the " + + "consensus: " + detailsString, 150L * 60L * 1000L); break; case DifferentRecommendedServerVersions: - warningStrings.put("The following directory authorities " - + "recommend other server versions than the consensus: " - + details, 150L * 60L * 1000L); + warningStrings.put("NOTICE: The following directory " + + "authorities recommend other server versions than the " + + "consensus: " + detailsString, 150L * 60L * 1000L); break; case ConflictingOrInvalidConsensusParams: - warningStrings.put("The following directory authorities set " - + "conflicting or invalid consensus parameters: " + details, - 150L * 60L * 1000L); + warningStrings.put("NOTICE: The following directory " + + "authorities set conflicting or invalid consensus " + + "parameters: " + detailsString, 150L * 60L * 1000L); break; case CertificateExpiresInThreeMonths: - warningStrings.put("The certificates of the following " + warningStrings.put("NOTICE: The certificates of the following " + "directory authorities expire within the next three " - + "months: " + details, 5L * 7L * 24L * 60L * 60L * 1000L); + + "months: " + detailsString, + 5L * 7L * 24L * 60L * 60L * 1000L); break; case CertificateExpiresInTwoMonths: - warningStrings.put("The certificates of the following " + warningStrings.put("NOTICE: The certificates of the following " + "directory authorities expire within the next two " - + "months: " + details, 7L * 24L * 60L * 60L * 1000L); + + "months: " + detailsString, 7L * 24L * 60L * 60L * 1000L); break; case CertificateExpiresInTwoWeeks: - warningStrings.put("The certificates of the following " + warningStrings.put("WARNING: The certificates of the following " + "directory authorities expire within the next 14 days: " - + details, 24L * 60L * 60L * 1000L); + + detailsString, 24L * 60L * 60L * 1000L); break; case VotesMissing: - warningStrings.put("We're missing votes from the following " - + "directory authorities: " + details, 150L * 60L * 1000L); + warningStrings.put("WARNING: We're missing votes from the " + + "following directory authorities: " + detailsString, + 150L * 60L * 1000L); break; case BandwidthScannerResultsMissing: - warningStrings.put("The following directory authorities are " - + "not reporting bandwidth scanner results: " + details, + warningStrings.put((details.size() > 1 ? "ERROR" : "WARNING") + + ": The following directory authorities are not reporting " + + "bandwidth scanner results: " + detailsString, 150L * 60L * 1000L); break; case ConsensusMissingVotes: - warningStrings.put("The consensuses downloaded from the " - + "following authorities are missing votes that are " + warningStrings.put("NOTICE: The consensuses downloaded from " + + "the following authorities are missing votes that are " + "contained in consensuses downloaded from other " - + "authorities: " + details, 150L * 60L * 1000L); + + "authorities: " + detailsString, 150L * 60L * 1000L); break; case ConsensusMissingSignatures: - warningStrings.put("The consensuses downloaded from the " - + "following authorities are missing signatures from " - + "previously voting authorities: " + details, + warningStrings.put("NOTICE: The consensuses downloaded from " + + "the following authorities are missing signatures from " + + "previously voting authorities: " + detailsString, 150L * 60L * 1000L); break; } @@ -153,13 +166,20 @@ public class StatusFileReport { long now = System.currentTimeMillis(); StringBuilder allSb = new StringBuilder(), newSb = new StringBuilder(); - for (Map.Entry<String, Long> e : warningStrings.entrySet()) { - String message = e.getKey(); - allSb.append(message + "\n"); - long warnInterval = e.getValue(); - if (!lastWarned.containsKey(message) || - lastWarned.get(message) + warnInterval < now) { - newSb.append(message + "\n"); + List<String> severities = Arrays.asList(new String[] { + "ERROR", "WARNING", "NOTICE" }); + for (String severity : severities) { + for (Map.Entry<String, Long> e : warningStrings.entrySet()) { + String message = e.getKey(); + if (!message.startsWith(severity)) { + continue; + } + allSb.append(message + "\n"); + long warnInterval = e.getValue(); + if (!lastWarned.containsKey(message) || + lastWarned.get(message) + warnInterval < now) { + newSb.append(message + "\n"); + } } } if (newSb.length() > 0) {