Rewrite client request into JSON (#9269, #11257)

Change-Id: I0001d54f890ad8e5787d1f6c076d1f1d75dd32d2

6 files changed, 121 insertions, 144 deletions

diff --git a/client/src/com/vaadin/client/ApplicationConnection.java b/client/src/com/vaadin/client/ApplicationConnection.java
index 2865e04757..364ce4521d 100644
--- a/client/src/com/vaadin/client/ApplicationConnection.java
+++ b/client/src/com/vaadin/client/ApplicationConnection.java
@@ -90,6 +90,7 @@ import com.vaadin.client.ui.ui.UIConnector;
 import com.vaadin.client.ui.window.WindowConnector;
 import com.vaadin.shared.AbstractComponentState;
 import com.vaadin.shared.ApplicationConstants;
+import com.vaadin.shared.JsonConstants;
 import com.vaadin.shared.Version;
 import com.vaadin.shared.communication.LegacyChangeVariablesInvocation;
 import com.vaadin.shared.communication.MethodInvocation;
@@ -136,10 +137,6 @@ public class ApplicationConnection {
 
     public static final String ERROR_CLASSNAME_EXT = "-error";
 
-    public static final char VAR_BURST_SEPARATOR = '\u001d';
-
-    public static final char VAR_ESCAPE_CHARACTER = '\u001b';
-
     /**
      * A string that, if found in a non-JSON response to a UIDL request, will
      * cause the browser to refresh the page. If followed by a colon, optional
@@ -675,7 +672,7 @@ public class ApplicationConnection {
     }-*/;
 
     protected void repaintAll() {
-        makeUidlRequest("", getRepaintAllParameters());
+        makeUidlRequest(new JSONArray(), getRepaintAllParameters());
     }
 
     /**
@@ -706,20 +703,23 @@ public class ApplicationConnection {
     /**
      * Makes an UIDL request to the server.
      * 
-     * @param requestData
-     *            Data that is passed to the server.
+     * @param reqInvocations
+     *            Data containing RPC invocations and all related information.
      * @param extraParams
      *            Parameters that are added as GET parameters to the url.
      *            Contains key=value pairs joined by & characters or is empty if
      *            no parameters should be added. Should not start with any
      *            special character.
      */
-    protected void makeUidlRequest(final String requestData,
+    protected void makeUidlRequest(final JSONArray reqInvocations,
             final String extraParams) {
         startRequest();
-        // Security: double cookie submission pattern
-        final String payload = getCsrfToken() + VAR_BURST_SEPARATOR
-                + requestData;
+
+        JSONObject payload = new JSONObject();
+        payload.put(ApplicationConstants.CSRF_TOKEN, new JSONString(
+                getCsrfToken()));
+        payload.put(ApplicationConstants.RPC_INVOCATIONS, reqInvocations);
+
         VConsole.log("Making UIDL Request with params: " + payload);
         String uri = translateVaadinUri(ApplicationConstants.APP_PROTOCOL_PREFIX
                 + ApplicationConstants.UIDL_PATH + '/');
@@ -743,7 +743,7 @@ public class ApplicationConnection {
      * @param payload
      *            The contents of the request to send
      */
-    protected void doUidlRequest(final String uri, final String payload) {
+    protected void doUidlRequest(final String uri, final JSONObject payload) {
         RequestCallback requestCallback = new RequestCallback() {
             @Override
             public void onError(Request request, Throwable exception) {
@@ -906,14 +906,14 @@ public class ApplicationConnection {
      * @throws RequestException
      *             if the request could not be sent
      */
-    protected void doAjaxRequest(String uri, String payload,
+    protected void doAjaxRequest(String uri, JSONObject payload,
             RequestCallback requestCallback) throws RequestException {
         RequestBuilder rb = new RequestBuilder(RequestBuilder.POST, uri);
         // TODO enable timeout
         // rb.setTimeoutMillis(timeoutMillis);
         // TODO this should be configurable
-        rb.setHeader("Content-Type", "text/plain;charset=utf-8");
-        rb.setRequestData(payload);
+        rb.setHeader("Content-Type", JsonConstants.JSON_CONTENT_TYPE);
+        rb.setRequestData(payload.toString());
         rb.setCallback(requestCallback);
 
         final Request request = rb.send();
@@ -2468,15 +2468,13 @@ public class ApplicationConnection {
      */
     private void buildAndSendVariableBurst(
             LinkedHashMap<String, MethodInvocation> pendingInvocations) {
-        final StringBuffer req = new StringBuffer();
 
-        while (!pendingInvocations.isEmpty()) {
+        JSONArray reqJson = new JSONArray();
+        if (!pendingInvocations.isEmpty()) {
             if (ApplicationConfiguration.isDebugMode()) {
                 Util.logVariableBurst(this, pendingInvocations.values());
             }
 
-            JSONArray reqJson = new JSONArray();
-
             for (MethodInvocation invocation : pendingInvocations.values()) {
                 JSONArray invocationJson = new JSONArray();
                 invocationJson.set(0,
@@ -2515,9 +2513,6 @@ public class ApplicationConnection {
                 reqJson.set(reqJson.size(), invocationJson);
             }
 
-            // escape burst separators (if any)
-            req.append(escapeBurstContents(reqJson.toString()));
-
             pendingInvocations.clear();
             // Keep tag string short
             lastInvocationTag = 0;
@@ -2541,7 +2536,7 @@ public class ApplicationConnection {
 
             getConfiguration().setWidgetsetVersionSent();
         }
-        makeUidlRequest(req.toString(), extraParams);
+        makeUidlRequest(reqJson, extraParams);
     }
 
     private boolean isJavascriptRpc(MethodInvocation invocation) {
@@ -2785,35 +2780,6 @@ public class ApplicationConnection {
     }
 
     /**
-     * Encode burst separator characters in a String for transport over the
-     * network. This protects from separator injection attacks.
-     * 
-     * @param value
-     *            to encode
-     * @return encoded value
-     */
-    protected String escapeBurstContents(String value) {
-        final StringBuilder result = new StringBuilder();
-        for (int i = 0; i < value.length(); ++i) {
-            char character = value.charAt(i);
-            switch (character) {
-            case VAR_ESCAPE_CHARACTER:
-                // fall-through - escape character is duplicated
-            case VAR_BURST_SEPARATOR:
-                result.append(VAR_ESCAPE_CHARACTER);
-                // encode as letters for easier reading
-                result.append(((char) (character + 0x30)));
-                break;
-            default:
-                // the char is not a special one - add it to the result as is
-                result.append(character);
-                break;
-            }
-        }
-        return result.toString();
-    }
-
-    /**
      * Does absolutely nothing. Replaced by {@link LayoutManager}.
      * 
      * @param container
diff --git a/client/src/com/vaadin/client/communication/AtmospherePushConnection.java b/client/src/com/vaadin/client/communication/AtmospherePushConnection.java
index 20ccd45173..94ea0aaab2 100644
--- a/client/src/com/vaadin/client/communication/AtmospherePushConnection.java
+++ b/client/src/com/vaadin/client/communication/AtmospherePushConnection.java
@@ -20,6 +20,7 @@ import java.util.ArrayList;
 
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.core.client.Scheduler;
+import com.google.gwt.json.client.JSONObject;
 import com.google.gwt.user.client.Command;
 import com.vaadin.client.ApplicationConnection;
 import com.vaadin.client.ApplicationConnection.CommunicationErrorHandler;
@@ -109,7 +110,7 @@ public class AtmospherePushConnection implements PushConnection {
 
     private JavaScriptObject socket;
 
-    private ArrayList<String> messageQueue = new ArrayList<String>();
+    private ArrayList<JSONObject> messageQueue = new ArrayList<JSONObject>();
 
     private State state = State.CONNECT_PENDING;
 
@@ -190,14 +191,8 @@ public class AtmospherePushConnection implements PushConnection {
         }
     }
 
-    /*
-     * (non-Javadoc)
-     * 
-     * @see
-     * com.vaadin.client.communication.PushConenction#push(java.lang.String)
-     */
     @Override
-    public void push(String message) {
+    public void push(JSONObject message) {
         switch (state) {
         case CONNECT_PENDING:
             assert isActive();
@@ -209,12 +204,13 @@ public class AtmospherePushConnection implements PushConnection {
             VConsole.log("Sending push message: " + message);
 
             if (transport.equals("websocket")) {
-                FragmentedMessage fragmented = new FragmentedMessage(message);
+                FragmentedMessage fragmented = new FragmentedMessage(
+                        message.toString());
                 while (fragmented.hasNextFragment()) {
                     doPush(socket, fragmented.getNextFragment());
                 }
             } else {
-                doPush(socket, message);
+                doPush(socket, message.toString());
             }
             break;
         case DISCONNECT_PENDING:
@@ -235,7 +231,7 @@ public class AtmospherePushConnection implements PushConnection {
         switch (state) {
         case CONNECT_PENDING:
             state = State.CONNECTED;
-            for (String message : messageQueue) {
+            for (JSONObject message : messageQueue) {
                 push(message);
             }
             messageQueue.clear();
diff --git a/client/src/com/vaadin/client/communication/PushConnection.java b/client/src/com/vaadin/client/communication/PushConnection.java
index a7eba224be..ba79af9d2c 100644
--- a/client/src/com/vaadin/client/communication/PushConnection.java
+++ b/client/src/com/vaadin/client/communication/PushConnection.java
@@ -16,6 +16,7 @@
 
 package com.vaadin.client.communication;
 
+import com.google.gwt.json.client.JSONObject;
 import com.google.gwt.user.client.Command;
 import com.vaadin.client.ApplicationConnection;
 import com.vaadin.client.ApplicationConnection.CommunicationErrorHandler;
@@ -53,14 +54,14 @@ public interface PushConnection {
      * replay those messages in the original order when the connection has been
      * established.
      * 
-     * @param message
-     *            the message to push
+     * @param payload
+     *            the payload to push
      * @throws IllegalStateException
      *             if this connection is not active
      * 
      * @see #isActive()
      */
-    public void push(String message);
+    public void push(JSONObject payload);
 
     /**
      * Checks whether this push connection is in a state where it can push
diff --git a/server/src/com/vaadin/server/communication/ServerRpcHandler.java b/server/src/com/vaadin/server/communication/ServerRpcHandler.java
index 3cc85909ee..e52ef6d037 100644
--- a/server/src/com/vaadin/server/communication/ServerRpcHandler.java
+++ b/server/src/com/vaadin/server/communication/ServerRpcHandler.java
@@ -20,8 +20,6 @@ import java.io.IOException;
 import java.io.Reader;
 import java.io.Serializable;
 import java.lang.reflect.Type;
-import java.text.CharacterIterator;
-import java.text.StringCharacterIterator;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -32,6 +30,7 @@ import java.util.logging.Logger;
 
 import org.json.JSONArray;
 import org.json.JSONException;
+import org.json.JSONObject;
 
 import com.vaadin.server.ClientConnector;
 import com.vaadin.server.JsonCodec;
@@ -62,10 +61,59 @@ import com.vaadin.ui.UI;
  */
 public class ServerRpcHandler implements Serializable {
 
-    /* Variable records indexes */
-    public static final char VAR_BURST_SEPARATOR = '\u001d';
+    /**
+     * A data transfer object representing an RPC request sent by the client
+     * side.
+     * 
+     * @since 7.2
+     * @author Vaadin Ltd
+     */
+    public static class RpcRequest {
+
+        private final String csrfToken;
+        private final JSONArray invocations;
+        private final JSONObject json;
 
-    public static final char VAR_ESCAPE_CHARACTER = '\u001b';
+        public RpcRequest(String jsonString) throws JSONException {
+            json = new JSONObject(jsonString);
+            csrfToken = json.getString(ApplicationConstants.CSRF_TOKEN);
+            invocations = new JSONArray(
+                    json.getString(ApplicationConstants.RPC_INVOCATIONS));
+        }
+
+        /**
+         * Gets the CSRF security token (double submit cookie) for this request.
+         * 
+         * @return the CSRF security token for this current change request
+         */
+        public String getCsrfToken() {
+            return csrfToken;
+        }
+
+        /**
+         * Gets the data to recreate the RPC as requested by the client side.
+         * 
+         * @return the data describing which RPC should be made, and all their
+         *         data
+         */
+        public JSONArray getRpcInvocationsData() {
+            return invocations;
+        }
+
+        /**
+         * Gets the entire request in JSON format, as it was received from the
+         * client.
+         * <p>
+         * <em>Note:</em> This is a shared reference - any modifications made
+         * will be shared.
+         * 
+         * @return the raw JSON object that was received from the client
+         * 
+         */
+        public JSONObject getRawJson() {
+            return json;
+        }
+    }
 
     private static final int MAX_BUFFER_SIZE = 64 * 1024;
 
@@ -90,45 +138,42 @@ public class ServerRpcHandler implements Serializable {
             throws IOException, InvalidUIDLSecurityKeyException, JSONException {
         ui.getSession().setLastRequestTimestamp(System.currentTimeMillis());
 
-        String changes = getMessage(reader);
-
-        final String[] bursts = changes.split(String
-                .valueOf(VAR_BURST_SEPARATOR));
+        String changeMessage = getMessage(reader);
 
-        if (bursts.length > 2) {
-            throw new RuntimeException(
-                    "Multiple variable bursts not supported in Vaadin 7");
-        } else if (bursts.length <= 1) {
+        if (changeMessage == null || changeMessage.equals("")) {
             // The client sometimes sends empty messages, this is probably a bug
             return;
         }
 
+        RpcRequest rpcRequest = new RpcRequest(changeMessage);
+
         // Security: double cookie submission pattern unless disabled by
         // property
-        if (!VaadinService.isCsrfTokenValid(ui.getSession(), bursts[0])) {
+        if (!VaadinService.isCsrfTokenValid(ui.getSession(),
+                rpcRequest.getCsrfToken())) {
             throw new InvalidUIDLSecurityKeyException("");
         }
-        handleBurst(ui, unescapeBurst(bursts[1]));
+        handleInvocations(ui, rpcRequest.getRpcInvocationsData());
     }
 
     /**
-     * Processes a message burst received from the client.
-     * 
-     * A burst can contain any number of RPC calls, including legacy variable
-     * change calls that are processed separately.
-     * 
+     * Processes invocations data received from the client.
+     * <p>
+     * The invocations data can contain any number of RPC calls, including
+     * legacy variable change calls that are processed separately.
+     * <p>
      * Consecutive changes to the value of the same variable are combined and
      * changeVariables() is only called once for them. This preserves the Vaadin
      * 6 semantics for components and add-ons that do not use Vaadin 7 RPC
      * directly.
      * 
-     * @param source
      * @param uI
-     *            the UI receiving the burst
-     * @param burst
-     *            the content of the burst as a String to be parsed
+     *            the UI receiving the invocations data
+     * @param invocationsData
+     *            JSON containing all information needed to execute all
+     *            requested RPC calls.
      */
-    private void handleBurst(UI uI, String burst) {
+    private void handleInvocations(UI uI, JSONArray invocationsData) {
         // TODO PUSH Refactor so that this is not needed
         LegacyCommunicationManager manager = uI.getSession()
                 .getCommunicationManager();
@@ -137,7 +182,7 @@ public class ServerRpcHandler implements Serializable {
             Set<Connector> enabledConnectors = new HashSet<Connector>();
 
             List<MethodInvocation> invocations = parseInvocations(
-                    uI.getConnectorTracker(), burst);
+                    uI.getConnectorTracker(), invocationsData);
             for (MethodInvocation invocation : invocations) {
                 final ClientConnector connector = manager.getConnector(uI,
                         invocation.getConnectorId());
@@ -250,21 +295,19 @@ public class ServerRpcHandler implements Serializable {
     }
 
     /**
-     * Parse a message burst from the client into a list of MethodInvocation
-     * instances.
+     * Parse JSON from the client into a list of MethodInvocation instances.
      * 
      * @param connectorTracker
      *            The ConnectorTracker used to lookup connectors
-     * @param burst
-     *            message string (JSON)
+     * @param invocationsJson
+     *            JSON containing all information needed to execute all
+     *            requested RPC calls.
      * @return list of MethodInvocation to perform
      * @throws JSONException
      */
     private List<MethodInvocation> parseInvocations(
-            ConnectorTracker connectorTracker, String burst)
+            ConnectorTracker connectorTracker, JSONArray invocationsJson)
             throws JSONException {
-        JSONArray invocationsJson = new JSONArray(burst);
-
         ArrayList<MethodInvocation> invocations = new ArrayList<MethodInvocation>();
 
         MethodInvocation previousInvocation = null;
@@ -403,50 +446,6 @@ public class ServerRpcHandler implements Serializable {
         owner.changeVariables(source, m);
     }
 
-    /**
-     * Unescape encoded burst separator characters in a burst received from the
-     * client. This protects from separator injection attacks.
-     * 
-     * @param encodedValue
-     *            to decode
-     * @return decoded value
-     */
-    protected String unescapeBurst(String encodedValue) {
-        final StringBuilder result = new StringBuilder();
-        final StringCharacterIterator iterator = new StringCharacterIterator(
-                encodedValue);
-        char character = iterator.current();
-        while (character != CharacterIterator.DONE) {
-            if (VAR_ESCAPE_CHARACTER == character) {
-                character = iterator.next();
-                switch (character) {
-                case VAR_ESCAPE_CHARACTER + 0x30:
-                    // escaped escape character
-                    result.append(VAR_ESCAPE_CHARACTER);
-                    break;
-                case VAR_BURST_SEPARATOR + 0x30:
-                    // +0x30 makes these letters for easier reading
-                    result.append((char) (character - 0x30));
-                    break;
-                case CharacterIterator.DONE:
-                    // error
-                    throw new RuntimeException(
-                            "Communication error: Unexpected end of message");
-                default:
-                    // other escaped character - probably a client-server
-                    // version mismatch
-                    throw new RuntimeException(
-                            "Invalid escaped character from the client - check that the widgetset and server versions match");
-                }
-            } else {
-                // not a special character - add it to the result as is
-                result.append(character);
-            }
-            character = iterator.next();
-        }
-        return result.toString();
-    }
-
     protected String getMessage(Reader reader) throws IOException {
 
         StringBuilder sb = new StringBuilder(MAX_BUFFER_SIZE);
diff --git a/server/src/com/vaadin/server/communication/UIInitHandler.java b/server/src/com/vaadin/server/communication/UIInitHandler.java
index b2c17d00c1..6ab9d9dc58 100644
--- a/server/src/com/vaadin/server/communication/UIInitHandler.java
+++ b/server/src/com/vaadin/server/communication/UIInitHandler.java
@@ -37,6 +37,7 @@ import com.vaadin.server.VaadinResponse;
 import com.vaadin.server.VaadinService;
 import com.vaadin.server.VaadinSession;
 import com.vaadin.shared.ApplicationConstants;
+import com.vaadin.shared.JsonConstants;
 import com.vaadin.shared.communication.PushMode;
 import com.vaadin.shared.ui.ui.Transport;
 import com.vaadin.shared.ui.ui.UIConstants;
@@ -107,7 +108,7 @@ public abstract class UIInitHandler extends SynchronizedRequestHandler {
     static boolean commitJsonResponse(VaadinRequest request,
             VaadinResponse response, String json) throws IOException {
         // The response was produced without errors so write it to the client
-        response.setContentType("application/json; charset=UTF-8");
+        response.setContentType(JsonConstants.JSON_CONTENT_TYPE);
 
         // Ensure that the browser does not cache UIDL responses.
         // iOS 6 Safari requires this (#9732)
diff --git a/shared/src/com/vaadin/shared/ApplicationConstants.java b/shared/src/com/vaadin/shared/ApplicationConstants.java
index d7de435735..1dff1e19cc 100644
--- a/shared/src/com/vaadin/shared/ApplicationConstants.java
+++ b/shared/src/com/vaadin/shared/ApplicationConstants.java
@@ -82,4 +82,18 @@ public class ApplicationConstants implements Serializable {
      * Name of the parameter used to transmit the CSRF token.
      */
     public static final String CSRF_TOKEN_PARAMETER = "v-csrfToken";
+
+    /**
+     * The name of the parameter used to transmit RPC invocations
+     * 
+     * @since 7.2
+     */
+    public static final String RPC_INVOCATIONS = "rpc";
+
+    /**
+     * The name of the parameter used to transmit the CSRF token
+     * 
+     * @since 7.2
+     */
+    public static final String CSRF_TOKEN = "csrfToken";
 }