again more fixes for WebRequest.getParameters() (#836)

src/changes/changes.xml

@@ -39,8 +39,7 @@
                 WebRequest.getParameters() updated to deliver the same results as the servlet API
                 will report if this request reaches a servlet. This implies several changes: For url-encoded and
                 plain text encoded post requests, the query parameters are part of the result.
-                For multipart requests only the query parameters forming the result. Additionally only
-                the first occurence of a key is part of the result.
+                For multipart requests only the query parameters forming the result.
             </action>
             <action type="add" dev="rbri" issue="#816">
                 WorkerNavigator and WorkerLocation implemented.

src/main/java/org/htmlunit/WebRequest.java

@@ -27,7 +27,6 @@
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -344,9 +343,14 @@ public void setEncodingType(final FormEncodingType encodingType) {
     }
 
     /**
-     * Retrieves the request parameters in use. Similar to the servlet api this will
-     * work depending on the request type and check the url parameters and
-     * the body. The value is also normalized - null is converted to an empty string.
+     * Retrieves the request parameters used. Similar to the servlet api function
+     * getParameterMap() this works depending on the request type and collects the
+     * url parameters and the body stuff.
+     * The value is also normalized - null is converted to an empty string.
+     * In contrast to the servlet api this creates a separate KeyValuePair for every
+     * parameter. This means that pairs with the same name can be part of the list. The
+     * servlet api will return a string[] as value for the key in this case.
+     *
      * @return the request parameters to use
      */
     public List<NameValuePair> getParameters() {
@@ -413,14 +417,9 @@ private static List<NameValuePair> normalize(final List<NameValuePair> pairs) {
             return pairs;
         }
 
-        final Set<String> keys = new HashSet<>();
-
         final List<NameValuePair> resultingPairs = new ArrayList<>();
         for (final NameValuePair pair : pairs) {
-            if (!keys.contains(pair.getName())) {
-                resultingPairs.add(pair.normalized());
-                keys.add(pair.getName());
-            }
+            resultingPairs.add(pair.normalized());
         }
 
         return resultingPairs;

src/test/java/org/htmlunit/WebRequest2Test.java

@@ -21,9 +21,9 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import javax.servlet.Servlet;
 import javax.servlet.ServletException;
@@ -51,6 +51,7 @@
  * see https://github.com/HtmlUnit/htmlunit/pull/836
  *
  * @author Ronald Brill
+ * @author Kristof Neirynck
  */
 @RunWith(BrowserParameterizedRunner.class)
 public class WebRequest2Test extends WebServerTestCase {
@@ -118,8 +119,19 @@ public static Collection<Object[]> data() throws Exception {
         parameterPairs.add(new NameValuePair("a", "b"));
         final TestParameters sameAsInQueryParameter = new TestParameters("sameAsInQuery", parameterPairs);
 
+        parameterPairs = new ArrayList<>();
+        parameterPairs.add(new NameValuePair("a", "other"));
+        final TestParameters sameKeyAsInQueryParameter = new TestParameters("sameKeyAsInQuery", parameterPairs);
+
+        parameterPairs = new ArrayList<>();
+        parameterPairs.add(new NameValuePair("same", "value1"));
+        parameterPairs.add(new NameValuePair("same", "value2"));
+        final TestParameters sameKeyDifferentValuesParameter
+                = new TestParameters("sameKeyDifferentValues", parameterPairs);
+
         final TestParameters[] parameters =
-            {null, emptyParameters, oneParameter, emptyValueParameter, sameAsInQueryParameter};
+            {null, emptyParameters, oneParameter, emptyValueParameter, sameAsInQueryParameter,
+                sameKeyAsInQueryParameter, sameKeyDifferentValuesParameter};
 
         final String[] bodies = {"", "a=b", "a=b&c=d", "a=", "a", "a=b&a=d"};
 
@@ -215,23 +227,46 @@ public void test() throws Exception {
 
                 String name = parts[0].trim();
                 name = StringUtils.strip(name, "'");
-                String value = parts[1].trim();
-                value = StringUtils.strip(value, "'");
 
-                final NameValuePair pair = new NameValuePair(name, value);
-                expectedParameters.add(pair);
+                String values = parts[1].trim();
+                values = StringUtils.strip(values, "[]");
+
+                for (String value : values.split(",")) {
+                    value = value.trim();
+                    value = StringUtils.strip(value, "'");
+
+                    final NameValuePair pair = new NameValuePair(name, value);
+                    expectedParameters.add(pair);
+                }
             }
         }
 
-        final List<NameValuePair> parameters = request.getParameters();
-        assertEquals(expectedParameters.size(), parameters.size());
-
-        final Iterator<NameValuePair> expectedIter = expectedParameters.iterator();
-        for (final NameValuePair nameValuePair : parameters) {
-            final NameValuePair expectedNameValuePair = expectedIter.next();
+        final List<String> expectedNames = expectedParameters.stream()
+                                                .map(NameValuePair::getName)
+                                                .distinct()
+                                                .collect(Collectors.toList());
 
-            assertEquals(expectedNameValuePair.getName(), nameValuePair.getName());
-            assertEquals(expectedNameValuePair.getValue(), nameValuePair.getValue());
+        final List<NameValuePair> parameters = request.getParameters();
+        final List<String> parameterNames = parameters.stream()
+                                                .map(NameValuePair::getName)
+                                                .distinct()
+                                                .collect(Collectors.toList());
+
+        assertEquals("Parameter names should match", expectedNames, parameterNames);
+
+        // we can't compare directly because the servlet api collects by name
+        // this checks for the same values in the same order
+        for (final String name : expectedNames) {
+            final List<String> expectedValues = expectedParameters.stream()
+                                                    .filter(pair -> name.equals(pair.getName()))
+                                                    .map(NameValuePair::getValue)
+                                                    .collect(Collectors.toList());
+            final List<String> values = parameters.stream()
+                                                    .filter(pair -> name.equals(pair.getName()))
+                                                    .map(NameValuePair::getValue)
+                                                    .collect(Collectors.toList());
+            assertEquals("Parameter values for parameter with name '" + name + "' should match",
+                    expectedValues, values);
         }
     }
 
@@ -319,13 +354,27 @@ private static void bounceToStatic(final HttpServletRequest req, final HttpServl
         private static void bounce(final Writer writer,
                 final HttpServletRequest req, final HttpServletResponse resp) throws IOException {
             writer.write("Parameters: \n");
-            for (final String key : req.getParameterMap().keySet()) {
-                final String val = req.getParameter(key);
-                if (val == null) {
-                    writer.write("  '" + key + "': '-null-'\n");
+            // use only getParameterMap() here because we like to have the same behavior
+            for (final Map.Entry<String, String[]> entry : req.getParameterMap().entrySet()) {
+                if (entry.getValue() == null) {
+                    writer.write("  '" + entry.getKey() + "': [null]\n");
+                }
+                else if (entry.getValue().length == 0) {
+                    writer.write("  '" + entry.getKey() + "': []\n");
                 }
                 else {
-                    writer.write("  '" + key + "': '" + val + "'\n");
+                    writer.write("  '" + entry.getKey() + "': [");
+                    boolean first = true;
+                    for (final String val : entry.getValue()) {
+                        if (first) {
+                            writer.write("'" + val + "'");
+                            first = false;
+                        }
+                        else {
+                            writer.write(", '" + val + "'");
+                        }
+                    }
+                    writer.write("]\n");
                 }
             }
         }