Correctly parse query string from servlet http requests

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

@@ -22,7 +22,6 @@
 
 import java.util.Iterator;
 import java.util.Objects;
-import java.util.stream.Stream;
 
 public class HttpRequest extends HttpMessage {
 
@@ -59,10 +58,11 @@ public String getQueryParameter(String name) {
    * 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) {
+  public HttpRequest addQueryParameter(String name, String value) {
     queryParameters.put(
         Objects.requireNonNull(name, "Name must be set"),
         Objects.requireNonNull(value, "Value must be set"));
+    return this;
   }
 
   public Iterable<String> getQueryParameterNames() {

java/server/src/org/openqa/grid/internal/utils/SelfRegisteringRemote.java

@@ -385,7 +385,8 @@ private boolean isAlreadyRegistered(RegistrationRequest node) {
       if (id == null) {
         id = node.getConfiguration().getRemoteHost();
       }
-      HttpRequest request = new HttpRequest(GET, api.toExternalForm() + "?id=" + id);
+      HttpRequest request = new HttpRequest(GET, api.toExternalForm())
+          .addQueryParameter("id", id);
 
       HttpResponse response = client.execute(request);
       if (response.getStatus() != 200) {

java/server/src/org/openqa/selenium/grid/server/ServletRequestWrappingHttpRequest.java

@@ -81,7 +81,7 @@ public void addHeader(String name, String value) {
   }
 
   @Override
-  public void addQueryParameter(String name, String value) {
+  public HttpRequest addQueryParameter(String name, String value) {
     throw new UnsupportedOperationException("addQueryParameter");
   }
 
@@ -108,7 +108,7 @@ private Map<String, Collection<String>> parseQueryString() {
         key = paramAndValue;
         value = "";
       } else {
-        key = paramAndValue.substring(index);
+        key = paramAndValue.substring(0, index);
         if (paramAndValue.length() >= index) {
           value = paramAndValue.substring(index + 1);
         } else {

java/server/test/org/openqa/selenium/grid/server/ServletRequestWrappingHttpRequestTest.java

@@ -0,0 +1,52 @@
+// 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.grid.server;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.google.common.collect.ImmutableMap;
+
+import org.junit.Test;
+import org.openqa.testing.FakeHttpServletRequest;
+import org.openqa.testing.UrlInfo;
+
+public class ServletRequestWrappingHttpRequestTest {
+
+  @Test
+  public void shouldNotExtractQueryParametersFromPostedBody() {
+    FakeHttpServletRequest request = new FakeHttpServletRequest(
+        "GET",
+        new UrlInfo("http://example.com", "/", "some-url"));
+    request.setParameters(ImmutableMap.of("cheese", "brie"));
+
+    String seen = new ServletRequestWrappingHttpRequest(request).getQueryParameter("cheese");
+    assertThat(seen).isNull();
+  }
+
+  @Test
+  public void shouldExtractQueryParametersFromQueryString() {
+    FakeHttpServletRequest request = new FakeHttpServletRequest(
+        "GET",
+        new UrlInfo("http://example.com", "/", "some-url", "cheese=cheddar"));
+    request.setParameters(ImmutableMap.of("cheese", "brie"));
+
+    String seen = new ServletRequestWrappingHttpRequest(request).getQueryParameter("cheese");
+    assertThat(seen).isEqualTo("cheddar");
+  }
+
+}

java/server/test/org/openqa/testing/FakeHttpServletRequest.java

@@ -145,7 +145,7 @@ public String getContextPath() {
 
   @Override
   public String getQueryString() {
-    throw new UnsupportedOperationException();
+    return requestUrl.getQueryString();
   }
 
   @Override

java/server/test/org/openqa/testing/UrlInfo.java

@@ -22,11 +22,17 @@ public class UrlInfo {
   private final String baseUrl;
   private final String contextPath;
   private final String pathInfo;
+  private final String queryString;
 
   public UrlInfo(String baseUrl, String contextPath, String pathInfo) {
+    this(baseUrl, contextPath, pathInfo, "");
+  }
+
+  public UrlInfo(String baseUrl, String contextPath, String pathInfo, String queryString) {
     this.baseUrl = baseUrl;
     this.contextPath = contextPath;
     this.pathInfo = pathInfo;
+    this.queryString = queryString;
   }
 
   public String getBaseUrl() {
@@ -45,6 +51,10 @@ public String getPathInfo() {
     return pathInfo;
   }
 
+  public String getQueryString() {
+    return queryString;
+  }
+
   @Override
   public String toString() {
     return baseUrl + contextPath + pathInfo;