[tor-commits] [metrics-lib/master] Fix the problems found while writing unit tests.
karsten at torproject.org
karsten at torproject.org
Thu Apr 26 14:15:27 UTC 2012
commit a287fa0c290fbe70de4f52c3c6b54df6b85ae8d1
Author: Karsten Loesing <karsten.loesing at gmx.net>
Date: Thu Apr 26 11:54:17 2012 +0200
Fix the problems found while writing unit tests.
---
.../descriptor/impl/BandwidthHistoryImpl.java | 8 +++
.../descriptor/impl/ExtraInfoDescriptorImpl.java | 58 ++++++++++++++++++++
.../torproject/descriptor/impl/ParseHelper.java | 6 +-
.../impl/ExtraInfoDescriptorImplTest.java | 36 +++++-------
4 files changed, 84 insertions(+), 24 deletions(-)
diff --git a/src/org/torproject/descriptor/impl/BandwidthHistoryImpl.java b/src/org/torproject/descriptor/impl/BandwidthHistoryImpl.java
index c696624..56ae512 100644
--- a/src/org/torproject/descriptor/impl/BandwidthHistoryImpl.java
+++ b/src/org/torproject/descriptor/impl/BandwidthHistoryImpl.java
@@ -26,6 +26,10 @@ public class BandwidthHistoryImpl implements BandwidthHistory {
partsNoOpt[4].equals("s)")) {
this.intervalLength = Long.parseLong(partsNoOpt[3].
substring(1));
+ if (this.intervalLength <= 0L) {
+ throw new DescriptorParseException("Only positive interval "
+ + "lengths are allowed in line '" + line + "'.");
+ }
if (partsNoOpt.length > 6) {
/* Invalid line, handle below. */
} else if (partsNoOpt.length == 5) {
@@ -36,6 +40,10 @@ public class BandwidthHistoryImpl implements BandwidthHistory {
String[] values = partsNoOpt[5].split(",", -1);
for (int i = values.length - 1; i >= 0; i--) {
long bandwidthValue = Long.parseLong(values[i]);
+ if (bandwidthValue < 0L) {
+ throw new DescriptorParseException("Negative bandwidth "
+ + "values are not allowed in line '" + line + "'.");
+ }
this.bandwidthValues.put(endMillis, bandwidthValue);
endMillis -= this.intervalLength * 1000L;
}
diff --git a/src/org/torproject/descriptor/impl/ExtraInfoDescriptorImpl.java b/src/org/torproject/descriptor/impl/ExtraInfoDescriptorImpl.java
index 3520934..184598a 100644
--- a/src/org/torproject/descriptor/impl/ExtraInfoDescriptorImpl.java
+++ b/src/org/torproject/descriptor/impl/ExtraInfoDescriptorImpl.java
@@ -5,6 +5,7 @@ package org.torproject.descriptor.impl;
import java.io.UnsupportedEncodingException;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
@@ -259,6 +260,10 @@ public class ExtraInfoDescriptorImpl extends DescriptorImpl
result[0] = ParseHelper.parseTimestampAtIndex(line, partsNoOpt, 1, 2);
result[1] = ParseHelper.parseSeconds(line,
partsNoOpt[3].substring(1));
+ if (result[1] <= 0) {
+ throw new DescriptorParseException("Interval length must be "
+ + "positive in line '" + line + "'.");
+ }
return result;
}
@@ -389,18 +394,30 @@ public class ExtraInfoDescriptorImpl extends DescriptorImpl
String[] partsNoOpt) throws DescriptorParseException {
this.cellProcessedCells = ParseHelper.
parseCommaSeparatedIntegerValueList(line, partsNoOpt, 1);
+ if (this.cellProcessedCells.size() != 10) {
+ throw new DescriptorParseException("There must be exact ten values "
+ + "in line '" + line + "'.");
+ }
}
private void parseCellQueuedCellsLine(String line, String lineNoOpt,
String[] partsNoOpt) throws DescriptorParseException {
this.cellQueuedCells = ParseHelper.parseCommaSeparatedDoubleValueList(
line, partsNoOpt, 1);
+ if (this.cellQueuedCells.size() != 10) {
+ throw new DescriptorParseException("There must be exact ten values "
+ + "in line '" + line + "'.");
+ }
}
private void parseCellTimeInQueueLine(String line, String lineNoOpt,
String[] partsNoOpt) throws DescriptorParseException {
this.cellTimeInQueue = ParseHelper.
parseCommaSeparatedIntegerValueList(line, partsNoOpt, 1);
+ if (this.cellTimeInQueue.size() != 10) {
+ throw new DescriptorParseException("There must be exact ten values "
+ + "in line '" + line + "'.");
+ }
}
private void parseCellCircuitsPerDecileLine(String line,
@@ -451,18 +468,24 @@ public class ExtraInfoDescriptorImpl extends DescriptorImpl
throws DescriptorParseException {
this.exitKibibytesWritten = this.sortByPorts(ParseHelper.
parseCommaSeparatedKeyValueList(line, partsNoOpt, 1, 0));
+ this.verifyPorts(line, this.exitKibibytesWritten.keySet());
+ this.verifyBytesOrStreams(line, this.exitKibibytesWritten.values());
}
private void parseExitKibibytesReadLine(String line, String lineNoOpt,
String[] partsNoOpt) throws DescriptorParseException {
this.exitKibibytesRead = this.sortByPorts(ParseHelper.
parseCommaSeparatedKeyValueList(line, partsNoOpt, 1, 0));
+ this.verifyPorts(line, this.exitKibibytesRead.keySet());
+ this.verifyBytesOrStreams(line, this.exitKibibytesRead.values());
}
private void parseExitStreamsOpenedLine(String line, String lineNoOpt,
String[] partsNoOpt) throws DescriptorParseException {
this.exitStreamsOpened = this.sortByPorts(ParseHelper.
parseCommaSeparatedKeyValueList(line, partsNoOpt, 1, 0));
+ this.verifyPorts(line, this.exitStreamsOpened.keySet());
+ this.verifyBytesOrStreams(line, this.exitStreamsOpened.values());
}
private SortedMap<String, Integer> sortByPorts(
@@ -493,6 +516,41 @@ public class ExtraInfoDescriptorImpl extends DescriptorImpl
return byPortNumber;
}
+ private void verifyPorts(String line, Set<String> ports)
+ throws DescriptorParseException {
+ boolean valid = true;
+ try {
+ for (String port : ports) {
+ if (!port.equals("other") && Integer.parseInt(port) <= 0) {
+ valid = false;
+ break;
+ }
+ }
+ } catch (NumberFormatException e) {
+ valid = false;
+ }
+ if (!valid) {
+ throw new DescriptorParseException("Invalid port in line '" + line
+ + "'.");
+ }
+ }
+
+ private void verifyBytesOrStreams(String line,
+ Collection<Integer> bytesOrStreams)
+ throws DescriptorParseException {
+ boolean valid = true;
+ for (int s : bytesOrStreams) {
+ if (s < 0) {
+ valid = false;
+ break;
+ }
+ }
+ if (!valid) {
+ throw new DescriptorParseException("Invalid value in line '" + line
+ + "'.");
+ }
+ }
+
private void parseBridgeStatsEndLine(String line, String lineNoOpt,
String[] partsNoOpt) throws DescriptorParseException {
long[] parsedStatsEndData = this.parseStatsEndLine(line, partsNoOpt,
diff --git a/src/org/torproject/descriptor/impl/ParseHelper.java b/src/org/torproject/descriptor/impl/ParseHelper.java
index daea018..a3f907d 100644
--- a/src/org/torproject/descriptor/impl/ParseHelper.java
+++ b/src/org/torproject/descriptor/impl/ParseHelper.java
@@ -218,7 +218,7 @@ public class ParseHelper {
+ "unrecognized values beyond the expected key-value list at "
+ "index " + index + ".");
} else if (partsNoOpt.length > index) {
- String[] listElements = partsNoOpt[index].split(",");
+ String[] listElements = partsNoOpt[index].split(",", -1);
for (String listElement : listElements) {
String[] keyAndValue = listElement.split("=");
String key = null;
@@ -256,7 +256,7 @@ public class ParseHelper {
+ "unrecognized values beyond the expected comma-separated "
+ "value list at index " + index + ".");
} else if (partsNoOpt.length > index) {
- String[] listElements = partsNoOpt[index].split(",");
+ String[] listElements = partsNoOpt[index].split(",", -1);
for (String listElement : listElements) {
try {
result.add(Integer.parseInt(listElement));
@@ -283,7 +283,7 @@ public class ParseHelper {
+ "unrecognized values beyond the expected comma-separated "
+ "value list at index " + index + ".");
} else if (partsNoOpt.length > index) {
- String[] listElements = partsNoOpt[index].split(",");
+ String[] listElements = partsNoOpt[index].split(",", -1);
for (String listElement : listElements) {
try {
result.add(Double.parseDouble(listElement));
diff --git a/test/org/torproject/descriptor/impl/ExtraInfoDescriptorImplTest.java b/test/org/torproject/descriptor/impl/ExtraInfoDescriptorImplTest.java
index c21a4ec..c8df3ce 100644
--- a/test/org/torproject/descriptor/impl/ExtraInfoDescriptorImplTest.java
+++ b/test/org/torproject/descriptor/impl/ExtraInfoDescriptorImplTest.java
@@ -746,9 +746,7 @@ public class ExtraInfoDescriptorImplTest {
assertEquals(1328951316000L, descriptor.getPublishedMillis());
}
- /* TODO This test should fail, even though dir-spec.txt doesn't
- * explicitly say that reported bytes must be non-negative. */
- @Test()
+ @Test(expected = DescriptorParseException.class)
public void testWriteHistoryNegativeBytes()
throws DescriptorParseException {
DescriptorBuilder.createWithWriteHistoryLine("write-history "
@@ -772,9 +770,7 @@ public class ExtraInfoDescriptorImplTest {
+ "4707695616,4699666432,4650004480,4489718784");
}
- /* TODO This test should fail, even though dir-spec.txt doesn't
- * explicitly say that the interval must be positive. */
- @Test()
+ @Test(expected = DescriptorParseException.class)
public void testReadHistoryNegativeInterval()
throws DescriptorParseException {
DescriptorBuilder.createWithReadHistoryLine("read-history "
@@ -918,9 +914,7 @@ public class ExtraInfoDescriptorImplTest {
+ "gb=208,ir=200");
}
- /* TODO Lines shouldn't be allowed to end with a comma. This also
- * applies to all other key-value pair lines. */
- @Test()
+ @Test(expected = DescriptorParseException.class)
public void testGeoipClientOriginsMissingEnd()
throws DescriptorParseException {
GeoipStatsBuilder.createWithGeoipClientOriginsLine(
@@ -1112,18 +1106,14 @@ public class ExtraInfoDescriptorImplTest {
+ "2012-02-11 01:59:39 (86400)");
}
- /* TODO Lines shouldn't be allowed to end with a comma. This also
- * applies to all other comma-separated value lines. */
- @Test()
+ @Test(expected = DescriptorParseException.class)
public void testCellProcessedCellsNineComma()
throws DescriptorParseException {
CellStatsBuilder.createWithCellProcessedCellsLine(
"cell-processed-cells 1441,11,6,4,2,1,1,1,1,");
}
- /* TODO We should check that there are really ten values, not more or
- * less. Applies to most cell-stats lines. */
- @Test()
+ @Test(expected = DescriptorParseException.class)
public void testCellProcessedCellsEleven()
throws DescriptorParseException {
CellStatsBuilder.createWithCellQueuedCellsLine("cell-queued-cells "
@@ -1207,16 +1197,21 @@ public class ExtraInfoDescriptorImplTest {
+ "2012-02-11 01:59 (86400 s)");
}
- /* TODO Negative ports should not be allowed. */
- @Test()
+ @Test(expected = DescriptorParseException.class)
public void testExitStatsWrittenNegativePort()
throws DescriptorParseException {
ExitStatsBuilder.createWithExitKibibytesWrittenLine(
"exit-kibibytes-written -25=74647");
}
- /* TODO Negative bytes should not be allowed. */
- @Test()
+ @Test(expected = DescriptorParseException.class)
+ public void testExitStatsWrittenUnknown()
+ throws DescriptorParseException {
+ ExitStatsBuilder.createWithExitKibibytesWrittenLine(
+ "exit-kibibytes-written unknown=74647");
+ }
+
+ @Test(expected = DescriptorParseException.class)
public void testExitStatsReadNegativeBytes()
throws DescriptorParseException {
ExitStatsBuilder.createWithExitKibibytesReadLine(
@@ -1243,8 +1238,7 @@ public class ExtraInfoDescriptorImplTest {
assertFalse(ips.containsKey("no"));
}
- /* TODO Only positive intervals should be allowed, for all stats. */
- @Test()
+ @Test(expected = DescriptorParseException.class)
public void testBridgeStatsEndIntervalZero()
throws DescriptorParseException {
BridgeStatsBuilder.createWithBridgeStatsEndLine("bridge-stats-end "
More information about the tor-commits
mailing list