Make HttpRequest support query parameters.

Author: shs96c

java/client/src/org/openqa/selenium/remote/BUCK

@@ -122,6 +122,7 @@ java_library(name = 'remote-lib',
     'http/W3CHttpResponseCodec.java',
     'internal/ApacheHttpClient.java',
     'internal/HttpClientFactory.java',
+    'internal/HttpUrlBuilder.java',
     'internal/JreHttpClient.java',
     'internal/OkHttpClient.java',
     'internal/JsonToWebElementConverter.java',

java/client/src/org/openqa/selenium/remote/http/HttpRequest.java

@@ -17,10 +17,20 @@
 
 package org.openqa.selenium.remote.http;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Multimaps;
+
+import java.util.Collection;
+import java.util.Objects;
+
 public class HttpRequest extends HttpMessage {
 
   private final HttpMethod method;
   private final String uri;
+  private final Multimap<String, String> queryParameters = Multimaps.newListMultimap(
+      Maps.<String, Collection<String>>newHashMap(), Lists::newLinkedList);
 
   public HttpRequest(HttpMethod method, String uri) {
     this.method = method;
@@ -34,4 +44,29 @@ public String getUri() {
   public HttpMethod getMethod() {
     return method;
   }
+
+  /**
+   * Get a query parameter. The implementation will take care of decoding the from percent encoding.
+   */
+  public String getQueryParameter(String name) {
+    return queryParameters.get(name).stream().findFirst().orElse(null);
+  }
+
+  /**
+   * Set a query parameter, adding to existing values if present. The implementation will ensure
+   * that the name and value are properly encoded.
+   */
+  public void addQueryParameter(String name, String value) {
+    queryParameters.put(
+        Objects.requireNonNull(name, "Name must be set"),
+        Objects.requireNonNull(value, "Value must be set"));
+  }
+
+  public Iterable<String> getQueryParameterNames() {
+    return queryParameters.keySet();
+  }
+
+  public Iterable<String> getQueryParameters(String name) {
+    return queryParameters.get(name);
+  }
 }

java/client/src/org/openqa/selenium/remote/internal/ApacheHttpClient.java

@@ -73,8 +73,8 @@ public HttpResponse execute(HttpRequest request) throws IOException {
   public HttpResponse execute(HttpRequest request, boolean followRedirects) throws IOException {
     HttpContext context = createContext();
 
-    String requestUrl = url.toExternalForm().replaceAll("/$", "") + request.getUri();
-    HttpUriRequest httpMethod = createHttpUriRequest(request.getMethod(), requestUrl);
+    URL url = HttpUrlBuilder.toUrl(this.url, request);
+    HttpUriRequest httpMethod = createHttpUriRequest(request.getMethod(), url);
     for (String name : request.getHeaderNames()) {
       // Skip content length as it is implicitly set when the message entity is set below.
       if (!"Content-Length".equalsIgnoreCase(name)) {
@@ -125,14 +125,22 @@ protected HttpContext createContext() {
     return new BasicHttpContext();
   }
 
-  private static HttpUriRequest createHttpUriRequest(HttpMethod method, String url) {
+  private static HttpUriRequest createHttpUriRequest(HttpMethod method, URL url)
+      throws IOException {
+    URI uri = null;
+    try {
+      uri = url.toURI();
+    } catch (URISyntaxException e) {
+      throw new IOException(e);
+    }
+
     switch (method) {
       case DELETE:
-        return new HttpDelete(url);
+        return new HttpDelete(uri);
       case GET:
-        return new HttpGet(url);
+        return new HttpGet(uri);
       case POST:
-        return new HttpPost(url);
+        return new HttpPost(uri);
     }
     throw new AssertionError("Unsupported method: " + method);
   }

java/client/src/org/openqa/selenium/remote/internal/HttpUrlBuilder.java

@@ -0,0 +1,73 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.openqa.selenium.remote.internal;
+
+import static java.nio.charset.StandardCharsets.US_ASCII;
+
+import com.google.common.base.Joiner;
+
+import org.openqa.selenium.remote.http.HttpRequest;
+
+import java.io.UncheckedIOException;
+import java.io.UnsupportedEncodingException;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLEncoder;
+import java.util.List;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
+
+class HttpUrlBuilder {
+
+  private final static Function<String, String> QUERY_ENCODE = str -> {
+    try {
+      return URLEncoder.encode(str, US_ASCII.toString());
+    } catch (UnsupportedEncodingException e) {
+      throw new UncheckedIOException(e);
+    }
+  };
+
+  private HttpUrlBuilder() {
+    // Helper class
+  }
+
+  static URL toUrl(URL base, HttpRequest request) throws MalformedURLException {
+    StringBuilder queryString = new StringBuilder();
+    Joiner parameters = Joiner.on("&");
+
+    List<String>
+        allParams = StreamSupport.stream(request.getQueryParameterNames().spliterator(), false)
+        .map(name -> {
+          String encoded = QUERY_ENCODE.apply(name);
+          return parameters.join(
+              StreamSupport.stream(request.getQueryParameters(name).spliterator(), false)
+                  .map(value -> encoded + "=" + QUERY_ENCODE.apply(value))
+                  .collect(Collectors.toList()));
+        })
+        .collect(Collectors.toList());
+    parameters.appendTo(queryString, allParams);
+
+    String baseUrl = base.toExternalForm().replaceAll("/$", "") + request.getUri();
+    if (!queryString.toString().isEmpty()) {
+      baseUrl += "?" + queryString.toString();
+    }
+
+    return new URL(baseUrl);
+  }
+}

java/client/src/org/openqa/selenium/remote/internal/JreHttpClient.java

@@ -17,6 +17,7 @@
 
 package org.openqa.selenium.remote.internal;
 
+import static java.nio.charset.StandardCharsets.US_ASCII;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import org.openqa.selenium.remote.http.HttpClient;
@@ -28,15 +29,26 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.io.UncheckedIOException;
+import java.io.UnsupportedEncodingException;
 import java.net.HttpURLConnection;
 import java.net.URL;
+import java.net.URLEncoder;
 import java.util.Base64;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.Function;
 
 public class JreHttpClient implements HttpClient {
 
+  private final static Function<String, String> QUERY_ENCODE = str -> {
+    try {
+      return URLEncoder.encode(str, US_ASCII.toString());
+    } catch (UnsupportedEncodingException e) {
+      throw new UncheckedIOException(e);
+    }
+  };
   private final URL url;
   private final String auth;
 
@@ -61,7 +73,7 @@ public HttpResponse execute(HttpRequest request) throws IOException {
 
   @Override
   public HttpResponse execute(HttpRequest request, boolean followRedirects) throws IOException {
-    URL url = new URL(this.url.toString() + request.getUri());
+    URL url = HttpUrlBuilder.toUrl(this.url, request);
 
     HttpURLConnection connection = (HttpURLConnection) url.openConnection();
     try {

java/client/src/org/openqa/selenium/remote/internal/OkHttpClient.java

@@ -22,6 +22,7 @@
 import org.openqa.selenium.remote.http.HttpResponse;
 
 import okhttp3.ConnectionPool;
+import okhttp3.HttpUrl;
 import okhttp3.MediaType;
 import okhttp3.Request;
 import okhttp3.RequestBody;
@@ -54,7 +55,20 @@ public HttpResponse execute(HttpRequest request, boolean followRedirects) throws
 
     Request.Builder builder = new Request.Builder();
 
-    builder.url(new URL(baseUrl.toString() + request.getUri()));
+    HttpUrl.Builder url;
+    try {
+      url = HttpUrl.parse(baseUrl.toString() + request.getUri()).newBuilder();
+    } catch (NullPointerException e) {
+      throw new IOException("Unable to parse URL: " + baseUrl.toString() + request.getUri());
+    }
+
+    for (String name : request.getQueryParameterNames()) {
+      for (String value : request.getQueryParameters(name)) {
+        url.addQueryParameter(name, value);
+      }
+    }
+
+    builder.url(url.build());
 
     for (String name : request.getHeaderNames()) {
       for (String value : request.getHeaders(name)) {

java/client/test/org/openqa/selenium/remote/internal/HttpClientTestBase.java

@@ -18,25 +18,31 @@
 package org.openqa.selenium.remote.internal;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.openqa.selenium.json.Json.MAP_TYPE;
+import static org.openqa.selenium.remote.http.HttpMethod.GET;
 
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Multimap;
 
 import org.junit.Test;
+import org.openqa.selenium.json.Json;
+import org.openqa.selenium.json.JsonOutput;
 import org.openqa.selenium.net.PortProber;
 import org.openqa.selenium.remote.http.HttpClient;
-import org.openqa.selenium.remote.http.HttpMethod;
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
 import org.seleniumhq.jetty9.server.Server;
 import org.seleniumhq.jetty9.servlet.ServletContextHandler;
 import org.seleniumhq.jetty9.servlet.ServletHolder;
 
 import java.io.IOException;
+import java.io.Writer;
+import java.util.Map;
+import java.util.stream.Stream;
 
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -76,6 +82,53 @@ public void responseShouldKeepMultipleHeadersSeparate() throws Exception {
     assertTrue(values.toString(), values.contains("Brie, Gouda"));
   }
 
+  @Test
+  public void shouldAddUrlParameters() throws Exception {
+    HttpRequest request = new HttpRequest(GET, "/query");
+    String value = request.getQueryParameter("cheese");
+    assertNull(value);
+
+    request.addQueryParameter("cheese", "brie");
+    value = request.getQueryParameter("cheese");
+    assertEquals("brie", value);
+  }
+
+  @Test
+  public void shouldSendSimpleQueryParameters() throws Exception {
+    HttpRequest request = new HttpRequest(GET, "/query");
+    request.addQueryParameter("cheese", "cheddar");
+
+    HttpResponse response = getQueryParameterResponse(request);
+    Map<String, Object> values = new Json().toType(response.getContentString(), MAP_TYPE);
+
+    assertEquals(ImmutableList.of("cheddar"), values.get("cheese"));
+  }
+
+  @Test
+  public void shouldEncodeParameterNamesAndValues() throws Exception {
+    HttpRequest request = new HttpRequest(GET, "/query");
+    request.addQueryParameter("cheese type", "tasty cheese");
+
+    HttpResponse response = getQueryParameterResponse(request);
+    Map<String, Object> values = new Json().toType(response.getContentString(), MAP_TYPE);
+
+    assertEquals(ImmutableList.of("tasty cheese"), values.get("cheese type"));
+  }
+
+  @Test
+  public void canAddMoreThanOneQueryParameter() throws Exception {
+    HttpRequest request = new HttpRequest(GET, "/query");
+    request.addQueryParameter("cheese", "cheddar");
+    request.addQueryParameter("cheese", "gouda");
+    request.addQueryParameter("vegetable", "peas");
+
+    HttpResponse response = getQueryParameterResponse(request);
+    Map<String, Object> values = new Json().toType(response.getContentString(), MAP_TYPE);
+
+    assertEquals(ImmutableList.of("cheddar", "gouda"), values.get("cheese"));
+    assertEquals(ImmutableList.of("peas"), values.get("vegetable"));
+  }
+
   private HttpResponse getResponseWithHeaders(final Multimap<String, String> headers)
       throws Exception {
     Server server = new Server(PortProber.findFreePort());
@@ -84,8 +137,7 @@ private HttpResponse getResponseWithHeaders(final Multimap<String, String> heade
 
     class Headers extends HttpServlet {
       @Override
-      protected void doGet(HttpServletRequest req, HttpServletResponse resp)
-          throws ServletException, IOException {
+      protected void doGet(HttpServletRequest req, HttpServletResponse resp) {
         headers.forEach(resp::addHeader);
         resp.setContentLengthLong(0);
       }
@@ -98,7 +150,43 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
     server.start();
     try {
       HttpClient client = createFactory().createClient(server.getURI().toURL());
-      HttpRequest request = new HttpRequest(HttpMethod.GET, "/foo");
+      HttpRequest request = new HttpRequest(GET, "/foo");
+      return client.execute(request);
+    } finally {
+      server.stop();
+    }
+  }
+
+  private HttpResponse getQueryParameterResponse(HttpRequest request) throws Exception {
+    Server server = new Server(PortProber.findFreePort());
+    ServletContextHandler handler = new ServletContextHandler();
+    handler.setContextPath("");
+
+    class Parameters extends HttpServlet {
+      @Override
+      protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        try (Writer writer = resp.getWriter()) {
+          JsonOutput json = new Json().newOutput(writer);
+          json.beginObject();
+          req.getParameterMap()
+              .forEach((key, value) -> {
+                json.name(key);
+                json.beginArray();
+                Stream.of(value).forEach(v -> json.write(v, String.class));
+                json.endArray();
+              });
+          json.endObject();
+        }
+      }
+    }
+    ServletHolder holder = new ServletHolder(new Parameters());
+    handler.addServlet(holder, "/*");
+
+    server.setHandler(handler);
+
+    server.start();
+    try {
+      HttpClient client = createFactory().createClient(server.getURI().toURL());
       return client.execute(request);
     } finally {
       server.stop();