Add a test for the ApacheHttpClient

Turns out it wasn't capturing all the headers correctly.

Author: shs96c

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

@@ -43,6 +43,7 @@
 
 import java.io.IOException;
 import java.net.BindException;
+import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
@@ -98,9 +99,7 @@ private HttpResponse createResponse(
 
     internalResponse.setStatus(response.getStatusLine().getStatusCode());
     for (Header header : response.getAllHeaders()) {
-      for (HeaderElement headerElement : header.getElements()) {
-        internalResponse.addHeader(header.getName(), headerElement.getValue());
-      }
+      internalResponse.addHeader(header.getName(), header.getValue());
     }
 
     HttpEntity entity = response.getEntity();
@@ -257,5 +256,19 @@ private static synchronized HttpClientFactory getDefaultHttpClientFactory() {
 	public void close() throws IOException {
 	  client.getConnectionManager().closeIdleConnections(0, TimeUnit.SECONDS);		
 	}
+
+  public static void main(String[] args) throws IOException {
+    org.openqa.selenium.remote.http.HttpClient client = new Factory().createClient(new URL("http://www.google.com/"));
+
+    HttpRequest request = new HttpRequest(HttpMethod.GET, "/");
+    HttpResponse response = client.execute(request, true);
+
+    for (String names : response.getHeaderNames()) {
+      Iterable<String> header = response.getHeaders(names);
+      for (String h : header) {
+        System.out.println(names + ": " + h);
+      }
+    }
+  }
 
 }

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

@@ -29,6 +29,7 @@ java_test(name = 'client-tests',
     'RemoteClientTests.java',
     'RemoteLogsTest.java',
     'RemoteWebDriverInitializationTest.java',
+    'internal/ApacheHttpClientTest.java',
     'internal/CircularOutputStreamTest.java',
     'internal/WebElementToJsonConverterTest.java',
   ],
@@ -38,6 +39,7 @@ java_test(name = 'client-tests',
     '//third_party/java/gson:gson',
     '//third_party/java/guava:guava',
     '//third_party/java/hamcrest:hamcrest-library',
+    '//third_party/java/jetty:jetty',
     '//third_party/java/junit:junit',
     '//third_party/java/mockito:mockito',
   ])

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

@@ -0,0 +1,107 @@
+// 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 org.junit.Assert.*;
+
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.MultimapBuilder;
+
+import org.junit.Test;
+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.HttpConnectionFactory;
+import org.seleniumhq.jetty9.server.Server;
+import org.seleniumhq.jetty9.server.ServerConnector;
+import org.seleniumhq.jetty9.servlet.ServletContextHandler;
+import org.seleniumhq.jetty9.servlet.ServletHolder;
+
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+public class ApacheHttpClientTest {
+
+  @Test
+  public void responseShouldCaptureASingleHeader() throws Exception {
+    HashMultimap<String, String> headers = HashMultimap.create();
+    headers.put("Cake", "Delicious");
+
+    HttpResponse response = getResponseWithHeaders(headers);
+
+    String value = response.getHeader("Cake");
+    assertEquals("Delicious", value);
+  }
+
+  /**
+   * The HTTP Spec that it should be
+   * <a href="https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2">safe to combine them
+   * </a>, but things like the <a href="https://www.ietf.org/rfc/rfc2109.txt">cookie spec</a> make
+   * this hard (notably when a legal value may contain a comma).
+   */
+  @Test
+  public void responseShouldKeepMultipleHeadersSeparate() throws Exception {
+    HashMultimap<String, String> headers = HashMultimap.create();
+    headers.put("Cheese", "Cheddar");
+    headers.put("Cheese", "Brie, Gouda");
+
+    HttpResponse response = getResponseWithHeaders(headers);
+
+    ImmutableList<String> values = ImmutableList.copyOf(response.getHeaders("Cheese"));
+
+    assertEquals("Cheddar", values.get(0));
+    assertEquals("Brie, Gouda", values.get(1));
+  }
+
+  private HttpResponse getResponseWithHeaders(final Multimap<String, String> headers)
+      throws Exception {
+    Server server = new Server(PortProber.findFreePort());
+    ServletContextHandler handler = new ServletContextHandler();
+    handler.setContextPath("");
+
+    class Headers extends HttpServlet {
+      @Override
+      protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+          throws ServletException, IOException {
+        headers.forEach(resp::addHeader);
+        resp.setContentLengthLong(0);
+      }
+    }
+    ServletHolder holder = new ServletHolder(new Headers());
+    handler.addServlet(holder, "/*");
+
+    server.setHandler(handler);
+
+    server.start();
+    try {
+      HttpClient client = new ApacheHttpClient.Factory().createClient(server.getURI().toURL());
+      HttpRequest request = new HttpRequest(HttpMethod.GET, "/foo");
+      return client.execute(request, true);
+    } finally {
+      server.stop();
+    }
+  }
+}

third_party/java/jetty/BUCK

@@ -14,6 +14,7 @@ java_library(
     '//java/client/test/com/thoughtworks/selenium:tests',
     '//java/client/test/org/openqa/selenium:tests',
     '//java/client/test/org/openqa/selenium/environment:environment',
+    '//java/client/test/org/openqa/selenium/remote:client-tests',
     '//java/server/src/org/openqa/grid:grid',
     '//java/server/test/org/openqa/grid:test',
     '//java/server/src/org/openqa/selenium/remote/server:standalone-server-lib',