[tor-commits] [onionoo/master] Parse search parameter from query string.

karsten at torproject.org karsten at torproject.org
Sat Nov 15 17:23:17 UTC 2014


commit c13da0ea9197401953ad81b40bc030082d2c8708
Author: Karsten Loesing <karsten.loesing at 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);





More information about the tor-commits mailing list