From c14334f284c7e7c344b2983726b9242e3ef8562e Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Mon, 2 Sep 2013 16:48:15 +0300 Subject: [PATCH] Rewrite client request into JSON (#9269, #11257) Change-Id: I0001d54f890ad8e5787d1f6c076d1f1d75dd32d2 --- .../vaadin/client/ApplicationConnection.java | 70 +++----- .../AtmospherePushConnection.java | 18 +-- .../client/communication/PushConnection.java | 7 +- .../communication/ServerRpcHandler.java | 153 +++++++++--------- .../server/communication/UIInitHandler.java | 3 +- .../vaadin/shared/ApplicationConstants.java | 14 ++ 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 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) { @@ -2784,35 +2779,6 @@ public class ApplicationConnection { addVariableToQueue(paintableId, variableName, values, immediate); } - /** - * 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}. * 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 messageQueue = new ArrayList(); + private ArrayList messageQueue = new ArrayList(); 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. + *

+ * Note: 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. + *

+ * The invocations data can contain any number of RPC calls, including + * legacy variable change calls that are processed separately. + *

* 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 enabledConnectors = new HashSet(); List 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 parseInvocations( - ConnectorTracker connectorTracker, String burst) + ConnectorTracker connectorTracker, JSONArray invocationsJson) throws JSONException { - JSONArray invocationsJson = new JSONArray(burst); - ArrayList invocations = new ArrayList(); 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"; } -- 2.39.5