[tor-commits] [onionoo/master] Avoid silently exiting methods.
karsten at torproject.org
karsten at torproject.org
Mon Dec 8 12:22:56 UTC 2014
commit 817a29c62670468170094f891c3ffc0c7c4c448e
Author: Karsten Loesing <karsten.loesing at gmx.net>
Date: Mon Dec 8 11:22:32 2014 +0100
Avoid silently exiting methods.
Whenever we return from a method before it ends, we should either log a
warning or leave a comment why this is intended behavior.
Suggested by iwakeh on #12651.
---
.../torproject/onionoo/docs/ClientsHistory.java | 20 ++++++++++++++++++++
.../org/torproject/onionoo/docs/DocumentStore.java | 16 ++++++++++++++++
.../org/torproject/onionoo/docs/UptimeHistory.java | 18 ++++++++++++++++--
.../org/torproject/onionoo/server/NodeIndexer.java | 1 +
.../torproject/onionoo/server/RequestHandler.java | 14 ++++++++++++++
.../torproject/onionoo/server/ResourceServlet.java | 13 +++++++++++++
.../onionoo/updater/ClientsStatusUpdater.java | 3 +++
.../onionoo/updater/DescriptorQueue.java | 2 ++
.../onionoo/updater/NodeDetailsStatusUpdater.java | 2 ++
.../onionoo/writer/ClientsDocumentWriter.java | 3 +++
.../onionoo/writer/UptimeDocumentWriter.java | 4 ++++
.../onionoo/writer/WeightsDocumentWriter.java | 3 +++
12 files changed, 97 insertions(+), 2 deletions(-)
diff --git a/src/main/java/org/torproject/onionoo/docs/ClientsHistory.java b/src/main/java/org/torproject/onionoo/docs/ClientsHistory.java
index 35a063a..effddf5 100644
--- a/src/main/java/org/torproject/onionoo/docs/ClientsHistory.java
+++ b/src/main/java/org/torproject/onionoo/docs/ClientsHistory.java
@@ -6,8 +6,14 @@ import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
public class ClientsHistory implements Comparable<ClientsHistory> {
+ private final static Logger log = LoggerFactory.getLogger(
+ ClientsHistory.class);
+
private long startMillis;
public long getStartMillis() {
return this.startMillis;
@@ -55,20 +61,29 @@ public class ClientsHistory implements Comparable<ClientsHistory> {
String responseHistoryString) {
String[] parts = responseHistoryString.split(" ", 8);
if (parts.length != 8) {
+ log.warn("Invalid number of space-separated strings in clients "
+ + "history: '" + responseHistoryString + "'. Skipping");
return null;
}
long startMillis = DateTimeHelper.parse(parts[0] + " " + parts[1]);
long endMillis = DateTimeHelper.parse(parts[2] + " " + parts[3]);
if (startMillis < 0L || endMillis < 0L) {
+ log.warn("Invalid start or end timestamp in clients history: '"
+ + responseHistoryString + "'. Skipping.");
return null;
}
if (startMillis >= endMillis) {
+ log.warn("Start timestamp must be smaller than end timestamp in "
+ + "clients history: '" + responseHistoryString
+ + "'. Skipping.");
return null;
}
double totalResponses = 0.0;
try {
totalResponses = Double.parseDouble(parts[4]);
} catch (NumberFormatException e) {
+ log.warn("Invalid response number format in clients history: '"
+ + responseHistoryString + "'. Skipping.");
return null;
}
SortedMap<String, Double> responsesByCountry =
@@ -79,6 +94,9 @@ public class ClientsHistory implements Comparable<ClientsHistory> {
parseResponses(parts[7]);
if (responsesByCountry == null || responsesByTransport == null ||
responsesByVersion == null) {
+ log.warn("Invalid format of responses by country, transport, or "
+ + "version in clients history: '" + responseHistoryString
+ + "'. Skipping.");
return null;
}
return new ClientsHistory(startMillis, endMillis, totalResponses,
@@ -92,12 +110,14 @@ public class ClientsHistory implements Comparable<ClientsHistory> {
for (String pair : responsesString.split(",")) {
String[] keyValue = pair.split("=");
if (keyValue.length != 2) {
+ /* Logged by caller */
return null;
}
double value = 0.0;
try {
value = Double.parseDouble(keyValue[1]);
} catch (NumberFormatException e) {
+ /* Logged by caller */
return null;
}
responses.put(keyValue[0], value);
diff --git a/src/main/java/org/torproject/onionoo/docs/DocumentStore.java b/src/main/java/org/torproject/onionoo/docs/DocumentStore.java
index 4b7bca5..a880e53 100644
--- a/src/main/java/org/torproject/onionoo/docs/DocumentStore.java
+++ b/src/main/java/org/torproject/onionoo/docs/DocumentStore.java
@@ -393,6 +393,9 @@ public class DocumentStore {
DetailsDocument detailsDocument = this.retrieveDocumentFile(
DetailsDocument.class, true, fingerprint);
if (detailsDocument == null) {
+ /* There is no details document available that we could serve as
+ * basis for generating a summary document on-the-fly. Nothing to
+ * worry about. */
return null;
}
boolean isRelay = detailsDocument.getHashedFingerprint() == null;
@@ -402,6 +405,10 @@ public class DocumentStore {
String countryCode = null, aSNumber = null, contact = null;
for (String orAddressAndPort : detailsDocument.getOrAddresses()) {
if (!orAddressAndPort.contains(":")) {
+ log.warn("Attempt to create summary document from details "
+ + "document for fingerprint " + fingerprint + " failed "
+ + "because of invalid OR address/port: '" + orAddressAndPort
+ + "'. Not returning a summary document in this case.");
return null;
}
String orAddress = orAddressAndPort.substring(0,
@@ -431,6 +438,7 @@ public class DocumentStore {
Class<T> documentType, boolean parse, String fingerprint) {
File documentFile = this.getDocumentFile(documentType, fingerprint);
if (documentFile == null || !documentFile.exists()) {
+ /* Document file does not exist. That's okay. */
return null;
} else if (documentFile.isDirectory()) {
log.error("Could not read file '"
@@ -451,6 +459,7 @@ public class DocumentStore {
bis.close();
byte[] allData = baos.toByteArray();
if (allData.length == 0) {
+ /* Document file is empty. */
return null;
}
documentString = new String(allData, "US-ASCII");
@@ -598,6 +607,9 @@ public class DocumentStore {
File documentFile = null;
if (fingerprint == null && !documentType.equals(UpdateStatus.class) &&
!documentType.equals(UptimeStatus.class)) {
+ log.warn("Attempted to locate a document file of type "
+ + documentType.getName() + " without providing a fingerprint. "
+ + "Such a file does not exist.");
return null;
}
File directory = null;
@@ -685,6 +697,8 @@ public class DocumentStore {
private void writeNodeStatuses() {
File directory = this.statusDir;
if (directory == null) {
+ log.error("Unable to write node statuses without knowing the "
+ + "'status' directory to write to!");
return;
}
File summaryFile = new File(directory, "summary");
@@ -766,6 +780,8 @@ public class DocumentStore {
private void writeUpdateStatus() {
if (this.outDir == null) {
+ log.error("Unable to write update status file without knowing the "
+ + "'out' directory to write to!");
return;
}
UpdateStatus updateStatus = new UpdateStatus();
diff --git a/src/main/java/org/torproject/onionoo/docs/UptimeHistory.java b/src/main/java/org/torproject/onionoo/docs/UptimeHistory.java
index 0d27242..b686c7e 100644
--- a/src/main/java/org/torproject/onionoo/docs/UptimeHistory.java
+++ b/src/main/java/org/torproject/onionoo/docs/UptimeHistory.java
@@ -1,8 +1,13 @@
package org.torproject.onionoo.docs;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
-public class UptimeHistory
- implements Comparable<UptimeHistory> {
+
+public class UptimeHistory implements Comparable<UptimeHistory> {
+
+ private final static Logger log = LoggerFactory.getLogger(
+ UptimeHistory.class);
private boolean relay;
public boolean isRelay() {
@@ -29,23 +34,32 @@ public class UptimeHistory
public static UptimeHistory fromString(String uptimeHistoryString) {
String[] parts = uptimeHistoryString.split(" ", 3);
if (parts.length != 3) {
+ log.warn("Invalid number of space-separated strings in uptime "
+ + "history: '" + uptimeHistoryString + "'. Skipping");
return null;
}
boolean relay = false;
if (parts[0].equals("r")) {
relay = true;
} else if (!parts[0].equals("b")) {
+ log.warn("Invalid node type in uptime history: '"
+ + uptimeHistoryString + "'. Supported types are 'r' and 'b'. "
+ + "Skipping.");
return null;
}
long startMillis = DateTimeHelper.parse(parts[1],
DateTimeHelper.DATEHOUR_NOSPACE_FORMAT);
if (DateTimeHelper.NO_TIME_AVAILABLE == startMillis) {
+ log.warn("Invalid start timestamp in uptime history: '"
+ + uptimeHistoryString + "'. Skipping.");
return null;
}
int uptimeHours = -1;
try {
uptimeHours = Integer.parseInt(parts[2]);
} catch (NumberFormatException e) {
+ log.warn("Invalid number format in uptime history: '"
+ + uptimeHistoryString + "'. Skipping.");
return null;
}
return new UptimeHistory(relay, startMillis, uptimeHours);
diff --git a/src/main/java/org/torproject/onionoo/server/NodeIndexer.java b/src/main/java/org/torproject/onionoo/server/NodeIndexer.java
index f8870db..1a67cc5 100644
--- a/src/main/java/org/torproject/onionoo/server/NodeIndexer.java
+++ b/src/main/java/org/torproject/onionoo/server/NodeIndexer.java
@@ -105,6 +105,7 @@ public class NodeIndexer implements ServletContextListener, Runnable {
}
synchronized (this) {
if (updateStatusMillis <= this.lastIndexed) {
+ /* Index on disk is no more recent than the one in memory. */
return;
}
}
diff --git a/src/main/java/org/torproject/onionoo/server/RequestHandler.java b/src/main/java/org/torproject/onionoo/server/RequestHandler.java
index 28b5531..2d8dd58 100644
--- a/src/main/java/org/torproject/onionoo/server/RequestHandler.java
+++ b/src/main/java/org/torproject/onionoo/server/RequestHandler.java
@@ -154,6 +154,7 @@ public class RequestHandler {
private void filterByType() {
if (this.type == null) {
+ /* Not filtering by type. */
return;
} else if (this.type.equals("relay")) {
this.filteredBridges.clear();
@@ -164,6 +165,7 @@ public class RequestHandler {
private void filterByRunning() {
if (this.running == null) {
+ /* Not filtering by running or not. */
return;
}
boolean runningRequested = this.running.equals("true");
@@ -191,6 +193,7 @@ public class RequestHandler {
private void filterBySearchTerms() {
if (this.search == null) {
+ /* Not filtering by search terms. */
return;
}
for (String searchTerm : this.search) {
@@ -276,6 +279,7 @@ public class RequestHandler {
private void filterByLookup() {
if (this.lookup == null) {
+ /* Not filtering by looking up relay or bridge. */
return;
}
String fingerprint = this.lookup;
@@ -293,6 +297,7 @@ public class RequestHandler {
private void filterByFingerprint() {
if (this.fingerprint == null) {
+ /* Not filtering by fingerprint. */
return;
}
this.filteredRelays.clear();
@@ -311,6 +316,7 @@ public class RequestHandler {
private void filterByCountryCode() {
if (this.country == null) {
+ /* Not filtering by country code. */
return;
}
String countryCode = this.country.toLowerCase();
@@ -335,6 +341,7 @@ public class RequestHandler {
private void filterByASNumber() {
if (this.as == null) {
+ /* Not filtering by AS number. */
return;
}
String aSNumber = this.as.toUpperCase();
@@ -361,6 +368,7 @@ public class RequestHandler {
private void filterByFlag() {
if (this.flag == null) {
+ /* Not filtering by relay flag. */
return;
}
String flag = this.flag.toLowerCase();
@@ -398,6 +406,7 @@ public class RequestHandler {
private void filterNodesByFirstSeenDays() {
if (this.firstSeenDays == null) {
+ /* Not filtering by first-seen days. */
return;
}
filterNodesByDays(this.filteredRelays,
@@ -408,6 +417,7 @@ public class RequestHandler {
private void filterNodesByLastSeenDays() {
if (this.lastSeenDays == null) {
+ /* Not filtering by last-seen days. */
return;
}
filterNodesByDays(this.filteredRelays,
@@ -436,6 +446,7 @@ public class RequestHandler {
private void filterByContact() {
if (this.contact == null) {
+ /* Not filtering by contact information. */
return;
}
Set<String> removeRelays = new HashSet<String>();
@@ -458,6 +469,7 @@ public class RequestHandler {
private void filterByFamily() {
if (this.family == null) {
+ /* Not filtering by relay family. */
return;
}
Set<String> removeRelays = new HashSet<String>(
@@ -506,6 +518,7 @@ public class RequestHandler {
private void offset() {
if (this.offset == null) {
+ /* Not skipping first results. */
return;
}
int offsetValue = Integer.parseInt(this.offset);
@@ -522,6 +535,7 @@ public class RequestHandler {
private void limit() {
if (this.limit == null) {
+ /* Not limiting number of results. */
return;
}
int limitValue = Integer.parseInt(this.limit);
diff --git a/src/main/java/org/torproject/onionoo/server/ResourceServlet.java b/src/main/java/org/torproject/onionoo/server/ResourceServlet.java
index f6f060a..6182704 100644
--- a/src/main/java/org/torproject/onionoo/server/ResourceServlet.java
+++ b/src/main/java/org/torproject/onionoo/server/ResourceServlet.java
@@ -372,6 +372,7 @@ public class ResourceServlet extends HttpServlet {
Matcher searchQueryStringMatcher = searchQueryStringPattern.matcher(
queryString);
if (!searchQueryStringMatcher.matches()) {
+ /* Search query contains illegal character(s). */
return null;
}
String parameter = searchQueryStringMatcher.group(1);
@@ -383,6 +384,7 @@ public class ResourceServlet extends HttpServlet {
}
for (String searchParameter : searchParameters) {
if (!searchParameterPattern.matcher(searchParameter).matches()) {
+ /* Illegal search term. */
return null;
}
}
@@ -393,9 +395,11 @@ public class ResourceServlet extends HttpServlet {
Pattern.compile("^[0-9a-zA-Z]{1,40}$");
private String parseFingerprintParameter(String parameter) {
if (!fingerprintParameterPattern.matcher(parameter).matches()) {
+ /* Fingerprint contains non-hex character(s). */
return null;
}
if (parameter.length() != 40) {
+ /* Only full fingerprints are accepted. */
return null;
}
return parameter;
@@ -405,6 +409,8 @@ public class ResourceServlet extends HttpServlet {
Pattern.compile("^[0-9a-zA-Z]{2}$");
private String parseCountryCodeParameter(String parameter) {
if (!countryCodeParameterPattern.matcher(parameter).matches()) {
+ /* Country code contains illegal characters or is shorter/longer
+ * than 2 characters. */
return null;
}
return parameter;
@@ -414,6 +420,7 @@ public class ResourceServlet extends HttpServlet {
Pattern.compile("^[asAS]{0,2}[0-9]{1,10}$");
private String parseASNumberParameter(String parameter) {
if (!aSNumberParameterPattern.matcher(parameter).matches()) {
+ /* AS number contains illegal character(s). */
return null;
}
return parameter;
@@ -423,6 +430,7 @@ public class ResourceServlet extends HttpServlet {
Pattern.compile("^[a-zA-Z0-9]{1,20}$");
private String parseFlagParameter(String parameter) {
if (!flagPattern.matcher(parameter).matches()) {
+ /* Flag contains illegal character(s). */
return null;
}
return parameter;
@@ -431,6 +439,7 @@ public class ResourceServlet extends HttpServlet {
private static Pattern daysPattern = Pattern.compile("^[0-9-]{1,10}$");
private int[] parseDaysParameter(String parameter) {
if (!daysPattern.matcher(parameter).matches()) {
+ /* Days contain illegal character(s). */
return null;
}
int x = 0, y = Integer.MAX_VALUE;
@@ -448,9 +457,11 @@ public class ResourceServlet extends HttpServlet {
}
}
} catch (NumberFormatException e) {
+ /* Invalid format. */
return null;
}
if (x > y) {
+ /* Second number or days must exceed first number. */
return null;
}
return new int[] { x, y };
@@ -459,6 +470,7 @@ public class ResourceServlet extends HttpServlet {
private String[] parseContactParameter(String parameter) {
for (char c : parameter.toCharArray()) {
if (c < 32 || c >= 127) {
+ /* Only accept printable ASCII. */
return null;
}
}
@@ -469,6 +481,7 @@ public class ResourceServlet extends HttpServlet {
Pattern.compile("^[0-9a-zA-Z_,]*$");
private String[] parseFieldsParameter(String parameter) {
if (!fieldsParameterPattern.matcher(parameter).matches()) {
+ /* Fields contain illegal character(s). */
return null;
}
return parameter.split(",");
diff --git a/src/main/java/org/torproject/onionoo/updater/ClientsStatusUpdater.java b/src/main/java/org/torproject/onionoo/updater/ClientsStatusUpdater.java
index f3f2e0f..72f5a92 100644
--- a/src/main/java/org/torproject/onionoo/updater/ClientsStatusUpdater.java
+++ b/src/main/java/org/torproject/onionoo/updater/ClientsStatusUpdater.java
@@ -73,10 +73,13 @@ public class ClientsStatusUpdater implements DescriptorListener,
if (dirreqStatsEndMillis < 0L ||
dirreqStatsIntervalLengthMillis != DateTimeHelper.ONE_DAY ||
responses == null || !responses.containsKey("ok")) {
+ /* No directory request responses in the descriptor that we would
+ * include in a clients document. */
return;
}
double okResponses = (double) (responses.get("ok") - 4);
if (okResponses < 0.0) {
+ /* Response numbers are not supposed to be negative. Skipping. */
return;
}
String hashedFingerprint = descriptor.getFingerprint().toUpperCase();
diff --git a/src/main/java/org/torproject/onionoo/updater/DescriptorQueue.java b/src/main/java/org/torproject/onionoo/updater/DescriptorQueue.java
index 8ba4260..93a291c 100644
--- a/src/main/java/org/torproject/onionoo/updater/DescriptorQueue.java
+++ b/src/main/java/org/torproject/onionoo/updater/DescriptorQueue.java
@@ -168,6 +168,8 @@ class DescriptorQueue {
public void writeHistoryFile() {
if (this.historyFile == null) {
+ log.error("Unable to write history file, possibly because of an "
+ + "unknown descriptor type. Skipping.");
return;
}
SortedMap<String, Long> excludedAndParsedFiles =
diff --git a/src/main/java/org/torproject/onionoo/updater/NodeDetailsStatusUpdater.java b/src/main/java/org/torproject/onionoo/updater/NodeDetailsStatusUpdater.java
index 935aa4e..ef87233 100644
--- a/src/main/java/org/torproject/onionoo/updater/NodeDetailsStatusUpdater.java
+++ b/src/main/java/org/torproject/onionoo/updater/NodeDetailsStatusUpdater.java
@@ -150,6 +150,7 @@ public class NodeDetailsStatusUpdater implements DescriptorListener,
} else if (detailsStatus.getDescPublished() != null &&
detailsStatus.getDescPublished() >=
descriptor.getPublishedMillis()) {
+ /* Already parsed more recent server descriptor from this relay. */
return;
}
long lastRestartedMillis = descriptor.getPublishedMillis()
@@ -276,6 +277,7 @@ public class NodeDetailsStatusUpdater implements DescriptorListener,
detailsStatus = new DetailsStatus();
} else if (descriptor.getPublishedMillis() <=
detailsStatus.getDescPublished()) {
+ /* Already parsed more recent server descriptor from this bridge. */
return;
}
long lastRestartedMillis = descriptor.getPublishedMillis()
diff --git a/src/main/java/org/torproject/onionoo/writer/ClientsDocumentWriter.java b/src/main/java/org/torproject/onionoo/writer/ClientsDocumentWriter.java
index 885494c..84d5a30 100644
--- a/src/main/java/org/torproject/onionoo/writer/ClientsDocumentWriter.java
+++ b/src/main/java/org/torproject/onionoo/writer/ClientsDocumentWriter.java
@@ -198,6 +198,7 @@ public class ClientsDocumentWriter implements DocumentWriter {
}
}
if (firstNonNullIndex < 0) {
+ /* Not a single non-negative value in the data points. */
return null;
}
long firstDataPointMillis = (((this.now - graphInterval)
@@ -273,6 +274,8 @@ public class ClientsDocumentWriter implements DocumentWriter {
if (foundTwoAdjacentDataPoints) {
return graphHistory;
} else {
+ /* There are no two adjacent values in the data points that are
+ * required to draw a line graph. */
return null;
}
}
diff --git a/src/main/java/org/torproject/onionoo/writer/UptimeDocumentWriter.java b/src/main/java/org/torproject/onionoo/writer/UptimeDocumentWriter.java
index 093bc98..c5f71a1 100644
--- a/src/main/java/org/torproject/onionoo/writer/UptimeDocumentWriter.java
+++ b/src/main/java/org/torproject/onionoo/writer/UptimeDocumentWriter.java
@@ -39,6 +39,7 @@ public class UptimeDocumentWriter implements DocumentWriter {
UptimeStatus uptimeStatus = this.documentStore.retrieve(
UptimeStatus.class, true);
if (uptimeStatus == null) {
+ /* No global uptime information available. */
return;
}
UpdateStatus updateStatus = this.documentStore.retrieve(
@@ -233,6 +234,7 @@ public class UptimeDocumentWriter implements DocumentWriter {
}
}
if (firstNonNullIndex < 0) {
+ /* Not a single non-negative value in the data points. */
return null;
}
long firstDataPointMillis = (((this.now - graphInterval)
@@ -273,6 +275,8 @@ public class UptimeDocumentWriter implements DocumentWriter {
if (foundTwoAdjacentDataPoints) {
return graphHistory;
} else {
+ /* There are no two adjacent values in the data points that are
+ * required to draw a line graph. */
return null;
}
}
diff --git a/src/main/java/org/torproject/onionoo/writer/WeightsDocumentWriter.java b/src/main/java/org/torproject/onionoo/writer/WeightsDocumentWriter.java
index 7d52a47..62ff533 100644
--- a/src/main/java/org/torproject/onionoo/writer/WeightsDocumentWriter.java
+++ b/src/main/java/org/torproject/onionoo/writer/WeightsDocumentWriter.java
@@ -157,6 +157,7 @@ public class WeightsDocumentWriter implements DocumentWriter {
}
}
if (firstNonNullIndex < 0) {
+ /* Not a single non-negative value in the data points. */
return null;
}
long firstDataPointMillis = (((this.now - graphInterval)
@@ -199,6 +200,8 @@ public class WeightsDocumentWriter implements DocumentWriter {
if (foundTwoAdjacentDataPoints) {
return graphHistory;
} else {
+ /* There are no two adjacent values in the data points that are
+ * required to draw a line graph. */
return null;
}
}
More information about the tor-commits
mailing list