From 42c4b2097bc955ca18107c449e04f0d5d5683ca3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Johannes=20Dahlstr=C3=B6m?= Date: Mon, 20 May 2013 15:08:48 +0300 Subject: [PATCH] Use "\0" instead of "|" as a push message delimiter (#11692) Used with TrackMessageSizeInterceptor and with client-to-server Websocket message splitting (see #11648) The original issue that the delimiter can not appear in the message (unescaped) is apparently fixed in Atmosphere 1.0.13 Also ensure the max size of a websocket fragment in bytes does not exceed the buffer size (#11842) Change-Id: I768524bb54a5b8b9479dc7bda821256bd843dc52 --- .../AtmospherePushConnection.java | 11 +++-- .../AtmospherePushConnection.java | 9 ++-- .../communication/PushRequestHandler.java | 17 ++++--- .../vaadin/shared/ApplicationConstants.java | 4 -- .../shared/communication/PushConstants.java | 46 +++++++++++++++++++ 5 files changed, 69 insertions(+), 18 deletions(-) create mode 100644 shared/src/com/vaadin/shared/communication/PushConstants.java diff --git a/client/src/com/vaadin/client/communication/AtmospherePushConnection.java b/client/src/com/vaadin/client/communication/AtmospherePushConnection.java index bc7e0b3fd2..ce3253ed50 100644 --- a/client/src/com/vaadin/client/communication/AtmospherePushConnection.java +++ b/client/src/com/vaadin/client/communication/AtmospherePushConnection.java @@ -27,6 +27,7 @@ import com.vaadin.client.ResourceLoader.ResourceLoadEvent; import com.vaadin.client.ResourceLoader.ResourceLoadListener; import com.vaadin.client.VConsole; import com.vaadin.shared.ApplicationConstants; +import com.vaadin.shared.communication.PushConstants; import com.vaadin.shared.ui.ui.UIConstants; /** @@ -67,8 +68,7 @@ public class AtmospherePushConnection implements PushConnection { */ protected static class FragmentedMessage { - // Jetty requires length less than buffer size - private int FRAGMENT_LENGTH = ApplicationConstants.WEBSOCKET_BUFFER_SIZE - 1; + private static final int FRAGMENT_LENGTH = PushConstants.WEBSOCKET_FRAGMENT_SIZE; private String message; private int index = 0; @@ -82,10 +82,12 @@ public class AtmospherePushConnection implements PushConnection { } public String getNextFragment() { + assert hasNextFragment(); + String result; if (index == 0) { String header = "" + message.length() - + ApplicationConstants.WEBSOCKET_MESSAGE_DELIMITER; + + PushConstants.MESSAGE_DELIMITER; int fragmentLen = FRAGMENT_LENGTH - header.length(); result = header + getFragment(0, fragmentLen); index += fragmentLen; @@ -384,7 +386,8 @@ public class AtmospherePushConnection implements PushConnection { contentType: 'application/json; charset=UTF-8', reconnectInterval: '5000', maxReconnectOnClose: 10000000, - trackMessageLength: true + trackMessageLength: true, + messageDelimiter: String.fromCharCode(@com.vaadin.shared.communication.PushConstants::MESSAGE_DELIMITER) }; }-*/; diff --git a/server/src/com/vaadin/server/communication/AtmospherePushConnection.java b/server/src/com/vaadin/server/communication/AtmospherePushConnection.java index 0bba65ff1d..96507b55f1 100644 --- a/server/src/com/vaadin/server/communication/AtmospherePushConnection.java +++ b/server/src/com/vaadin/server/communication/AtmospherePushConnection.java @@ -32,7 +32,7 @@ import org.atmosphere.cpr.AtmosphereResource; import org.atmosphere.cpr.AtmosphereResource.TRANSPORT; import org.json.JSONException; -import com.vaadin.shared.ApplicationConstants; +import com.vaadin.shared.communication.PushConstants; import com.vaadin.ui.UI; /** @@ -52,11 +52,12 @@ public class AtmospherePushConnection implements Serializable, PushConnection { private final int messageLength; public FragmentedMessage(Reader reader) throws IOException { - // Messages are prefixed by the total message length plus '|' + // Messages are prefixed by the total message length plus a + // delimiter String length = ""; int c; while ((c = reader.read()) != -1 - && c != ApplicationConstants.WEBSOCKET_MESSAGE_DELIMITER) { + && c != PushConstants.MESSAGE_DELIMITER) { length += (char) c; } try { @@ -76,7 +77,7 @@ public class AtmospherePushConnection implements Serializable, PushConnection { * @throws IOException */ public boolean append(Reader reader) throws IOException { - char[] buffer = new char[ApplicationConstants.WEBSOCKET_BUFFER_SIZE]; + char[] buffer = new char[PushConstants.WEBSOCKET_BUFFER_SIZE]; int read; while ((read = reader.read(buffer)) != -1) { message.append(buffer, 0, read); diff --git a/server/src/com/vaadin/server/communication/PushRequestHandler.java b/server/src/com/vaadin/server/communication/PushRequestHandler.java index 8360e08af9..e75798a980 100644 --- a/server/src/com/vaadin/server/communication/PushRequestHandler.java +++ b/server/src/com/vaadin/server/communication/PushRequestHandler.java @@ -23,6 +23,7 @@ import javax.servlet.ServletException; import org.atmosphere.client.TrackMessageSizeInterceptor; import org.atmosphere.cpr.ApplicationConfig; import org.atmosphere.cpr.AtmosphereFramework; +import org.atmosphere.cpr.AtmosphereInterceptor; import org.atmosphere.cpr.AtmosphereRequest; import org.atmosphere.cpr.AtmosphereResponse; @@ -36,7 +37,7 @@ import com.vaadin.server.VaadinServletRequest; import com.vaadin.server.VaadinServletResponse; import com.vaadin.server.VaadinServletService; import com.vaadin.server.VaadinSession; -import com.vaadin.shared.ApplicationConstants; +import com.vaadin.shared.communication.PushConstants; /** * Handles requests to open a push (bidirectional) communication channel between @@ -61,9 +62,11 @@ public class PushRequestHandler implements RequestHandler, atmosphere.addAtmosphereHandler("/*", pushHandler); atmosphere.addInitParameter(ApplicationConfig.PROPERTY_SESSION_SUPPORT, "true"); + atmosphere.addInitParameter(ApplicationConfig.MESSAGE_DELIMITER, + String.valueOf(PushConstants.MESSAGE_DELIMITER)); final String bufferSize = String - .valueOf(ApplicationConstants.WEBSOCKET_BUFFER_SIZE); + .valueOf(PushConstants.WEBSOCKET_BUFFER_SIZE); atmosphere.addInitParameter(ApplicationConfig.WEBSOCKET_BUFFER_SIZE, bufferSize); atmosphere.addInitParameter(ApplicationConfig.WEBSOCKET_MAXTEXTSIZE, @@ -75,12 +78,14 @@ public class PushRequestHandler implements RequestHandler, atmosphere.addInitParameter("org.atmosphere.cpr.showSupportMessage", "false"); - // Required to ensure the client-side knows at which points to split the - // message stream into individual messages when using certain transports - atmosphere.interceptor(new TrackMessageSizeInterceptor()); - try { atmosphere.init(service.getServlet().getServletConfig()); + + // Ensure the client-side knows how to split the message stream + // into individual messages when using certain transports + AtmosphereInterceptor trackMessageSize = new TrackMessageSizeInterceptor(); + trackMessageSize.configure(atmosphere.getAtmosphereConfig()); + atmosphere.interceptor(trackMessageSize); } catch (ServletException e) { throw new ServiceException("Could not read atmosphere settings", e); } diff --git a/shared/src/com/vaadin/shared/ApplicationConstants.java b/shared/src/com/vaadin/shared/ApplicationConstants.java index fc4abd1988..04cba79c0c 100644 --- a/shared/src/com/vaadin/shared/ApplicationConstants.java +++ b/shared/src/com/vaadin/shared/ApplicationConstants.java @@ -83,8 +83,4 @@ public class ApplicationConstants implements Serializable { * Name of the parameter used to transmit the CSRF token. */ public static final String CSRF_TOKEN_PARAMETER = "v-csrfToken"; - - public static final int WEBSOCKET_BUFFER_SIZE = 65536; - - public static final char WEBSOCKET_MESSAGE_DELIMITER = '|'; } diff --git a/shared/src/com/vaadin/shared/communication/PushConstants.java b/shared/src/com/vaadin/shared/communication/PushConstants.java new file mode 100644 index 0000000000..2d63621c12 --- /dev/null +++ b/shared/src/com/vaadin/shared/communication/PushConstants.java @@ -0,0 +1,46 @@ +/* + * Copyright 2000-2013 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.shared.communication; + +import java.io.Serializable; + +/** + * Shared constants used by push. + * + * @since 7.1 + * @author Vaadin Ltd + */ +public class PushConstants implements Serializable { + + /** + * The size, in bytes, of the receiving buffer used by some servers. + */ + public static final int WEBSOCKET_BUFFER_SIZE = 65536; + + /** + * The maximum size, in characters, of a websocket message fragment. + * This is a conservative maximum chosen so that the size in bytes will not + * exceed {@link PushConstants#WEBSOCKET_BUFFER_SIZE} given a UTF-8 encoded + * message. + */ + public static final int WEBSOCKET_FRAGMENT_SIZE = WEBSOCKET_BUFFER_SIZE / 4 - 1; + + /** + * The character used to mark message boundaries when messages may be split + * into multiple fragments. + */ + public static final char MESSAGE_DELIMITER = '\0'; +} -- 2.39.5