HttpProxy: prevent lost content for POST requests with query parameters

Problem:
When performing a POST request with JSON as payload via HttpProxy, and
the URL contains an additional query parameter (?x=1), the original
payload ist lost. The special handling for requests that contain
parameters is only valid for HTTP form submissions with content type
application/x-www-form-urlencoded, not for REST calls.

Solution:
Check content type before applying special handling.

265484

Change-Id: I8130a6531e34045f6fa4cfce81cdd3af87150bcd
Reviewed-on: https://git.eclipse.org/r/159724
Tested-by: CI Bot
Reviewed-by: Claudio Guglielmo <claudio.guglielmo@bsiag.com>

2 files changed, 49 insertions, 11 deletions

diff --git a/org.eclipse.scout.rt.server.commons.test/src/test/java/org/eclipse/scout/rt/server/commons/servlet/HttpProxyTest.java b/org.eclipse.scout.rt.server.commons.test/src/test/java/org/eclipse/scout/rt/server/commons/servlet/HttpProxyTest.java
index 88efb0aff7..791a457a95 100644
--- a/org.eclipse.scout.rt.server.commons.test/src/test/java/org/eclipse/scout/rt/server/commons/servlet/HttpProxyTest.java
+++ b/org.eclipse.scout.rt.server.commons.test/src/test/java/org/eclipse/scout/rt/server/commons/servlet/HttpProxyTest.java
@@ -10,18 +10,11 @@
  */
 package org.eclipse.scout.rt.server.commons.servlet;
 
-import static java.util.Collections.emptyMap;
-import static java.util.Collections.singletonMap;
+import static java.util.Collections.*;
 import static org.eclipse.scout.rt.platform.util.CollectionUtility.hashMap;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
 import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.atLeast;
-import static org.mockito.Mockito.atLeastOnce;
-import static org.mockito.Mockito.doAnswer;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyNoMoreInteractions;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -78,6 +71,34 @@ public class HttpProxyTest {
   }
 
   @Test
+  public void testShouldWriteParametersAsPayload() {
+    Map<String, String[]> oneParameter = CollectionUtility.hashMap(ImmutablePair.of("maxRows", new String[]{"2"}));
+    Map<String, String[]> multiParameters = CollectionUtility.hashMap(ImmutablePair.of("name", new String[]{"alice"}), ImmutablePair.of("pets", new String[]{"dog", "cat"}));
+
+    assertFalse(m_proxy.shouldWriteParametersAsPayload(mockRequest("POST", "application/json", null)));
+    assertFalse(m_proxy.shouldWriteParametersAsPayload(mockRequest("POST", "application/json", oneParameter)));
+    assertFalse(m_proxy.shouldWriteParametersAsPayload(mockRequest("POST", "application/json", multiParameters)));
+
+    assertFalse(m_proxy.shouldWriteParametersAsPayload(mockRequest("POST", "application/x-www-form-urlencoded", null)));
+    assertTrue(m_proxy.shouldWriteParametersAsPayload(mockRequest("POST", "application/x-www-form-urlencoded", oneParameter)));
+    assertTrue(m_proxy.shouldWriteParametersAsPayload(mockRequest("POST", "application/x-www-form-urlencoded", multiParameters)));
+  }
+
+  private HttpServletRequest mockRequest(String method, String contentType, Map<String, String[]> parameterMap) {
+    HttpServletRequest req = mock(HttpServletRequest.class);
+    if (method != null) {
+      when(req.getMethod()).thenReturn(method);
+    }
+    if (contentType != null) {
+      when(req.getContentType()).thenReturn(contentType);
+    }
+    if (parameterMap != null) {
+      when(req.getParameterMap()).thenReturn(parameterMap);
+    }
+    return req;
+  }
+
+  @Test
   public void testGetConnectionHeaderValuesHttpRequest() throws Exception {
     assertGetConnectionHeaderValues(null);
     assertGetConnectionHeaderValues("Keep-Alive", "keep-alive");
diff --git a/org.eclipse.scout.rt.server.commons/src/main/java/org/eclipse/scout/rt/server/commons/servlet/HttpProxy.java b/org.eclipse.scout.rt.server.commons/src/main/java/org/eclipse/scout/rt/server/commons/servlet/HttpProxy.java
index 1912b4ed7e..c59b37f0a7 100644
--- a/org.eclipse.scout.rt.server.commons/src/main/java/org/eclipse/scout/rt/server/commons/servlet/HttpProxy.java
+++ b/org.eclipse.scout.rt.server.commons/src/main/java/org/eclipse/scout/rt/server/commons/servlet/HttpProxy.java
@@ -117,6 +117,23 @@ public class HttpProxy {
   }
 
   /**
+   * @return Whether the {@linkplain HttpServletRequest#getParameterMap() request parameters} should be written as
+   *         payload instead of the {@linkplain HttpServletRequest#getInputStream() original payload}.
+   *         <p>
+   *         This is mostly relevant for form submissions (content type <code>application/x-www-form-urlencoded</code>).
+   *         Because the servlet container parses the parameters from the payload, they cannot be read again from the
+   *         request body. Instead, they have to be read from the parameter map and be converted back to a valid body.
+   * @see #writeRequestParameters(HttpServletRequest, HttpRequest)
+   */
+  protected boolean shouldWriteParametersAsPayload(HttpServletRequest req) {
+    if (req.getParameterMap().isEmpty()) {
+      return false;
+    }
+    // https://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.1
+    return "application/x-www-form-urlencoded".equals(req.getContentType());
+  }
+
+  /**
    * Forwards the given request to the remote URL using the given method.
    * <ul>
    * <li>Adds every request header beside the blacklisted to the forwarded request.<br>
@@ -148,7 +165,7 @@ public class HttpProxy {
     if (shouldIncludeRequestPayload(req)) {
       // Payload is empty if parameters are used (usually with content type = application/x-www-form-urlencoded)
       // -> write parameters if there are any, otherwise write the raw payload
-      if (!req.getParameterMap().isEmpty()) {
+      if (shouldWriteParametersAsPayload(req)) {
         writeRequestParameters(req, httpReq);
       }
       else {