Handle JSONException and InvalidUIDLSecurityKeyException the same way in PushHandler and UIDLRequestHandler (#11556)

Change-Id: Ide3a162bd77fb9b2ec3d44ea4422b5841d19eec8

9 files changed, 176 insertions, 188 deletions

diff --git a/server/src/com/vaadin/server/LegacyCommunicationManager.java b/server/src/com/vaadin/server/LegacyCommunicationManager.java
index 2f559e4b26..7dea5890e9 100644
--- a/server/src/com/vaadin/server/LegacyCommunicationManager.java
+++ b/server/src/com/vaadin/server/LegacyCommunicationManager.java
@@ -65,22 +65,6 @@ public class LegacyCommunicationManager implements Serializable {
     // TODO PUSH move
     public static final String WRITE_SECURITY_TOKEN_FLAG = "writeSecurityToken";
 
-    /**
-     * TODO Document me!
-     * 
-     * @author peholmst
-     * 
-     * @deprecated As of 7.0. Will likely change or be removed in a future
-     *             version
-     */
-    @Deprecated
-    public interface Callback extends Serializable {
-
-        public void criticalNotification(VaadinRequest request,
-                VaadinResponse response, String cap, String msg,
-                String details, String outOfSyncURL) throws IOException;
-    }
-
     // TODO Refactor (#11410)
     private final HashMap<Integer, ClientCache> uiToClientCache = new HashMap<Integer, ClientCache>();
 
diff --git a/server/src/com/vaadin/server/VaadinPortlet.java b/server/src/com/vaadin/server/VaadinPortlet.java
index 6aa7546b16..12c68e8673 100644
--- a/server/src/com/vaadin/server/VaadinPortlet.java
+++ b/server/src/com/vaadin/server/VaadinPortlet.java
@@ -15,11 +15,7 @@
  */
 package com.vaadin.server;
 
-import java.io.BufferedWriter;
 import java.io.IOException;
-import java.io.OutputStream;
-import java.io.OutputStreamWriter;
-import java.io.PrintWriter;
 import java.io.Serializable;
 import java.lang.reflect.Method;
 import java.util.Enumeration;
@@ -423,70 +419,6 @@ public class VaadinPortlet extends GenericPortlet implements Constants,
         handleRequest(request, response);
     }
 
-    /**
-     * Send notification to client's application. Used to notify client of
-     * critical errors and session expiration due to long inactivity. Server has
-     * no knowledge of what application client refers to.
-     * 
-     * @param request
-     *            the Portlet request instance.
-     * @param response
-     *            the Portlet response to write to.
-     * @param caption
-     *            for the notification
-     * @param message
-     *            for the notification
-     * @param details
-     *            a detail message to show in addition to the passed message.
-     *            Currently shown directly but could be hidden behind a details
-     *            drop down.
-     * @param url
-     *            url to load after message, null for current page
-     * @throws IOException
-     *             if the writing failed due to input/output error.
-     * 
-     * @deprecated As of 7.0. Will likely change or be removed in a future
-     *             version
-     */
-    @Deprecated
-    void criticalNotification(VaadinPortletRequest request,
-            VaadinPortletResponse response, String caption, String message,
-            String details, String url) throws IOException {
-
-        // clients JS app is still running, but server application either
-        // no longer exists or it might fail to perform reasonably.
-        // send a notification to client's application and link how
-        // to "restart" application.
-
-        if (caption != null) {
-            caption = "\"" + caption + "\"";
-        }
-        if (details != null) {
-            if (message == null) {
-                message = details;
-            } else {
-                message += "<br/><br/>" + details;
-            }
-        }
-        if (message != null) {
-            message = "\"" + message + "\"";
-        }
-        if (url != null) {
-            url = "\"" + url + "\"";
-        }
-
-        // Set the response type
-        response.setContentType("application/json; charset=UTF-8");
-        final OutputStream out = response.getOutputStream();
-        final PrintWriter outWriter = new PrintWriter(new BufferedWriter(
-                new OutputStreamWriter(out, "UTF-8")));
-        outWriter.print("for(;;);[{\"changes\":[], \"meta\" : {"
-                + "\"appError\": {" + "\"caption\":" + caption + ","
-                + "\"message\" : " + message + "," + "\"url\" : " + url
-                + "}}, \"resources\": {}, \"locales\":[]}]");
-        outWriter.close();
-    }
-
     private static final Logger getLogger() {
         return Logger.getLogger(VaadinPortlet.class.getName());
     }
diff --git a/server/src/com/vaadin/server/VaadinPortletService.java b/server/src/com/vaadin/server/VaadinPortletService.java
index b9dcfa5768..05ce8626c5 100644
--- a/server/src/com/vaadin/server/VaadinPortletService.java
+++ b/server/src/com/vaadin/server/VaadinPortletService.java
@@ -17,7 +17,6 @@
 package com.vaadin.server;
 
 import java.io.File;
-import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
 import java.util.List;
@@ -55,23 +54,6 @@ public class VaadinPortletService extends VaadinService {
         }
     }
 
-    /*
-     * (non-Javadoc)
-     * 
-     * @see
-     * com.vaadin.server.LegacyCommunicationManager.Callback#criticalNotification
-     * (com.vaadin.server.VaadinRequest, com.vaadin.server.VaadinResponse,
-     * java.lang.String, java.lang.String, java.lang.String, java.lang.String)
-     */
-    @Deprecated
-    @Override
-    public void criticalNotification(VaadinRequest request,
-            VaadinResponse response, String cap, String msg, String details,
-            String url) throws IOException {
-        getPortlet().criticalNotification((VaadinPortletRequest) request,
-                (VaadinPortletResponse) response, cap, msg, details, url);
-    }
-
     @Override
     protected List<RequestHandler> createRequestHandlers() {
         List<RequestHandler> handlers = super.createRequestHandlers();
diff --git a/server/src/com/vaadin/server/VaadinService.java b/server/src/com/vaadin/server/VaadinService.java
index 86ccf00a78..7d93904248 100644
--- a/server/src/com/vaadin/server/VaadinService.java
+++ b/server/src/com/vaadin/server/VaadinService.java
@@ -16,9 +16,13 @@
 
 package com.vaadin.server;
 
+import java.io.BufferedWriter;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
 import java.io.Serializable;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
@@ -38,14 +42,17 @@ import javax.portlet.PortletContext;
 import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletResponse;
 
+import org.json.JSONException;
+import org.json.JSONObject;
+
 import com.vaadin.annotations.PreserveOnRefresh;
 import com.vaadin.event.EventRouter;
-import com.vaadin.server.LegacyCommunicationManager.Callback;
 import com.vaadin.server.communication.FileUploadHandler;
 import com.vaadin.server.communication.HeartbeatHandler;
 import com.vaadin.server.communication.PublishedFileHandler;
 import com.vaadin.server.communication.SessionRequestHandler;
 import com.vaadin.server.communication.UidlRequestHandler;
+import com.vaadin.shared.JsonConstants;
 import com.vaadin.shared.ui.ui.UIConstants;
 import com.vaadin.ui.UI;
 import com.vaadin.util.CurrentInstance;
@@ -59,7 +66,7 @@ import com.vaadin.util.ReflectTools;
  * 
  * @since 7.0
  */
-public abstract class VaadinService implements Serializable, Callback {
+public abstract class VaadinService implements Serializable {
     static final String REINITIALIZING_SESSION_MARKER = VaadinService.class
             .getName() + ".reinitializing";
 
@@ -142,7 +149,7 @@ public abstract class VaadinService implements Serializable, Callback {
         handlers.add(new PublishedFileHandler());
         handlers.add(new HeartbeatHandler());
         handlers.add(new FileUploadHandler());
-        handlers.add(new UidlRequestHandler(this));
+        handlers.add(new UidlRequestHandler());
         handlers.add(new UnsupportedBrowserHandler());
         handlers.add(new ConnectorResourceHandler());
 
@@ -1312,16 +1319,19 @@ public abstract class VaadinService implements Serializable, Callback {
             ErrorHandler errorHandler = ErrorEvent
                     .findErrorHandler(vaadinSession);
 
-            // if this was an UIDL request, response UIDL back to client
+            // if this was an UIDL request, send UIDL back to the client
             if (ServletPortletHelper.isUIDLRequest(request)) {
                 SystemMessages ci = getSystemMessages(
                         ServletPortletHelper.findLocale(null, vaadinSession,
                                 request), request);
                 try {
-                    criticalNotification(request, response,
-                            ci.getInternalErrorCaption(),
-                            ci.getInternalErrorMessage(), null,
-                            ci.getInternalErrorURL());
+                    writeStringResponse(
+                            response,
+                            JsonConstants.JSON_CONTENT_TYPE,
+                            createCriticalNotificationJSON(
+                                    ci.getInternalErrorCaption(),
+                                    ci.getInternalErrorMessage(), null,
+                                    ci.getInternalErrorURL()));
                 } catch (IOException e) {
                     // An exception occured while writing the response. Log
                     // it and continue handling only the original error.
@@ -1350,6 +1360,30 @@ public abstract class VaadinService implements Serializable, Callback {
     }
 
     /**
+     * Writes the given string as a response using the given content type.
+     * 
+     * @param response
+     *            The response reference
+     * @param contentType
+     *            The content type of the response
+     * @param reponseString
+     *            The actual response
+     * @throws IOException
+     *             If an error occured while writing the response
+     */
+    protected void writeStringResponse(VaadinResponse response,
+            String contentType, String reponseString) throws IOException {
+
+        response.setContentType(contentType);
+
+        final OutputStream out = response.getOutputStream();
+        final PrintWriter outWriter = new PrintWriter(new BufferedWriter(
+                new OutputStreamWriter(out, "UTF-8")));
+        outWriter.print(reponseString);
+        outWriter.close();
+    }
+
+    /**
      * Called when the session has expired and the request handling is therefore
      * aborted.
      * 
@@ -1364,4 +1398,66 @@ public abstract class VaadinService implements Serializable, Callback {
     protected abstract void handleSessionExpired(VaadinRequest request,
             VaadinResponse response) throws ServiceException;
 
+    /**
+     * Creates a JSON message which, when sent to client as-is, will cause a
+     * critical error to be shown with the given details.
+     * 
+     * @param caption
+     *            The caption of the error or null to omit
+     * @param message
+     *            The error message or null to omit
+     * @param details
+     *            Additional error details or null to omit
+     * @param url
+     *            A url to redirect to. If no other details are given then the
+     *            user will be immediately redirected to this URL. Otherwise the
+     *            message will be shown and the browser will redirect to the
+     *            given URL only after the user acknowledges the message. If
+     *            null then the browser will refresh the current page.
+     * @return A JSON string to be sent to the client
+     */
+    public static String createCriticalNotificationJSON(String caption,
+            String message, String details, String url) {
+        String returnString = "";
+        try {
+            if (message == null) {
+                message = details;
+            } else if (details != null) {
+                message += "<br/><br/>" + details;
+            }
+
+            JSONObject appError = new JSONObject();
+            appError.put("caption", caption);
+            appError.put("message", message);
+            appError.put("url", url);
+
+            JSONObject meta = new JSONObject();
+            meta.put("appError", appError);
+
+            JSONObject json = new JSONObject();
+            json.put("changes", Collections.EMPTY_LIST);
+            json.put("resources", Collections.EMPTY_MAP);
+            json.put("locales", Collections.EMPTY_LIST);
+            json.put("meta", meta);
+            returnString = json.toString();
+        } catch (JSONException e) {
+            getLogger().log(Level.WARNING,
+                    "Error creating critical notification JSON message", e);
+        }
+
+        return "for(;;);[" + returnString + "]";
+    }
+
+    /**
+     * @deprecated As of 7.0. Will likely change or be removed in a future
+     *             version
+     */
+    @Deprecated
+    public void criticalNotification(VaadinRequest request,
+            VaadinResponse response, String caption, String message,
+            String details, String url) throws IOException {
+        writeStringResponse(response, JsonConstants.JSON_CONTENT_TYPE,
+                createCriticalNotificationJSON(caption, message, details, url));
+    }
+
 }
diff --git a/server/src/com/vaadin/server/VaadinServlet.java b/server/src/com/vaadin/server/VaadinServlet.java
index b9141039dc..70e77b0b8a 100644
--- a/server/src/com/vaadin/server/VaadinServlet.java
+++ b/server/src/com/vaadin/server/VaadinServlet.java
@@ -40,6 +40,7 @@ import javax.servlet.http.HttpServletResponse;
 
 import com.vaadin.sass.internal.ScssStylesheet;
 import com.vaadin.server.communication.ServletUIInitHandler;
+import com.vaadin.shared.JsonConstants;
 import com.vaadin.util.CurrentInstance;
 
 @SuppressWarnings("serial")
@@ -278,10 +279,13 @@ public class VaadinServlet extends HttpServlet implements Constants {
                 SystemMessages systemMessages = getService().getSystemMessages(
                         ServletPortletHelper.findLocale(null, null, request),
                         request);
-                criticalNotification(request, response,
-                        systemMessages.getCookiesDisabledCaption(),
-                        systemMessages.getCookiesDisabledMessage(), null,
-                        systemMessages.getCookiesDisabledURL());
+                getService().writeStringResponse(
+                        response,
+                        JsonConstants.JSON_CONTENT_TYPE,
+                        VaadinService.createCriticalNotificationJSON(
+                                systemMessages.getCookiesDisabledCaption(),
+                                systemMessages.getCookiesDisabledMessage(),
+                                null, systemMessages.getCookiesDisabledURL()));
                 return false;
             }
         }
@@ -312,8 +316,8 @@ public class VaadinServlet extends HttpServlet implements Constants {
      * @throws IOException
      *             if the writing failed due to input/output error.
      * 
-     * @deprecated As of 7.0. Will likely change or be removed in a future
-     *             version
+     * @deprecated As of 7.0. This method is retained only for backwards
+     *             compatibility and for {@link GAEVaadinServlet}.
      */
     @Deprecated
     protected void criticalNotification(VaadinServletRequest request,
@@ -321,30 +325,10 @@ public class VaadinServlet extends HttpServlet implements Constants {
             String details, String url) throws IOException {
 
         if (ServletPortletHelper.isUIDLRequest(request)) {
-
-            if (caption != null) {
-                caption = "\"" + JsonPaintTarget.escapeJSON(caption) + "\"";
-            }
-            if (details != null) {
-                if (message == null) {
-                    message = details;
-                } else {
-                    message += "<br/><br/>" + details;
-                }
-            }
-
-            if (message != null) {
-                message = "\"" + JsonPaintTarget.escapeJSON(message) + "\"";
-            }
-            if (url != null) {
-                url = "\"" + JsonPaintTarget.escapeJSON(url) + "\"";
-            }
-
-            String output = "for(;;);[{\"changes\":[], \"meta\" : {"
-                    + "\"appError\": {" + "\"caption\":" + caption + ","
-                    + "\"message\" : " + message + "," + "\"url\" : " + url
-                    + "}}, \"resources\": {}, \"locales\":[]}]";
-            writeResponse(response, "application/json; charset=UTF-8", output);
+            String output = VaadinService.createCriticalNotificationJSON(
+                    caption, message, details, url);
+            getService().writeStringResponse(response,
+                    JsonConstants.JSON_CONTENT_TYPE, output);
         } else {
             // Create an HTML reponse with the error
             String output = "";
@@ -367,7 +351,8 @@ public class VaadinServlet extends HttpServlet implements Constants {
             if (url != null) {
                 output += "</a>";
             }
-            writeResponse(response, "text/html; charset=UTF-8", output);
+            getService().writeStringResponse(response,
+                    "text/html; charset=UTF-8", output);
         }
     }
 
@@ -485,10 +470,14 @@ public class VaadinServlet extends HttpServlet implements Constants {
                     request.getLocale(), request);
             if (ServletPortletHelper.isUIDLRequest(request)) {
                 // send uidl redirect
-                criticalNotification(request, response,
-                        ci.getCommunicationErrorCaption(),
-                        ci.getCommunicationErrorMessage(),
-                        INVALID_SECURITY_KEY_MSG, ci.getCommunicationErrorURL());
+                getService().writeStringResponse(
+                        response,
+                        JsonConstants.JSON_CONTENT_TYPE,
+                        VaadinService.createCriticalNotificationJSON(
+                                ci.getCommunicationErrorCaption(),
+                                ci.getCommunicationErrorMessage(),
+                                INVALID_SECURITY_KEY_MSG,
+                                ci.getCommunicationErrorURL()));
             } else if (ServletPortletHelper.isHeartbeatRequest(request)) {
                 response.sendError(HttpServletResponse.SC_FORBIDDEN,
                         "Forbidden");
diff --git a/server/src/com/vaadin/server/VaadinServletService.java b/server/src/com/vaadin/server/VaadinServletService.java
index 4ae70c1c3b..8b445b6061 100644
--- a/server/src/com/vaadin/server/VaadinServletService.java
+++ b/server/src/com/vaadin/server/VaadinServletService.java
@@ -31,6 +31,7 @@ import javax.servlet.http.HttpServletResponse;
 import com.vaadin.server.communication.PushRequestHandler;
 import com.vaadin.server.communication.ServletBootstrapHandler;
 import com.vaadin.server.communication.ServletUIInitHandler;
+import com.vaadin.shared.JsonConstants;
 import com.vaadin.shared.communication.PushMode;
 import com.vaadin.ui.UI;
 
@@ -53,23 +54,6 @@ public class VaadinServletService extends VaadinService {
         }
     }
 
-    /*
-     * (non-Javadoc)
-     * 
-     * @see
-     * com.vaadin.server.LegacyCommunicationManager.Callback#criticalNotification
-     * (com.vaadin.server.VaadinRequest, com.vaadin.server.VaadinResponse,
-     * java.lang.String, java.lang.String, java.lang.String, java.lang.String)
-     */
-    @Deprecated
-    @Override
-    public void criticalNotification(VaadinRequest request,
-            VaadinResponse response, String cap, String msg, String details,
-            String url) throws IOException {
-        getServlet().criticalNotification((VaadinServletRequest) request,
-                (VaadinServletResponse) response, cap, msg, details, url);
-    }
-
     @Override
     protected List<RequestHandler> createRequestHandlers() {
         List<RequestHandler> handlers = super.createRequestHandlers();
@@ -292,12 +276,13 @@ public class VaadinServletService extends VaadinService {
                  */
                 servletRequest.getSession().invalidate();
 
-                // send uidl redirect
-                criticalNotification(request, response,
-                        ci.getSessionExpiredCaption(),
-                        ci.getSessionExpiredMessage(), null,
-                        ci.getSessionExpiredURL());
-
+                writeStringResponse(
+                        response,
+                        JsonConstants.JSON_CONTENT_TYPE,
+                        createCriticalNotificationJSON(
+                                ci.getSessionExpiredCaption(),
+                                ci.getSessionExpiredMessage(), null,
+                                ci.getSessionExpiredURL()));
             } else if (ServletPortletHelper.isHeartbeatRequest(request)) {
                 response.sendError(HttpServletResponse.SC_GONE,
                         "Session expired");
diff --git a/server/src/com/vaadin/server/communication/AtmospherePushConnection.java b/server/src/com/vaadin/server/communication/AtmospherePushConnection.java
index 0bd3dc5a14..5ff1d14232 100644
--- a/server/src/com/vaadin/server/communication/AtmospherePushConnection.java
+++ b/server/src/com/vaadin/server/communication/AtmospherePushConnection.java
@@ -74,8 +74,17 @@ public class AtmospherePushConnection implements Serializable, PushConnection {
             throw new IOException("Error writing UIDL", e);
         }
         // "Broadcast" the changes to the single client only
-        getResource().getBroadcaster().broadcast(writer.toString(),
-                getResource());
+        sendMessage(writer.toString());
+    }
+
+    /**
+     * Sends the given message to the current client
+     * 
+     * @param message
+     *            The message to send
+     */
+    void sendMessage(String message) {
+        getResource().getBroadcaster().broadcast(message, getResource());
     }
 
     /**
diff --git a/server/src/com/vaadin/server/communication/PushHandler.java b/server/src/com/vaadin/server/communication/PushHandler.java
index fd061dc99d..a92d56235d 100644
--- a/server/src/com/vaadin/server/communication/PushHandler.java
+++ b/server/src/com/vaadin/server/communication/PushHandler.java
@@ -32,6 +32,7 @@ import org.json.JSONException;
 import com.vaadin.server.LegacyCommunicationManager.InvalidUIDLSecurityKeyException;
 import com.vaadin.server.ServiceException;
 import com.vaadin.server.SessionExpiredException;
+import com.vaadin.server.VaadinService;
 import com.vaadin.server.VaadinServletRequest;
 import com.vaadin.server.VaadinServletService;
 import com.vaadin.server.VaadinSession;
@@ -126,16 +127,27 @@ public class PushHandler implements AtmosphereHandler {
                  * via the push channel (we do not respond to the request
                  * directly.)
                  */
-                new ServerRpcHandler().handleRpc(ui, req.getReader(),
-                        vaadinRequest);
-                connection.push(false);
+                try {
+                    new ServerRpcHandler().handleRpc(ui, req.getReader(),
+                            vaadinRequest);
+                    connection.push(false);
+                } catch (JSONException e) {
+                    getLogger().log(Level.SEVERE,
+                            "Error writing JSON to response", e);
+                    // Refresh on client side
+                    connection.sendMessage(VaadinService
+                            .createCriticalNotificationJSON(null, null, null,
+                                    null));
+                } catch (InvalidUIDLSecurityKeyException e) {
+                    getLogger().log(Level.WARNING,
+                            "Invalid security key received from {}",
+                            resource.getRequest().getRemoteHost());
+                    // Refresh on client side
+                    connection.sendMessage(VaadinService
+                            .createCriticalNotificationJSON(null, null, null,
+                                    null));
+                }
             }
-        } catch (InvalidUIDLSecurityKeyException e) {
-            // TODO Error handling
-            e.printStackTrace();
-        } catch (JSONException e) {
-            // TODO Error handling
-            e.printStackTrace();
         } catch (IOException e) {
             // TODO Error handling
             e.printStackTrace();
diff --git a/server/src/com/vaadin/server/communication/UidlRequestHandler.java b/server/src/com/vaadin/server/communication/UidlRequestHandler.java
index ef627a818b..169b2cf545 100644
--- a/server/src/com/vaadin/server/communication/UidlRequestHandler.java
+++ b/server/src/com/vaadin/server/communication/UidlRequestHandler.java
@@ -28,12 +28,12 @@ import org.json.JSONException;
 import com.vaadin.server.ClientConnector;
 import com.vaadin.server.Constants;
 import com.vaadin.server.LegacyCommunicationManager;
-import com.vaadin.server.LegacyCommunicationManager.Callback;
 import com.vaadin.server.LegacyCommunicationManager.InvalidUIDLSecurityKeyException;
 import com.vaadin.server.ServletPortletHelper;
 import com.vaadin.server.SynchronizedRequestHandler;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.server.VaadinResponse;
+import com.vaadin.server.VaadinService;
 import com.vaadin.server.VaadinSession;
 import com.vaadin.shared.ApplicationConstants;
 import com.vaadin.shared.Version;
@@ -54,12 +54,9 @@ public class UidlRequestHandler extends SynchronizedRequestHandler {
 
     public static final String UIDL_PATH = "UIDL/";
 
-    private Callback criticalNotifier;
-
     private ServerRpcHandler rpcHandler = new ServerRpcHandler();
 
-    public UidlRequestHandler(Callback criticalNotifier) {
-        this.criticalNotifier = criticalNotifier;
+    public UidlRequestHandler() {
     }
 
     @Override
@@ -118,16 +115,18 @@ public class UidlRequestHandler extends SynchronizedRequestHandler {
         } catch (JSONException e) {
             getLogger().log(Level.SEVERE, "Error writing JSON to response", e);
             // Refresh on client side
-            criticalNotifier.criticalNotification(request, response, null,
-                    null, null, null);
+            response.getWriter().write(
+                    VaadinService.createCriticalNotificationJSON(null, null,
+                            null, null));
             return true;
         } catch (InvalidUIDLSecurityKeyException e) {
             getLogger().log(Level.WARNING,
                     "Invalid security key received from {}",
                     request.getRemoteHost());
             // Refresh on client side
-            criticalNotifier.criticalNotification(request, response, null,
-                    null, null, null);
+            response.getWriter().write(
+                    VaadinService.createCriticalNotificationJSON(null, null,
+                            null, null));
             return true;
         } finally {
             stringWriter.close();