fix: Not write UIDL sync message when RPC handling throws (#215)

Liferay container is so strict regards ServletResponse::setContentLength comparing to normal servlet containers and Apache Pluto portal, thus the PortalUidlRequestHandler works incorrectly in case of exception in the server-side listener: it writes the error meta info JSON into response, then appends sync UIDL JSON message, then calls setContentLength with the length of the sync UIDL message. Which in turn leads to an unpredictable JSON sent to client.

When this change is applied, UidlRequestHandler doesn't write sync UIDL message to the response once the RPC handler execution throws, but instead sends only an error message to the client.

No IT tests added, because this is covered by LiferayErrorHandlingIT being added afterwards within Liferay tests PR.

Fixes: #213

vaadin-portlet/src/main/java/com/vaadin/flow/portal/DefaultPortletErrorHandler.java

@@ -13,6 +13,8 @@
  * client-side, displaying the errors on the portlet which caused the exception.
  */
 public class DefaultPortletErrorHandler implements ErrorHandler {
+    static final String ERROR_ATTRIBUTE_NAME =
+            DefaultPortletErrorHandler.class.getName() + ".error.thrown";
     private static final Logger logger = LoggerFactory
             .getLogger(DefaultPortletErrorHandler.class);
 
@@ -36,6 +38,11 @@ public void error(ErrorEvent event) {
                                 event.getThrowable().getMessage(),
                                 getCauseString(event.getThrowable()), null,
                                 getQuerySelector(response)));
+                // Liferay related: tells UIDL handler not to write the sync
+                // UIDL, because it corrupts RPC response in case of exception
+                // see https://github.com/vaadin/portlet/issues/213
+                VaadinPortletRequest.getCurrentPortletRequest().setAttribute(
+                        ERROR_ATTRIBUTE_NAME, Boolean.TRUE);
             } catch (Exception e) {
                 logger.error("Failed to send critical notification!", e);
             }
@@ -47,7 +54,7 @@ public void error(ErrorEvent event) {
      * error box should be added. If the element found by the
      * {@code querySelector} has a shadow root, the error will be added into the
      * shadow instead.
-     * 
+     *
      * @param response
      *            the portlet response used to write the error to the
      *            client-side

vaadin-portlet/src/main/java/com/vaadin/flow/portal/PortletUidlRequestHandler.java

@@ -15,7 +15,16 @@
  */
 package com.vaadin.flow.portal;
 
+import javax.servlet.http.Cookie;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.PrintWriter;
+import java.io.Serializable;
+
 import com.vaadin.flow.server.VaadinRequest;
+import com.vaadin.flow.server.VaadinResponse;
+import com.vaadin.flow.server.VaadinService;
+import com.vaadin.flow.server.VaadinSession;
 import com.vaadin.flow.server.communication.UidlRequestHandler;
 
 /**
@@ -30,4 +39,132 @@ class PortletUidlRequestHandler extends UidlRequestHandler {
     protected boolean canHandleRequest(VaadinRequest request) {
         return "/uidl".equals(request.getPathInfo());
     }
+
+    @Override
+    public boolean synchronizedHandleRequest(VaadinSession session, VaadinRequest request,
+                                             VaadinResponse response) throws IOException {
+        VaadinResponseWrapper vaadinResponseWrapper =
+                new VaadinResponseWrapper(request, response);
+        return super.synchronizedHandleRequest(session, request, vaadinResponseWrapper);
+    }
+
+    /**
+     * Wraps the portlet response to stub the writing actions so as to not
+     * write the UIDL sync message, when the error occurs during RPC handling.
+     * Specific to Liferay.
+     * See https://github.com/vaadin/portlet/issues/213
+     */
+    private static class VaadinResponseWrapper implements VaadinResponse {
+
+        private final VaadinPortletRequest request;
+        private final VaadinPortletResponse delegate;
+
+        public VaadinResponseWrapper(VaadinRequest vaadinRequest,
+                                     VaadinResponse vaadinResponse) {
+            if (!(vaadinResponse instanceof VaadinPortletResponse &&
+                  vaadinRequest instanceof VaadinPortletRequest)) {
+                throw new IllegalArgumentException(
+                        "Portlet request/response expected, make sure you run the application in the portal container");
+            }
+            request = (VaadinPortletRequest) vaadinRequest;
+            delegate = (VaadinPortletResponse) vaadinResponse;
+        }
+
+        @Override
+        public void setStatus(int statusCode) {
+            delegate.setStatus(statusCode);
+        }
+
+        @Override
+        public void setContentType(String contentType) {
+            delegate.setContentType(contentType);
+        }
+
+        @Override
+        public void setHeader(String name, String value) {
+            delegate.setHeader(name, value);
+        }
+
+        @Override
+        public void setDateHeader(String name, long timestamp) {
+            delegate.setDateHeader(name, timestamp);
+        }
+
+        @Override
+        public OutputStream getOutputStream() throws IOException {
+            if (noError()) {
+                return delegate.getOutputStream();
+            } else {
+                return new OutputStreamWrapper();
+            }
+        }
+
+        @Override
+        public PrintWriter getWriter() throws IOException {
+            return null;
+        }
+
+        @Override
+        public void setCacheTime(long milliseconds) {
+            delegate.setCacheTime(milliseconds);
+        }
+
+        @Override
+        public void sendError(int errorCode, String message) throws IOException {
+            delegate.sendError(errorCode, message);
+        }
+
+        @Override
+        public VaadinService getService() {
+            return delegate.getService();
+        }
+
+        @Override
+        public void addCookie(Cookie cookie) {
+            delegate.addCookie(cookie);
+        }
+
+        @Override
+        public void setContentLength(int len) {
+            if (noError()) {
+                delegate.setContentLength(len);
+            }
+        }
+
+        @Override
+        public void setNoCacheHeaders() {
+            delegate.setNoCacheHeaders();
+        }
+
+        private boolean noError() {
+            return request.getPortletRequest().getAttribute(
+                    DefaultPortletErrorHandler.ERROR_ATTRIBUTE_NAME) == null;
+        }
+    }
+
+    /**
+     * Null output stream implementation.
+     */
+    private static class OutputStreamWrapper extends OutputStream
+            implements Serializable {
+        private volatile boolean closed;
+
+        private void ensureOpen() throws IOException {
+            if (this.closed) {
+                throw new IOException("Stream closed");
+            }
+        }
+
+        public void write(int b) throws IOException {
+            this.ensureOpen();
+        }
+
+        public void write(byte[] b, int off, int len) throws IOException {
+            this.ensureOpen();
+        }
+
+        public void close() {
+            this.closed = true;
+        }
+    }
 }

vaadin-portlet/src/main/java/com/vaadin/flow/portal/VaadinPortletService.java

@@ -50,6 +50,7 @@
 import com.vaadin.flow.server.WrappedSession;
 import com.vaadin.flow.server.communication.HeartbeatHandler;
 import com.vaadin.flow.server.communication.StreamRequestHandler;
+import com.vaadin.flow.server.communication.UidlRequestHandler;
 import com.vaadin.flow.server.startup.PortletApplicationRouteRegistryUtil;
 import com.vaadin.flow.shared.Registration;
 import com.vaadin.flow.theme.AbstractTheme;
@@ -117,6 +118,9 @@ protected List<RequestHandler> createRequestHandlers()
         handlers.add(new PortletBootstrapHandler());
         handlers.add(new PortletWebComponentProvider());
         handlers.add(new PortletWebComponentBootstrapHandler());
+
+        handlers.removeIf(
+                requestHandler -> requestHandler instanceof UidlRequestHandler);
         handlers.add(new PortletUidlRequestHandler());
 
         handlers.removeIf(

vaadin-portlet/src/test/java/com/vaadin/flow/portal/PortletClassesSerializableTest.java

@@ -33,7 +33,9 @@ protected Stream<String> getExcludedPatterns() {
                         "PortletStreamReceiverHandler\\$StreamRequestContext",
                 "com\\.vaadin\\.flow\\.portal\\.VaadinHttpAndPortletRequest",
                 "com\\.vaadin\\.flow\\.portal\\.VaadinHttpPortletRequest",
-                "com\\.vaadin\\.flow\\.portal\\.VaadinLiferayRequest"
+                "com\\.vaadin\\.flow\\.portal\\.VaadinLiferayRequest",
+                "com\\.vaadin\\.flow\\.portal\\." +
+                        "PortletUidlRequestHandler\\$VaadinResponseWrapper"
         );
     }