commit 405faa26c694f0d3c323357f0b3d501a3edd0b7d Author: Iain R. Learmonth irl@fsfe.org Date: Fri Aug 17 08:55:04 2018 +0100

Improves AS number parameter parsing

* AS0 is an allowed value. This is used for unknown AS numbers. * Leading zeros are stripped from the AS number. * If an AS number is larger than the maximum possible (in 32-bits) then this will be treated as an error. * Tests are increased to cover additional cases.

Fixes: #27163 --- CHANGELOG.md | 7 +++++ .../torproject/onionoo/server/ResourceServlet.java | 12 ++++++-- .../onionoo/server/ResourceServletTest.java | 36 +++++++++++++++++++++- 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md index 84c6dd8..0ce49b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +# Changes in version 6.2-1.17.1 - 2018-08-?? + + * Minor changes + - Parsing of the "as" parameter allows AS0 to be specified. It will now + strip leading zeros. Specifying an AS number larger than the maximum + possible with 32-bits will be treated as an error. + # Changes in version 6.2-1.17.0 - 2018-08-16

* Medium changes diff --git a/src/main/java/org/torproject/onionoo/server/ResourceServlet.java b/src/main/java/org/torproject/onionoo/server/ResourceServlet.java index 15087de..afcb867 100644 --- a/src/main/java/org/torproject/onionoo/server/ResourceServlet.java +++ b/src/main/java/org/torproject/onionoo/server/ResourceServlet.java @@ -502,7 +502,7 @@ public class ResourceServlet extends HttpServlet { }

private static Pattern asNumberParameterPattern = - Pattern.compile("((^|,)([aA][sS])?[1-9][0-9]{0,9})+$"); + Pattern.compile("((^|,)([aA][sS])?0*[0-9]{1,10})+$");

private String[] parseAsNumberParameter(String parameter) { if (!asNumberParameterPattern.matcher(parameter).matches()) { @@ -512,8 +512,14 @@ public class ResourceServlet extends HttpServlet { String[] parameterParts = parameter.toUpperCase().split(","); String[] parsedParameter = new String[parameterParts.length]; for (int i = 0; i < parameterParts.length; i++) { - parsedParameter[i] = (!parameterParts[i].startsWith("AS") ? "AS" : "") - + parameterParts[i]; + boolean asPrefix = parameterParts[i].startsWith("AS"); + Long asNumber = Long.parseLong(asPrefix + ? parameterParts[i].substring(2) : parameterParts[i]); + if (asNumber > 4294967295L) { + /* AS number was too large */ + return null; + } + parsedParameter[i] = "AS" + asNumber.toString(); } return parsedParameter; } diff --git a/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java b/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java index 923fb6e..bc01003 100644 --- a/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java +++ b/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java @@ -178,7 +178,7 @@ public class ResourceServletTest { DateTimeHelper.parse("2013-04-22 20:00:00"), false, new TreeSet<>(Arrays.asList(new String[] { "Fast", "Running", "Unnamed", "V2Dir", "Valid" })), 63L, "a1", - DateTimeHelper.parse("2013-04-16 18:00:00"), "AS6830", + DateTimeHelper.parse("2013-04-16 18:00:00"), null, "liberty global operations b.v.", "1024d/51e2a1c7 "steven j. murdoch" " + "tor+steven.murdoch@cl.cam.ac.uk fb-token:5sr_k_zs2wm=", @@ -1167,6 +1167,12 @@ public class ResourceServletTest { }

@Test(timeout = 100) + public void testAsas8767WithLeadingZeros() { + this.assertSummaryDocument( + "/summary?as=as008767", 1, new String[] { "TorkaZ" }, 0, null); + } + + @Test(timeout = 100) public void testAsAsSpace8767() { this.assertErrorStatusCode( "/summary?as=AS 8767", 400); @@ -1179,6 +1185,34 @@ public class ResourceServletTest { }

@Test(timeout = 100) + public void testAsUnknown() { + this.assertSummaryDocument("/summary?as=0", 1, + new String[] {"TimMayTribute"}, 0, null); + } + + @Test(timeout = 100) + public void testAsAsUnknown() { + this.assertSummaryDocument("/summary?as=as0", 1, + new String[] {"TimMayTribute"}, 0, null); + } + + @Test(timeout = 100) + public void testAsAsUnknownWithLeadingZeros() { + this.assertSummaryDocument("/summary?as=as0000", 1, + new String[] {"TimMayTribute"}, 0, null); + } + + @Test(timeout = 100) + public void testAsTooLarge() { + this.assertErrorStatusCode("/summary?as=4294967296", 400); + } + + @Test(timeout = 100) + public void testAsNegative() { + this.assertErrorStatusCode("/summary?as=-3", 400); + } + + @Test(timeout = 100) public void testAsNameComcast() { this.assertSummaryDocument("/summary?as_name=Comcast", 1, null, 0, null); }