commit 4333d0666dbb8a252bd85d1cc79ebd18dc84d4bd Author: Karsten Loesing karsten.loesing@gmx.net Date: Fri May 18 16:26:12 2018 +0200

Don't terminate the hidserv module when parsing descriptors.

This issue came up when reprocessing descriptor archives for #26022: we'd terminate the hidserv module when running into an issue with parsing descriptors, where a possible issue is a consensus without a "bandwidth-weights" line.

This seems wrong. It's okay to print out a warning in this case. But in general, we don't want to abort the entire module in this case.

Related to #26022. --- .../org/torproject/metrics/stats/hidserv/Main.java | 6 +----- .../torproject/metrics/stats/hidserv/Parser.java | 23 +++++++++------------- 2 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/src/main/java/org/torproject/metrics/stats/hidserv/Main.java b/src/main/java/org/torproject/metrics/stats/hidserv/Main.java index a23c17f..eafeeb1 100644 --- a/src/main/java/org/torproject/metrics/stats/hidserv/Main.java +++ b/src/main/java/org/torproject/metrics/stats/hidserv/Main.java @@ -38,11 +38,7 @@ public class Main { /* Parse new descriptors and store their contents using the document * stores. */ System.out.println("Parsing descriptors..."); - if (!parser.parseDescriptors()) { - System.err.println("Could not store parsed descriptors. " - + "Terminating."); - return; - } + parser.parseDescriptors();

/* Write the parse history to avoid parsing descriptor files again * next time. It's okay to do this now and not at the end of the diff --git a/src/main/java/org/torproject/metrics/stats/hidserv/Parser.java b/src/main/java/org/torproject/metrics/stats/hidserv/Parser.java index f2abc78..2423526 100644 --- a/src/main/java/org/torproject/metrics/stats/hidserv/Parser.java +++ b/src/main/java/org/torproject/metrics/stats/hidserv/Parser.java @@ -162,16 +162,14 @@ public class Parser { /** Instructs the descriptor reader to parse descriptor files, and * handles the resulting parsed descriptors if they are either * extra-info descriptors or consensuses. */ - public boolean parseDescriptors() { + public void parseDescriptors() { for (Descriptor descriptor : descriptorReader.readDescriptors( this.inDirectories)) { if (descriptor instanceof ExtraInfoDescriptor) { this.parseExtraInfoDescriptor((ExtraInfoDescriptor) descriptor); } else if (descriptor instanceof RelayNetworkStatusConsensus) { - if (!this.parseRelayNetworkStatusConsensus( - (RelayNetworkStatusConsensus) descriptor)) { - return false; - } + this.parseRelayNetworkStatusConsensus( + (RelayNetworkStatusConsensus) descriptor); } }

@@ -180,7 +178,7 @@ public class Parser { * descriptors. In contrast, sets of computed network fractions are * stored immediately after processing the consensus they are based * on. */ - return this.reportedHidServStatsStore.store( + this.reportedHidServStatsStore.store( this.reportedHidServStatsFile, this.reportedHidServStats); }

@@ -252,7 +250,7 @@ public class Parser { }

/** Parses the given consensus. */ - public boolean parseRelayNetworkStatusConsensus( + public void parseRelayNetworkStatusConsensus( RelayNetworkStatusConsensus consensus) {

/* Make sure that the consensus contains Wxx weights. */ @@ -262,7 +260,7 @@ public class Parser { System.err.printf("Consensus with valid-after time %s doesn't " + "contain any Wxx weights. Skipping.%n", DateTimeHelper.format(consensus.getValidAfterMillis())); - return false; + return; }

/* More precisely, make sure that it contains Wmx weights, and then @@ -274,7 +272,7 @@ public class Parser { System.err.printf("Consensus with valid-after time %s doesn't " + "contain expected Wmx weights. Skipping.%n", DateTimeHelper.format(consensus.getValidAfterMillis())); - return false; + return; } double wmg = ((double) bandwidthWeights.get("Wmg")) / 10000.0; double wmm = ((double) bandwidthWeights.get("Wmm")) / 10000.0; @@ -421,11 +419,8 @@ public class Parser { DateTimeHelper.ISO_DATE_FORMAT); File documentFile = new File(this.computedNetworkFractionsDirectory, date); - if (!this.computedNetworkFractionsStore.store(documentFile, - computedNetworkFractions)) { - return false; - } - return true; + this.computedNetworkFractionsStore.store(documentFile, + computedNetworkFractions); } }