commit c13da0ea9197401953ad81b40bc030082d2c8708 Author: Karsten Loesing karsten.loesing@gmx.net Date: Fri Oct 17 09:28:04 2014 +0200

Parse search parameter from query string.

We can't rely on Tomcat to parse the query string, because it magically replaces `+` with the space character. This is going to break as soon as `+` will be a valid part of a search term.

This is in preparation of #13135. --- .../onionoo/server/HttpServletRequestWrapper.java | 3 ++ .../torproject/onionoo/server/ResourceServlet.java | 17 +++++++-- .../torproject/onionoo/ResourceServletTest.java | 39 ++++++++++---------- 3 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/src/main/java/org/torproject/onionoo/server/HttpServletRequestWrapper.java b/src/main/java/org/torproject/onionoo/server/HttpServletRequestWrapper.java index 3349acd..6cf022f 100644 --- a/src/main/java/org/torproject/onionoo/server/HttpServletRequestWrapper.java +++ b/src/main/java/org/torproject/onionoo/server/HttpServletRequestWrapper.java @@ -21,4 +21,7 @@ public class HttpServletRequestWrapper { protected String[] getParameterValues(String parameterKey) { return this.request.getParameterValues(parameterKey); } + protected String getQueryString() { + return this.request.getQueryString(); + } } \ No newline at end of file diff --git a/src/main/java/org/torproject/onionoo/server/ResourceServlet.java b/src/main/java/org/torproject/onionoo/server/ResourceServlet.java index cd99dcb..46617c1 100644 --- a/src/main/java/org/torproject/onionoo/server/ResourceServlet.java +++ b/src/main/java/org/torproject/onionoo/server/ResourceServlet.java @@ -11,6 +11,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Matcher; import java.util.regex.Pattern;

import javax.servlet.ServletConfig; @@ -146,8 +147,8 @@ public class ResourceServlet extends HttpServlet {

/* Filter relays and bridges matching the request. */ if (parameterMap.containsKey("search")) { - String[] searchTerms = this.parseSearchParameters( - parameterMap.get("search")); + String[] searchTerms = parseSearchParameters( + request.getQueryString()); if (searchTerms == null) { response.sendError(HttpServletResponse.SC_BAD_REQUEST); return; @@ -357,12 +358,22 @@ public class ResourceServlet extends HttpServlet { bridgeDocumentsWritten, charsWritten, writtenResponseMillis); }

+ private static Pattern searchQueryStringPattern = + Pattern.compile("(?:.*[\?&])*?" // lazily skip other parameters + + "search=([0-9a-zA-Z+/\.: \$\[\]]+)" // capture parameter + + "(?:&.*)*"); // skip remaining parameters private static Pattern searchParameterPattern = Pattern.compile("^\$?[0-9a-fA-F]{1,40}$|" /* Fingerprint. */ + "^[0-9a-zA-Z\.]{1,19}$|" /* Nickname or IPv4 address. */ + "^\[[0-9a-fA-F:\.]{1,39}\]?$|" /* IPv6 address. */ + "^[a-zA-Z_]+:[0-9a-zA-Z_,-]+$" /* Qualified search term. */); - private String[] parseSearchParameters(String parameter) { + protected static String[] parseSearchParameters(String queryString) { + Matcher searchQueryStringMatcher = searchQueryStringPattern.matcher( + queryString); + if (!searchQueryStringMatcher.matches()) { + return null; + } + String parameter = searchQueryStringMatcher.group(1); String[] searchParameters; if (parameter.contains(" ")) { searchParameters = parameter.split(" "); diff --git a/src/test/java/org/torproject/onionoo/ResourceServletTest.java b/src/test/java/org/torproject/onionoo/ResourceServletTest.java index 11068dc..0a861c3 100644 --- a/src/test/java/org/torproject/onionoo/ResourceServletTest.java +++ b/src/test/java/org/torproject/onionoo/ResourceServletTest.java @@ -48,11 +48,13 @@ public class ResourceServletTest { private class TestingHttpServletRequestWrapper extends HttpServletRequestWrapper { private String requestURI; + private String queryString; private Map<String, String[]> parameterMap; private TestingHttpServletRequestWrapper(String requestURI, - Map<String, String[]> parameterMap) { + String queryString, Map<String, String[]> parameterMap) { super(null); this.requestURI = requestURI; + this.queryString = queryString; this.parameterMap = parameterMap == null ? new HashMap<String, String[]>() : parameterMap; } @@ -66,6 +68,9 @@ public class ResourceServletTest { protected String[] getParameterValues(String parameterKey) { return this.parameterMap.get(parameterKey); } + protected String getQueryString() { + return this.queryString; + } }

private class TestingHttpServletResponseWrapper extends @@ -185,13 +190,12 @@ public class ResourceServletTest { bridgegummy); }

- private void runTest(String requestURI, - Map<String, String[]> parameterMap) { + private void runTest(String request) { try { this.createDummyTime(); this.createDummyDocumentStore(); this.createNodeIndexer(); - this.makeRequest(requestURI, parameterMap); + this.makeRequest(request); this.parseResponse(); } catch (IOException e) { throw new RuntimeException(e); @@ -226,11 +230,14 @@ public class ResourceServletTest { NodeIndexerFactory.setNodeIndexer(newNodeIndexer); }

- private void makeRequest(String requestURI, - Map<String, String[]> parameterMap) throws IOException { + private void makeRequest(String request) throws IOException { ResourceServlet rs = new ResourceServlet(); - this.request = new TestingHttpServletRequestWrapper(requestURI, - parameterMap); + String requestParts[] = request.split("\?"); + String path = requestParts[0]; + String queryString = requestParts.length > 1 ? requestParts[1] : null; + Map<String, String[]> parameterMap = parseParameters(request); + this.request = new TestingHttpServletRequestWrapper(path, queryString, + parameterMap); this.response = new TestingHttpServletResponseWrapper(); rs.doGet(this.request, this.response); } @@ -246,18 +253,14 @@ public class ResourceServletTest {

private void assertErrorStatusCode(String request, int errorStatusCode) { - String requestURI = parseRequestURI(request); - Map<String, String[]> parameters = parseParameters(request); - this.runTest(requestURI, parameters); + this.runTest(request); assertEquals(errorStatusCode, this.response.errorStatusCode); }

private void assertSummaryDocument(String request, int expectedRelaysNumber, String[] expectedRelaysNicknames, int expectedBridgesNumber, String[] expectedBridgesNicknames) { - String requestURI = parseRequestURI(request); - Map<String, String[]> parameters = parseParameters(request); - this.runTest(requestURI, parameters); + this.runTest(request); assertNotNull(this.summaryDocument); assertEquals(expectedRelaysNumber, this.summaryDocument.relays.length); @@ -277,10 +280,6 @@ public class ResourceServletTest { } }

- private String parseRequestURI(String request) { - return request.split("\?")[0]; - } - private Map<String, String[]> parseParameters(String request) { Map<String, String[]> parameters = null; String[] uriParts = request.split("\?"); @@ -327,7 +326,7 @@ public class ResourceServletTest {

@Test() public void testValidSummaryRelay() throws IOException { - this.runTest("/summary", null); + this.runTest("/summary"); assertEquals("2013-04-24 12:00:00", this.summaryDocument.relays_published); assertEquals(3, this.summaryDocument.relays.length); @@ -347,7 +346,7 @@ public class ResourceServletTest {

@Test() public void testValidSummaryBridge() { - this.runTest("/summary", null); + this.runTest("/summary"); assertEquals("2013-04-24 01:07:04", this.summaryDocument.bridges_published); assertEquals(3, this.summaryDocument.bridges.length);