commit 71885a8e694b2a37b8a868389e4ea6d1ac037b30 Author: Karsten Loesing <karsten.loesing@gmx.net> Date: Wed May 3 09:41:10 2017 +0200 Accept extra arguments in extra-info descriptors. According to the specification it's valid to add extra arguments to descriptor lines unless they are tagged with "[No extra arguments]". This is not the case for any of the statistics-related lines in extra-info descriptors, so we should allow extra arguments there. Fixes #21934. --- CHANGELOG.md | 4 + .../descriptor/impl/BandwidthHistoryImpl.java | 4 +- .../descriptor/impl/ExtraInfoDescriptorImpl.java | 12 +- .../torproject/descriptor/impl/ParseHelper.java | 16 --- .../impl/ExtraInfoDescriptorImplTest.java | 127 +++++++++++++++++++++ .../descriptor/impl/ServerDescriptorImplTest.java | 6 +- 6 files changed, 142 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da9cb37..a7f0aac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ below crypto blocks were silently skipped. - Add support for six new key-value pairs added by OnionPerf. + * Minor changes + - Accept extra arguments in statistics-related extra-info + descriptor lines, as permitted by dir-spec.txt. + # Changes in version 1.6.0 - 2017-02-17 diff --git a/src/main/java/org/torproject/descriptor/impl/BandwidthHistoryImpl.java b/src/main/java/org/torproject/descriptor/impl/BandwidthHistoryImpl.java index 8f18b62..ef29a3b 100644 --- a/src/main/java/org/torproject/descriptor/impl/BandwidthHistoryImpl.java +++ b/src/main/java/org/torproject/descriptor/impl/BandwidthHistoryImpl.java @@ -15,7 +15,7 @@ public class BandwidthHistoryImpl implements BandwidthHistory { String[] partsNoOpt) throws DescriptorParseException { boolean isValid = false; this.line = line; - if (partsNoOpt.length == 5 || partsNoOpt.length == 6) { + if (partsNoOpt.length >= 5) { try { this.historyEndMillis = ParseHelper.parseTimestampAtIndex(line, partsNoOpt, 1, 2); @@ -32,7 +32,7 @@ public class BandwidthHistoryImpl implements BandwidthHistory { && partsNoOpt[4].equals("s)")) { /* There are no bandwidth values to parse. */ isValid = true; - } else if (partsNoOpt.length == 6) { + } else if (partsNoOpt.length >= 6) { /* There are bandwidth values to parse. */ values = partsNoOpt[5].split(",", -1); } else if (partsNoOpt[4].length() > 2) { diff --git a/src/main/java/org/torproject/descriptor/impl/ExtraInfoDescriptorImpl.java b/src/main/java/org/torproject/descriptor/impl/ExtraInfoDescriptorImpl.java index 136f31e..bbaae19 100644 --- a/src/main/java/org/torproject/descriptor/impl/ExtraInfoDescriptorImpl.java +++ b/src/main/java/org/torproject/descriptor/impl/ExtraInfoDescriptorImpl.java @@ -324,7 +324,7 @@ public abstract class ExtraInfoDescriptorImpl extends DescriptorImpl private void parseGeoipDbDigestLine(String line, String lineNoOpt, String[] partsNoOpt) throws DescriptorParseException { - if (partsNoOpt.length != 2) { + if (partsNoOpt.length < 2) { throw new DescriptorParseException("Illegal line '" + line + "' in extra-info descriptor."); } @@ -334,7 +334,7 @@ public abstract class ExtraInfoDescriptorImpl extends DescriptorImpl private void parseGeoip6DbDigestLine(String line, String lineNoOpt, String[] partsNoOpt) throws DescriptorParseException { - if (partsNoOpt.length != 2) { + if (partsNoOpt.length < 2) { throw new DescriptorParseException("Illegal line '" + line + "' in extra-info descriptor."); } @@ -344,7 +344,7 @@ public abstract class ExtraInfoDescriptorImpl extends DescriptorImpl private void parseGeoipStartTimeLine(String line, String lineNoOpt, String[] partsNoOpt) throws DescriptorParseException { - if (partsNoOpt.length != 3) { + if (partsNoOpt.length < 3) { throw new DescriptorParseException("Illegal line '" + line + "' in extra-info descriptor."); } @@ -369,7 +369,7 @@ public abstract class ExtraInfoDescriptorImpl extends DescriptorImpl private long[] parseStatsEndLine(String line, String[] partsNoOpt, int partsNoOptExpectedLength) throws DescriptorParseException { - if (partsNoOpt.length != partsNoOptExpectedLength + if (partsNoOpt.length < partsNoOptExpectedLength || partsNoOpt[3].length() < 2 || !partsNoOpt[3].startsWith("(") || !partsNoOpt[4].equals("s)")) { throw new DescriptorParseException("Illegal line '" + line + "'."); @@ -424,7 +424,7 @@ public abstract class ExtraInfoDescriptorImpl extends DescriptorImpl private double parseShareLine(String line, String[] partsNoOpt) throws DescriptorParseException { double share = -1.0; - if (partsNoOpt.length == 2 && partsNoOpt[1].length() >= 2 + if (partsNoOpt.length >= 2 && partsNoOpt[1].length() >= 2 && partsNoOpt[1].endsWith("%")) { String shareString = partsNoOpt[1]; shareString = shareString.substring(0, shareString.length() - 1); @@ -550,7 +550,7 @@ public abstract class ExtraInfoDescriptorImpl extends DescriptorImpl String lineNoOpt, String[] partsNoOpt) throws DescriptorParseException { int circuits = -1; - if (partsNoOpt.length == 2) { + if (partsNoOpt.length >= 2) { try { circuits = Integer.parseInt(partsNoOpt[1]); } catch (NumberFormatException e) { diff --git a/src/main/java/org/torproject/descriptor/impl/ParseHelper.java b/src/main/java/org/torproject/descriptor/impl/ParseHelper.java index f73a591..fa6ca75 100644 --- a/src/main/java/org/torproject/descriptor/impl/ParseHelper.java +++ b/src/main/java/org/torproject/descriptor/impl/ParseHelper.java @@ -341,10 +341,6 @@ public class ParseHelper { if (partsNoOpt.length < index) { throw new DescriptorParseException("Line '" + line + "' does not " + "contain a key-value list at index " + index + "."); - } else if (partsNoOpt.length > index + 1 ) { - throw new DescriptorParseException("Line '" + line + "' contains " - + "unrecognized values beyond the expected key-value list at " - + "index " + index + "."); } else if (partsNoOpt.length > index) { if (!commaSeparatedKeyValueListPatterns.containsKey(keyLength)) { String keyPattern = "[0-9a-zA-Z?<>\\-_]" @@ -390,10 +386,6 @@ public class ParseHelper { if (partsNoOpt.length < index) { throw new DescriptorParseException("Line '" + line + "' does not " + "contain a key-value list at index " + index + "."); - } else if (partsNoOpt.length > index + 1 ) { - throw new DescriptorParseException("Line '" + line + "' contains " - + "unrecognized values beyond the expected key-value list at " - + "index " + index + "."); } else if (partsNoOpt.length > index) { String[] listElements = partsNoOpt[index].split(",", -1); for (String listElement : listElements) { @@ -428,10 +420,6 @@ public class ParseHelper { throw new DescriptorParseException("Line '" + line + "' does not " + "contain a comma-separated value list at index " + index + "."); - } else if (partsNoOpt.length > index + 1) { - throw new DescriptorParseException("Line '" + line + "' contains " - + "unrecognized values beyond the expected comma-separated " - + "value list at index " + index + "."); } else if (partsNoOpt.length > index) { String[] listElements = partsNoOpt[index].split(",", -1); result = new Integer[listElements.length]; @@ -456,10 +444,6 @@ public class ParseHelper { throw new DescriptorParseException("Line '" + line + "' does not " + "contain a comma-separated value list at index " + index + "."); - } else if (partsNoOpt.length > index + 1) { - throw new DescriptorParseException("Line '" + line + "' contains " - + "unrecognized values beyond the expected comma-separated " - + "value list at index " + index + "."); } else if (partsNoOpt.length > index) { String[] listElements = partsNoOpt[index].split(",", -1); result = new Double[listElements.length]; diff --git a/src/test/java/org/torproject/descriptor/impl/ExtraInfoDescriptorImplTest.java b/src/test/java/org/torproject/descriptor/impl/ExtraInfoDescriptorImplTest.java index c29df8c..118ada8 100644 --- a/src/test/java/org/torproject/descriptor/impl/ExtraInfoDescriptorImplTest.java +++ b/src/test/java/org/torproject/descriptor/impl/ExtraInfoDescriptorImplTest.java @@ -1093,6 +1093,14 @@ public class ExtraInfoDescriptorImplTest { + "81281024,,60625920,67922944"); } + @Test() + public void testDirreqWriteHistoryExtraArg() throws DescriptorParseException { + DescriptorBuilder.createWithDirreqWriteHistoryLine( + "dirreq-write-history " + + "2012-02-11 09:03:39 (900 s) 81281024,64996352,60625920," + + "67922944 bin_size=1024"); + } + @Test(expected = DescriptorParseException.class) public void testDirreqReadHistoryMissingBytesEnd() throws DescriptorParseException { @@ -1131,6 +1139,17 @@ public class ExtraInfoDescriptorImplTest { } @Test() + public void testGeoipDbDigestExtraArg() throws DescriptorParseException { + ExtraInfoDescriptor descriptor = DescriptorBuilder + .createWithGeoip6DbDigestLine("geoip-db-digest " + + "916A3CA8B7DF61473D5AE5B21711F35F301CE9E8 " + + "yblgrXtEgF3glaKv5ZvHhRREUI1t1c37SxparXSmYR4Q1yiK5zg4HE8eT9ILPRW9" + + "3I5W/pZGQxL8Bu42dGjnAQ"); + assertEquals("916A3CA8B7DF61473D5AE5B21711F35F301CE9E8", + descriptor.getGeoipDbDigest()); + } + + @Test() public void testGeoip6DbDigestValid() throws DescriptorParseException { ExtraInfoDescriptor descriptor = DescriptorBuilder .createWithGeoip6DbDigestLine("geoip6-db-digest " @@ -1140,6 +1159,17 @@ public class ExtraInfoDescriptorImplTest { } @Test() + public void testGeoip6DbDigestExtraArg() throws DescriptorParseException { + ExtraInfoDescriptor descriptor = DescriptorBuilder + .createWithGeoip6DbDigestLine("geoip6-db-digest " + + "916A3CA8B7DF61473D5AE5B21711F35F301CE9E8 " + + "yblgrXtEgF3glaKv5ZvHhRREUI1t1c37SxparXSmYR4Q1yiK5zg4HE8eT9ILPRW9" + + "3I5W/pZGQxL8Bu42dGjnAQ"); + assertEquals("916A3CA8B7DF61473D5AE5B21711F35F301CE9E8", + descriptor.getGeoip6DbDigest()); + } + + @Test() public void testGeoipStatsValid() throws DescriptorParseException { ExtraInfoDescriptor descriptor = GeoipStatsBuilder .createWithDefaultLines(); @@ -1242,6 +1272,14 @@ public class ExtraInfoDescriptorImplTest { } @Test() + public void testGeoipClientOriginsExtraArg() + throws DescriptorParseException { + GeoipStatsBuilder.createWithGeoipClientOriginsLine( + "geoip-client-origins de=1152,de=952,cn=896,us=712,it=504 " + + "ru=352 fr=208 gb=208 ir=200"); + } + + @Test() public void testDirreqStatsValid() throws DescriptorParseException { ExtraInfoDescriptor descriptor = DirreqStatsBuilder .createWithDefaultLines(); @@ -1279,6 +1317,13 @@ public class ExtraInfoDescriptorImplTest { + "2012-02-11 00:59:53 (172800 s)"); } + @Test() + public void testDirreqStatsExtraArg() + throws DescriptorParseException { + DirreqStatsBuilder.createWithDirreqStatsEndLine("dirreq-stats-end " + + "2012-02-11 00:59:53 (172800 s) XXXXXXXXXXXXXXXXXXXXXXXXXXX"); + } + @Test(expected = DescriptorParseException.class) public void testDirreqV3IpsThreeLetterCountry() throws DescriptorParseException { @@ -1287,6 +1332,13 @@ public class ExtraInfoDescriptorImplTest { } @Test() + public void testDirreqV3IpsExtraArg() + throws DescriptorParseException { + DirreqStatsBuilder.createWithDirreqV3IpsLine("dirreq-v3-ips " + + "ab=12,cd=34 ef=56"); + } + + @Test() public void testDirreqV2IpsDigitCountry() throws DescriptorParseException { DirreqStatsBuilder.createWithDirreqV2IpsLine("dirreq-v2-ips 00=8"); @@ -1312,6 +1364,13 @@ public class ExtraInfoDescriptorImplTest { + "ok==10848"); } + @Test() + public void testDirreqV3RespExtraArg() + throws DescriptorParseException { + DirreqStatsBuilder.createWithDirreqV3RespLine("dirreq-v3-resp " + + "ok=1084 4801=ko"); + } + @Test(expected = DescriptorParseException.class) public void testDirreqV2RespNull() throws DescriptorParseException { @@ -1333,6 +1392,13 @@ public class ExtraInfoDescriptorImplTest { + "0.37"); } + @Test() + public void testDirreqV3ShareExtraArg() + throws DescriptorParseException { + DirreqStatsBuilder.createWithDirreqV3ShareLine("dirreq-v3-share " + + "0.37% 123456"); + } + @Test(expected = DescriptorParseException.class) public void testDirreqV3DirectDlSpace() throws DescriptorParseException { @@ -1362,6 +1428,13 @@ public class ExtraInfoDescriptorImplTest { } @Test() + public void testDirreqV3TunneledDlExtraArg() + throws DescriptorParseException { + DirreqStatsBuilder.createWithDirreqV2TunneledDlLine( + "dirreq-v2-tunneled-dl complete=-8 incomplete=1/-8"); + } + + @Test() public void testEntryStatsValid() throws DescriptorParseException { ExtraInfoDescriptor descriptor = EntryStatsBuilder .createWithDefaultLines(); @@ -1443,6 +1516,13 @@ public class ExtraInfoDescriptorImplTest { } @Test() + public void testCellCircuitsPerDecileExtraArg() + throws DescriptorParseException { + CellStatsBuilder.createWithCellCircuitsPerDecileLine( + "cell-circuits-per-decile 866 866 866 866 866"); + } + + @Test() public void testConnBiDirectValid() throws DescriptorParseException { ExtraInfoDescriptor descriptor = DescriptorBuilder @@ -1465,6 +1545,13 @@ public class ExtraInfoDescriptorImplTest { } @Test() + public void testConnBiDirectStatsExtraArg() + throws DescriptorParseException { + DescriptorBuilder.createWithConnBiDirectLine("conn-bi-direct " + + "2012-02-11 01:59:39 (86400 s) 42173,1591,1310,1744 +1"); + } + + @Test() public void testExitStatsValid() throws DescriptorParseException { ExtraInfoDescriptor descriptor = ExitStatsBuilder .createWithDefaultLines(); @@ -1541,6 +1628,12 @@ public class ExtraInfoDescriptorImplTest { } @Test() + public void testExitStatsStreamsExtraArg() throws DescriptorParseException { + ExitStatsBuilder.createWithExitStreamsOpenedLine( + "exit-streams-opened 25=21474 3648"); + } + + @Test() public void testBridgeStatsValid() throws DescriptorParseException { ExtraInfoDescriptor descriptor = BridgeStatsBuilder .createWithDefaultLines(); @@ -1570,6 +1663,13 @@ public class ExtraInfoDescriptorImplTest { + "2012-02-11 01:59:39 (0 s)"); } + @Test() + public void testBridgeStatsEndExtraArg() + throws DescriptorParseException { + BridgeStatsBuilder.createWithBridgeStatsEndLine("bridge-stats-end " + + "2012-02-11 01:59:39 (86400 s) 99999999999999999999999999999999"); + } + @Test(expected = DescriptorParseException.class) public void testBridgeIpsDouble() throws DescriptorParseException { @@ -1585,6 +1685,12 @@ public class ExtraInfoDescriptorImplTest { 0x69, 0x70, 0x73 }, false); // "ips" (no newline) } + @Test() + public void testBridgeIpsExtraArg() + throws DescriptorParseException { + BridgeStatsBuilder.createWithBridgeIpsLine("bridge-ips ir=24 5"); + } + @Test(expected = DescriptorParseException.class) public void testBridgeIpVersionsDouble() throws DescriptorParseException { @@ -1592,6 +1698,13 @@ public class ExtraInfoDescriptorImplTest { "bridge-ip-versions v4=24.5"); } + @Test() + public void testBridgeIpVersionsExtraArg() + throws DescriptorParseException { + BridgeStatsBuilder.createWithBridgeIpVersionsLine( + "bridge-ip-versions v4=24 5"); + } + @Test(expected = DescriptorParseException.class) public void testBridgeIpTransportsDouble() throws DescriptorParseException { @@ -1607,6 +1720,13 @@ public class ExtraInfoDescriptorImplTest { } @Test() + public void testBridgeIpTransportsExtraArg() + throws DescriptorParseException { + BridgeStatsBuilder.createWithBridgeIpTransportsLine( + "bridge-ip-transports obfs2=24 5"); + } + + @Test() public void testHidservStatsValid() throws DescriptorParseException { ExtraInfoDescriptor descriptor = HidservStatsBuilder .createWithDefaultLines(); @@ -1670,6 +1790,13 @@ public class ExtraInfoDescriptorImplTest { } @Test() + public void testHidservDirOnionsSeenExtraArg() + throws DescriptorParseException { + HidservStatsBuilder.createWithHidservDirOnionsSeenLine( + "hidserv-dir-onions-seen -3 delta_f=8 epsilon=0.30 bin_size=8 pi=3"); + } + + @Test() public void testRouterSignatureOpt() throws DescriptorParseException { DescriptorBuilder.createWithRouterSignatureLines("opt " diff --git a/src/test/java/org/torproject/descriptor/impl/ServerDescriptorImplTest.java b/src/test/java/org/torproject/descriptor/impl/ServerDescriptorImplTest.java index 80dc815..b713dd6 100644 --- a/src/test/java/org/torproject/descriptor/impl/ServerDescriptorImplTest.java +++ b/src/test/java/org/torproject/descriptor/impl/ServerDescriptorImplTest.java @@ -1245,11 +1245,11 @@ public class ServerDescriptorImplTest { "write-history 2012-01-01 03:51:44 (900 "); } - @Test(expected = DescriptorParseException.class) - public void testWriteHistoryTrailingNumber() + @Test() + public void testWriteHistoryExtraArg() throws DescriptorParseException { DescriptorBuilder.createWithWriteHistoryLine("write-history " - + "2012-01-01 03:51:44 (900 s) 4345856 1"); + + "2012-01-01 03:51:44 (900 s) 4345856 bin_size=1024"); } @Test()