commit a287fa0c290fbe70de4f52c3c6b54df6b85ae8d1 Author: Karsten Loesing <karsten.loesing@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 "