[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #19259 [Metrics/Onionoo]: uncaught NFE
#19259: uncaught NFE
-----------------------------+---------------------
Reporter: iwakeh | Owner:
Type: defect | Status: new
Priority: High | Milestone:
Component: Metrics/Onionoo | Version:
Severity: Major | Resolution:
Keywords: | Actual Points:
Parent ID: | Points:
Reviewer: | Sponsor:
-----------------------------+---------------------
Comment (by karsten):
Good catch (no pun intended) and good suggestions there. I started
looking through the code to find similar issues. Here's what I found:
{{{
diff --git
a/src/main/java/org/torproject/onionoo/docs/BandwidthStatus.java
b/src/main/java/org/torproject/onionoo/docs/BandwidthStatus.java
index 06f295b..9e4664c 100644
--- a/src/main/java/org/torproject/onionoo/docs/BandwidthStatus.java
+++ b/src/main/java/org/torproject/onionoo/docs/BandwidthStatus.java
@@ -62,7 +62,14 @@ public class BandwidthStatus extends Document {
+ "bandwidth history. Skipping.");
break;
}
- long bandwidth = Long.parseLong(parts[5]);
+ long bandwidth;
+ try {
+ bandwidth = Long.parseLong(parts[5]);
+ } catch (NumberFormatException e) {
+ log.error("Could not parse bandwidth value while reading "
+ + "bandwidth history. Skipping.");
+ break;
+ }
long previousEndMillis = history.headMap(startMillis).isEmpty()
? startMillis
: history.get(history.headMap(startMillis).lastKey())[1];
diff --git a/src/main/java/org/torproject/onionoo/docs/WeightsStatus.java
b/src/main/java/org/torproject/onionoo/docs/WeightsStatus.java
index 5d7f23e..b663fc4 100644
--- a/src/main/java/org/torproject/onionoo/docs/WeightsStatus.java
+++ b/src/main/java/org/torproject/onionoo/docs/WeightsStatus.java
@@ -67,13 +67,20 @@ public class WeightsStatus extends Document {
}
long[] interval = new long[] { validAfterMillis,
freshUntilMillis };
- double[] weights = new double[] { -1.0,
- Double.parseDouble(parts[5]),
- Double.parseDouble(parts[6]),
- Double.parseDouble(parts[7]),
- Double.parseDouble(parts[8]), -1.0, -1.0 };
- if (parts.length == 11) {
- weights[6] = Double.parseDouble(parts[10]);
+ double[] weights;
+ try {
+ weights = new double[] { -1.0,
+ Double.parseDouble(parts[5]),
+ Double.parseDouble(parts[6]),
+ Double.parseDouble(parts[7]),
+ Double.parseDouble(parts[8]), -1.0, -1.0 };
+ if (parts.length == 11) {
+ weights[6] = Double.parseDouble(parts[10]);
+ }
+ } catch (NumberFormatException e) {
+ log.error("Could not parse weights values in line '" + line
+ + "' while reading weights status file. Skipping.");
+ break;
}
this.history.put(interval, weights);
}
}}}
Of course, we'll also need to fix the parts where strings are written
using the system locale rather than `Locale.US`.
And then we'll need unit tests for these. Would you be able to write one
of those that breaks with the current code?
However, one thing I don't necessarily agree with: Not every `try` needs a
`catch`, if we're only using the `try` as try-with-resources. For
example, `NumberFormatException` is a runtime exception, and in some cases
it's simply not possible for one of those to be thrown. So there may be
valid cases for using `try` without `catch`. We might want to add a
comment to those.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19259#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs