From 1344e1ff970b279de4f16973ebbcf1be2d8e3dd4 Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Wed, 18 Jan 2012 19:26:47 +0200 Subject: [PATCH] Use simple JSON for RPC calls from client to server (#8279). A MethodInvocation can have multiple parameters, but only String is supported as a data type for them. Values to updateVariable() are still encoded using old mechanisms and sent as name/type/value string tuples. --- .../terminal/gwt/DefaultWidgetSet.gwt.xml | 2 + .../gwt/client/ApplicationConnection.java | 49 ++-- src/com/vaadin/terminal/gwt/client/Util.java | 29 +- .../communication/MethodInvocation.java | 6 +- .../server/AbstractCommunicationManager.java | 250 +++++++++++------- 5 files changed, 200 insertions(+), 136 deletions(-) diff --git a/src/com/vaadin/terminal/gwt/DefaultWidgetSet.gwt.xml b/src/com/vaadin/terminal/gwt/DefaultWidgetSet.gwt.xml index 9bc05dee2e..92b23836e3 100644 --- a/src/com/vaadin/terminal/gwt/DefaultWidgetSet.gwt.xml +++ b/src/com/vaadin/terminal/gwt/DefaultWidgetSet.gwt.xml @@ -9,6 +9,8 @@ + + diff --git a/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java b/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java index a7f146be16..6f7d6818f9 100644 --- a/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java +++ b/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java @@ -23,6 +23,8 @@ import com.google.gwt.http.client.RequestBuilder; import com.google.gwt.http.client.RequestCallback; import com.google.gwt.http.client.RequestException; import com.google.gwt.http.client.Response; +import com.google.gwt.json.client.JSONArray; +import com.google.gwt.json.client.JSONString; import com.google.gwt.user.client.Command; import com.google.gwt.user.client.DOM; import com.google.gwt.user.client.Element; @@ -76,12 +78,9 @@ public class ApplicationConnection { public static final String UPDATE_VARIABLE_METHOD = "v"; - public static final char VAR_RECORD_SEPARATOR = '\u001e'; - - public static final char VAR_FIELD_SEPARATOR = '\u001f'; - public static final char VAR_BURST_SEPARATOR = '\u001d'; + @Deprecated public static final char VAR_ARRAYITEM_SEPARATOR = '\u001c'; public static final char VAR_ESCAPE_CHARACTER = '\u001b'; @@ -1109,10 +1108,10 @@ public class ApplicationConnection { private void addVariableToQueue(String paintableId, String variableName, String encodedValue, boolean immediate, char type) { // TODO could eliminate invocations of same shared variable setter - String param = variableName + VAR_FIELD_SEPARATOR + type - + VAR_FIELD_SEPARATOR + encodedValue; addMethodInvocationToQueue(paintableId, new MethodInvocation( - paintableId, UPDATE_VARIABLE_METHOD, param), immediate); + paintableId, UPDATE_VARIABLE_METHOD, new String[] { + variableName, String.valueOf(type), encodedValue }), + immediate); } private void addMethodInvocationToQueue(String paintableId, @@ -1183,20 +1182,27 @@ public class ApplicationConnection { if (ApplicationConfiguration.isDebugMode()) { Util.logVariableBurst(this, pendingInvocations); } - // TODO use JSON for messages - for (int i = 0; i < pendingInvocations.size(); i++) { - if (i > 0) { - req.append(VAR_RECORD_SEPARATOR); + + JSONArray reqJson = new JSONArray(); + + // TODO support typed parameters + for (MethodInvocation invocation : pendingInvocations) { + JSONArray invocationJson = new JSONArray(); + invocationJson.set(0, + new JSONString(invocation.getPaintableId())); + invocationJson.set(1, + new JSONString(invocation.getMethodName())); + JSONArray paramJson = new JSONArray(); + for (int i = 0; i < invocation.getParameters().length; ++i) { + paramJson.set(i, new JSONString( + invocation.getParameters()[i])); } - MethodInvocation invocation = pendingInvocations.get(i); - req.append(invocation.getPaintableId()); - req.append(VAR_FIELD_SEPARATOR); - req.append(invocation.getMethodName()); - req.append(VAR_FIELD_SEPARATOR); - // TODO support multiple parameters - req.append(invocation.getParameters()); + invocationJson.set(2, paramJson); + reqJson.set(reqJson.size(), invocationJson); } + req.append(reqJson.toString()); + pendingInvocations.clear(); // Append all the bursts to this synchronous request if (forceSync && !pendingBursts.isEmpty()) { @@ -1531,9 +1537,8 @@ public class ApplicationConnection { } /** - * Encode burst, record, field and array item separator characters in a - * String for transport over the network. This protects from separator - * injection attacks. + * Encode burst and other separator characters in a String for transport + * over the network. This protects from separator injection attacks. * * @param value * to encode @@ -1547,8 +1552,6 @@ public class ApplicationConnection { case VAR_ESCAPE_CHARACTER: // fall-through - escape character is duplicated case VAR_BURST_SEPARATOR: - case VAR_RECORD_SEPARATOR: - case VAR_FIELD_SEPARATOR: case VAR_ARRAYITEM_SEPARATOR: result.append(VAR_ESCAPE_CHARACTER); // encode as letters for easier reading diff --git a/src/com/vaadin/terminal/gwt/client/Util.java b/src/com/vaadin/terminal/gwt/client/Util.java index 2789a73078..b0142a7b7b 100644 --- a/src/com/vaadin/terminal/gwt/client/Util.java +++ b/src/com/vaadin/terminal/gwt/client/Util.java @@ -945,25 +945,18 @@ public class Util { if (paintable != null) { VConsole.log("\t" + id + " (" + paintable.getClass() + ") :"); for (MethodInvocation invocation : invocations) { - String formattedParams = invocation.getParameters(); + String[] parameters = invocation.getParameters(); + String formattedParams = null; if (ApplicationConnection.UPDATE_VARIABLE_METHOD - .equals(invocation.getMethodName())) { - // if an updateVariable call, format as variable changes - // ensure with limit that also trailing parameters are - // included - String[] split = invocation - .getParameters() - .split(String - .valueOf(ApplicationConnection.VAR_FIELD_SEPARATOR), - 3); - - // unless of correct length, do not reformat - if (split.length == 3) { - // TODO needs to be reworked - // name, type, value - formattedParams = split[0] + " (" + split[1] + ")" - + " : " + split[2]; - } + .equals(invocation.getMethodName()) + && parameters.length == 3) { + // name, type, value + formattedParams = parameters[0] + " (" + parameters[1] + + ")" + " : " + parameters[2]; + } + if (null == formattedParams) { + formattedParams = (null != parameters) ? parameters + .toString() : null; } VConsole.log("\t\t" + invocation.getMethodName() + "(" + formattedParams + ")"); diff --git a/src/com/vaadin/terminal/gwt/client/communication/MethodInvocation.java b/src/com/vaadin/terminal/gwt/client/communication/MethodInvocation.java index c07427fc9d..92ae528367 100644 --- a/src/com/vaadin/terminal/gwt/client/communication/MethodInvocation.java +++ b/src/com/vaadin/terminal/gwt/client/communication/MethodInvocation.java @@ -10,11 +10,11 @@ public class MethodInvocation { private final String paintableId; private final String methodName; - private final String parameters; + private final String[] parameters; // TODO Parameters should be an Object[]? public MethodInvocation(String paintableId, String methodName, - String parameters) { + String[] parameters) { this.paintableId = paintableId; this.methodName = methodName; this.parameters = parameters; @@ -28,7 +28,7 @@ public class MethodInvocation { return methodName; } - public String getParameters() { + public String[] getParameters() { return parameters; } } \ No newline at end of file diff --git a/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java b/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java index c71bb0ea10..c359d70ea7 100644 --- a/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java +++ b/src/com/vaadin/terminal/gwt/server/AbstractCommunicationManager.java @@ -45,6 +45,7 @@ import java.util.logging.Logger; import com.vaadin.Application; import com.vaadin.Application.SystemMessages; import com.vaadin.RootRequiresMoreInformationException; +import com.vaadin.external.json.JSONArray; import com.vaadin.external.json.JSONException; import com.vaadin.external.json.JSONObject; import com.vaadin.terminal.CombinedRequest; @@ -62,6 +63,7 @@ import com.vaadin.terminal.VariableOwner; import com.vaadin.terminal.WrappedRequest; import com.vaadin.terminal.WrappedResponse; import com.vaadin.terminal.gwt.client.ApplicationConnection; +import com.vaadin.terminal.gwt.client.communication.MethodInvocation; import com.vaadin.terminal.gwt.server.BootstrapHandler.BootstrapContext; import com.vaadin.terminal.gwt.server.ComponentSizeValidator.InvalidLayout; import com.vaadin.ui.AbstractComponent; @@ -140,12 +142,9 @@ public abstract class AbstractCommunicationManager implements private static final char VTYPE_STRINGARRAY = 'c'; private static final char VTYPE_MAP = 'm'; - private static final char VAR_RECORD_SEPARATOR = '\u001e'; - - private static final char VAR_FIELD_SEPARATOR = '\u001f'; - public static final char VAR_BURST_SEPARATOR = '\u001d'; + @Deprecated public static final char VAR_ARRAYITEM_SEPARATOR = '\u001c'; public static final char VAR_ESCAPE_CHARACTER = '\u001b'; @@ -1182,104 +1181,173 @@ public abstract class AbstractCommunicationManager implements return success; } + /** + * Helper class for parsing variable change RPC calls. + * + * TODO refactor the code related to this, maybe eliminate this class + */ + private static class VariableChange { + private final String name; + private final char type; + private final String value; + + public VariableChange(MethodInvocation invocation) { + name = invocation.getParameters()[0]; + type = invocation.getParameters()[1].charAt(0); + value = invocation.getParameters()[2]; + } + + public String getName() { + return name; + } + + public char getType() { + return type; + } + + public String getValue() { + return value; + } + } + public boolean handleBurst(Object source, Application app, boolean success, final String burst) { - // extract variables to two dim string array - final String[] tmp = burst.split(String.valueOf(VAR_RECORD_SEPARATOR)); - // TODO support variable number of parameters - final String[][] variableRecords = new String[tmp.length][5]; - for (int i = 0; i < tmp.length; i++) { - // ensure with limit that also trailing parameters are included - variableRecords[i] = tmp[i].split( - String.valueOf(VAR_FIELD_SEPARATOR), 5); - } - - for (int i = 0; i < variableRecords.length; i++) { - String[] variable = variableRecords[i]; - String[] nextVariable = null; - if (i + 1 < variableRecords.length) { - nextVariable = variableRecords[i + 1]; - } - final VariableOwner owner = getVariableOwner(variable[VAR_PID]); - final String methodName = variable[VAR_METHOD]; - if (!ApplicationConnection.UPDATE_VARIABLE_METHOD - .equals(methodName)) { - // TODO handle other RPC calls - continue; - } - if (owner != null && owner.isEnabled()) { - Map m; - if (nextVariable != null - && variable[VAR_PID].equals(nextVariable[VAR_PID])) { - // we have more than one value changes in row for - // one variable owner, collect them in HashMap - m = new HashMap(); - m.put(variable[VAR_VARNAME], - convertVariableValue(variable[VAR_TYPE].charAt(0), - variable[VAR_VALUE])); - } else { - // use optimized single value map - m = Collections.singletonMap( - variable[VAR_VARNAME], - convertVariableValue(variable[VAR_TYPE].charAt(0), - variable[VAR_VALUE])); - } + try { + List invocations = parseInvocations(burst); - // collect following variable changes for this owner - while (nextVariable != null - && variable[VAR_PID].equals(nextVariable[VAR_PID])) { - i++; - variable = nextVariable; - if (i + 1 < variableRecords.length) { - nextVariable = variableRecords[i + 1]; - } else { - nextVariable = null; - } - m.put(variable[VAR_VARNAME], - convertVariableValue(variable[VAR_TYPE].charAt(0), - variable[VAR_VALUE])); + // perform the method invocations, grouping consecutive variable + // changes for the same paintable + // TODO simplify to do each call separately? breaks old semantics + for (int i = 0; i < invocations.size(); i++) { + MethodInvocation invocation = invocations.get(i); + + MethodInvocation nextInvocation = null; + if (i + 1 < invocations.size()) { + nextInvocation = invocations.get(i + 1); } - try { - changeVariables(source, owner, m); - } catch (Exception e) { - if (owner instanceof Component) { - handleChangeVariablesError(app, (Component) owner, e, m); - } else { - // TODO DragDropService error handling - throw new RuntimeException(e); + + final VariableOwner owner = getVariableOwner(invocation + .getPaintableId()); + final String methodName = invocation.getMethodName(); + + if (owner != null && owner.isEnabled()) { + if (!ApplicationConnection.UPDATE_VARIABLE_METHOD + .equals(methodName)) { + // TODO handle other RPC calls + continue; } - } - } else { - // Handle special case where window-close is called - // after the window has been removed from the - // application or the application has closed - if ("close".equals(variable[VAR_VARNAME]) - && "true".equals(variable[VAR_VALUE])) { - // Silently ignore this - continue; - } + VariableChange change = new VariableChange(invocation); + + // TODO could optimize with a single value map if only one + // change for a paintable + + Map m = new HashMap(); + m.put(change.getName(), + convertVariableValue(change.getType(), + change.getValue())); + while (nextInvocation != null + && invocation.getPaintableId().equals( + nextInvocation.getPaintableId())) { + change = new VariableChange(invocation); + m.put(change.getName(), + convertVariableValue(change.getType(), + change.getValue())); + i++; + invocation = nextInvocation; + if (i + 1 < invocations.size()) { + nextInvocation = invocations.get(i + 1); + } else { + nextInvocation = null; + } + } - // Ignore variable change - String msg = "Warning: Ignoring variable change for "; - if (owner != null) { - msg += "disabled component " + owner.getClass(); - String caption = ((Component) owner).getCaption(); - if (caption != null) { - msg += ", caption=" + caption; + try { + changeVariables(source, owner, m); + } catch (Exception e) { + if (owner instanceof Component) { + handleChangeVariablesError(app, (Component) owner, + e, m); + } else { + // TODO DragDropService error handling + throw new RuntimeException(e); + } } } else { - msg += "non-existent component, VAR_PID=" - + variable[VAR_PID]; - success = false; + // TODO convert window close to a separate RPC call, not a + // variable change + + VariableChange change = new VariableChange(invocation); + + // Handle special case where window-close is called + // after the window has been removed from the + // application or the application has closed + if ("close".equals(change.getName()) + && "true".equals(change.getValue())) { + // Silently ignore this + continue; + } + + // Ignore variable change + String msg = "Warning: Ignoring RPC call for "; + if (owner != null) { + msg += "disabled component " + owner.getClass(); + String caption = ((Component) owner).getCaption(); + if (caption != null) { + msg += ", caption=" + caption; + } + } else { + msg += "non-existent component, VAR_PID=" + + invocation.getPaintableId(); + success = false; + } + logger.warning(msg); + continue; } - logger.warning(msg); - continue; } + + } catch (JSONException e) { + logger.warning("Unable to parse RPC call from the client: " + + e.getMessage()); + throw new RuntimeException(e); } + return success; } + /** + * Parse a message burst from the client into a list of MethodInvocation + * instances. + * + * @param burst + * message string (JSON) + * @return list of MethodInvocation to perform + * @throws JSONException + */ + private List parseInvocations(final String burst) + throws JSONException { + JSONArray invocationsJson = new JSONArray(burst); + + ArrayList invocations = new ArrayList(); + + // parse JSON to MethodInvocations + for (int i = 0; i < invocationsJson.length(); ++i) { + JSONArray invocationJson = invocationsJson.getJSONArray(i); + String paintableId = invocationJson.getString(0); + String methodName = invocationJson.getString(1); + JSONArray parametersJson = invocationJson.getJSONArray(2); + String[] parameters = new String[parametersJson.length()]; + // TODO support typed parameters + for (int j = 0; j < parametersJson.length(); ++j) { + parameters[j] = parametersJson.getString(j); + } + MethodInvocation invocation = new MethodInvocation(paintableId, + methodName, parameters); + invocations.add(invocation); + } + return invocations; + } + protected void changeVariables(Object source, final VariableOwner owner, Map m) { owner.changeVariables(source, m); @@ -1488,9 +1556,9 @@ public abstract class AbstractCommunicationManager implements } /** - * Decode encoded burst, record, field and array item separator characters - * in a variable value String received from the client. This protects from - * separator injection attacks. + * Decode encoded burst and other separator characters in a variable value + * String received from the client. This protects from separator injection + * attacks. * * @param encodedValue * to decode @@ -1510,8 +1578,6 @@ public abstract class AbstractCommunicationManager implements result.append(VAR_ESCAPE_CHARACTER); break; case VAR_BURST_SEPARATOR + 0x30: - case VAR_RECORD_SEPARATOR + 0x30: - case VAR_FIELD_SEPARATOR + 0x30: case VAR_ARRAYITEM_SEPARATOR + 0x30: // +0x30 makes these letters for easier reading result.append((char) (character - 0x30)); -- 2.39.5