diff --git a/flow-server/src/main/java/com/vaadin/flow/server/SynchronizedRequestHandler.java b/flow-server/src/main/java/com/vaadin/flow/server/SynchronizedRequestHandler.java index d271b8a189f..a5432b00587 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/SynchronizedRequestHandler.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/SynchronizedRequestHandler.java @@ -15,7 +15,11 @@ */ package com.vaadin.flow.server; +import java.io.BufferedReader; import java.io.IOException; +import java.io.Reader; +import java.io.Serializable; +import java.util.Optional; /** * RequestHandler which takes care of locking and unlocking of the VaadinSession @@ -28,6 +32,21 @@ */ public abstract class SynchronizedRequestHandler implements RequestHandler { + public static final int MAX_BUFFER_SIZE = 64 * 1024; + + /** + * ResponseWriter is optionally returned by request handlers which implement + * {@link SynchronizedRequestHandler#synchronizedHandleRequest(VaadinSession, VaadinRequest, VaadinResponse, String)} + * + * The ResponseWriter will be executed by + * {@link #handleRequest(VaadinSession, VaadinRequest, VaadinResponse)} + * without holding Vaadin session lock. + */ + @FunctionalInterface + public interface ResponseWriter extends Serializable { + void writeResponse() throws IOException; + } + @Override public boolean handleRequest(VaadinSession session, VaadinRequest request, VaadinResponse response) throws IOException { @@ -35,11 +54,27 @@ public boolean handleRequest(VaadinSession session, VaadinRequest request, return false; } - session.lock(); try { - return synchronizedHandleRequest(session, request, response); + if (isReadAndWriteOutsideSessionLock()) { + BufferedReader reader = request.getReader(); + String requestBody = reader == null ? null + : getRequestBody(reader); + session.lock(); + Optional responseWriter = synchronizedHandleRequest( + session, request, response, requestBody); + session.unlock(); + if (responseWriter.isPresent()) { + responseWriter.get().writeResponse(); + } + return responseWriter.isPresent(); + } else { + session.lock(); + return synchronizedHandleRequest(session, request, response); + } } finally { - session.unlock(); + if (session.hasLock()) { + session.unlock(); + } } } @@ -65,6 +100,51 @@ public boolean handleRequest(VaadinSession session, VaadinRequest request, public abstract boolean synchronizedHandleRequest(VaadinSession session, VaadinRequest request, VaadinResponse response) throws IOException; + /** + * Gets if request body should be read and the response written without + * holding {@link VaadinSession} lock + * + * @return {@literal true} if + * {@link #synchronizedHandleRequest(VaadinSession, VaadinRequest, VaadinResponse, String)} + * should be called. Returns {@literal false} if + * {@link #synchronizedHandleRequest(VaadinSession, VaadinRequest, VaadinResponse)} + * should be called. + */ + public boolean isReadAndWriteOutsideSessionLock() { + return false; + } + + /** + * Identical to + * {@link #synchronizedHandleRequest(VaadinSession, VaadinRequest, VaadinResponse)} + * except the {@link VaadinSession} is locked before this is called and the + * response requestBody has been read before locking the session and is + * provided as a separate parameter. + * + * @param session + * The session for the request + * @param request + * The request to handle + * @param response + * The response object to which a response can be written. + * @param requestBody + * Request body pre-read from the request object + * @return a ResponseWriter wrapped into an Optional, if this handler will + * write the response and no further request handlers should be + * called, otherwise an empty Optional. The ResponseWriter will be + * executed after the VaadinSession is unlocked. + * + * @throws IOException + * If an IO error occurred + * @see #handleRequest(VaadinSession, VaadinRequest, VaadinResponse) + */ + public Optional synchronizedHandleRequest( + VaadinSession session, VaadinRequest request, + VaadinResponse response, String requestBody) + throws IOException, UnsupportedOperationException { + throw new UnsupportedOperationException(); + } + /** * Check whether a request may be handled by this handler. This can be used * as an optimization to avoid locking the session just to investigate some @@ -85,4 +165,18 @@ protected boolean canHandleRequest(VaadinRequest request) { return true; } + public static String getRequestBody(Reader reader) throws IOException { + StringBuilder sb = new StringBuilder(MAX_BUFFER_SIZE); + char[] buffer = new char[MAX_BUFFER_SIZE]; + + while (true) { + int read = reader.read(buffer); + if (read == -1) { + break; + } + sb.append(buffer, 0, read); + } + + return sb.toString(); + } } diff --git a/flow-server/src/main/java/com/vaadin/flow/server/communication/PushHandler.java b/flow-server/src/main/java/com/vaadin/flow/server/communication/PushHandler.java index 883acfccc2c..e1c28104759 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/communication/PushHandler.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/communication/PushHandler.java @@ -42,6 +42,7 @@ import com.vaadin.flow.server.ErrorEvent; import com.vaadin.flow.server.HandlerHelper; import com.vaadin.flow.server.SessionExpiredException; +import com.vaadin.flow.server.SynchronizedRequestHandler; import com.vaadin.flow.server.SystemMessages; import com.vaadin.flow.server.VaadinContext; import com.vaadin.flow.server.VaadinRequest; @@ -54,7 +55,6 @@ import com.vaadin.flow.shared.ApplicationConstants; import com.vaadin.flow.shared.JsonConstants; import com.vaadin.flow.shared.communication.PushMode; - import elemental.json.JsonException; /** @@ -162,7 +162,9 @@ interface PushEventCallback { assert vaadinRequest != null; try { - new ServerRpcHandler().handleRpc(ui, reader, vaadinRequest); + new ServerRpcHandler().handleRpc(ui, + SynchronizedRequestHandler.getRequestBody(reader), + vaadinRequest); connection.push(false); } catch (JsonException e) { getLogger().error("Error writing JSON to response", e); diff --git a/flow-server/src/main/java/com/vaadin/flow/server/communication/ServerRpcHandler.java b/flow-server/src/main/java/com/vaadin/flow/server/communication/ServerRpcHandler.java index 9ebc866614a..c877837bf09 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/communication/ServerRpcHandler.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/communication/ServerRpcHandler.java @@ -37,6 +37,7 @@ import com.vaadin.flow.internal.StateNode; import com.vaadin.flow.router.PreserveOnRefresh; import com.vaadin.flow.server.ErrorEvent; +import com.vaadin.flow.server.SynchronizedRequestHandler; import com.vaadin.flow.server.VaadinRequest; import com.vaadin.flow.server.VaadinService; import com.vaadin.flow.server.communication.rpc.AttachExistingElementRpcHandler; @@ -90,6 +91,11 @@ public static class RpcRequest implements Serializable { * the request through which the JSON was received */ public RpcRequest(String jsonString, VaadinRequest request) { + this(jsonString, request.getService().getDeploymentConfiguration() + .isSyncIdCheckEnabled()); + } + + public RpcRequest(String jsonString, boolean isSyncIdCheckEnabled) { json = JsonUtil.parse(jsonString); JsonValue token = json.get(ApplicationConstants.CSRF_TOKEN); @@ -103,8 +109,7 @@ public RpcRequest(String jsonString, VaadinRequest request) { this.csrfToken = csrfToken; } - if (request.getService().getDeploymentConfiguration() - .isSyncIdCheckEnabled()) { + if (isSyncIdCheckEnabled) { syncId = (int) json .getNumber(ApplicationConstants.SERVER_SYNC_ID); } else { @@ -196,8 +201,6 @@ private boolean isUnloadBeaconRequest() { } - private static final int MAX_BUFFER_SIZE = 64 * 1024; - /** * Exception thrown then the security key sent by the client does not match * the expected one. @@ -248,16 +251,35 @@ public ResynchronizationRequiredException() { */ public void handleRpc(UI ui, Reader reader, VaadinRequest request) throws IOException, InvalidUIDLSecurityKeyException { - ui.getSession().setLastRequestTimestamp(System.currentTimeMillis()); + handleRpc(ui, SynchronizedRequestHandler.getRequestBody(reader), + request); + } - String changeMessage = getMessage(reader); + /** + * Reads JSON containing zero or more serialized RPC calls (including legacy + * variable changes) and executes the calls. + * + * @param ui + * The {@link UI} receiving the calls. Cannot be null. + * @param message + * The JSON message from the request. + * @param request + * The request through which the RPC was received + * @throws InvalidUIDLSecurityKeyException + * If the received security key does not match the one stored in + * the session. + */ + public void handleRpc(UI ui, String message, VaadinRequest request) + throws InvalidUIDLSecurityKeyException { + ui.getSession().setLastRequestTimestamp(System.currentTimeMillis()); - if (changeMessage == null || changeMessage.equals("")) { + if (message == null || message.isEmpty()) { // The client sometimes sends empty messages, this is probably a bug return; } - RpcRequest rpcRequest = new RpcRequest(changeMessage, request); + RpcRequest rpcRequest = new RpcRequest(message, request.getService() + .getDeploymentConfiguration().isSyncIdCheckEnabled()); // Security: double cookie submission pattern unless disabled by // property @@ -265,9 +287,9 @@ public void handleRpc(UI ui, Reader reader, VaadinRequest request) throw new InvalidUIDLSecurityKeyException(); } - String hashMessage = changeMessage; + String hashMessage = message; if (hashMessage.length() > 64 * 1024) { - hashMessage = changeMessage.substring(0, 64 * 1024); + hashMessage = message.substring(0, 64 * 1024); } byte[] messageHash = MessageDigestUtil.sha256(hashMessage); @@ -370,7 +392,6 @@ public void handleRpc(UI ui, Reader reader, VaadinRequest request) getLogger().debug("UI closed with a beacon request"); } } - } // Kind of same as in AbstractNavigationStateRenderer, but gets @@ -504,8 +525,9 @@ private static void callErrorHandler(UI ui, JsonObject invocationJson, protected String getMessage(Reader reader) throws IOException { - StringBuilder sb = new StringBuilder(MAX_BUFFER_SIZE); - char[] buffer = new char[MAX_BUFFER_SIZE]; + StringBuilder sb = new StringBuilder( + SynchronizedRequestHandler.MAX_BUFFER_SIZE); + char[] buffer = new char[SynchronizedRequestHandler.MAX_BUFFER_SIZE]; while (true) { int read = reader.read(buffer); diff --git a/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlRequestHandler.java b/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlRequestHandler.java index 38b17e42a01..fd60ee87326 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlRequestHandler.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/communication/UidlRequestHandler.java @@ -20,6 +20,7 @@ import java.io.OutputStream; import java.io.StringWriter; import java.io.Writer; +import java.util.Optional; import java.util.concurrent.atomic.AtomicReference; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -99,31 +100,48 @@ protected ServerRpcHandler createRpcHandler() { @Override public boolean synchronizedHandleRequest(VaadinSession session, VaadinRequest request, VaadinResponse response) throws IOException { + String requestBody = SynchronizedRequestHandler + .getRequestBody(request.getReader()); + Optional responseWriter = synchronizedHandleRequest( + session, request, response, requestBody); + if (responseWriter.isPresent()) { + responseWriter.get().writeResponse(); + } + return responseWriter.isPresent(); + } + + @Override + public boolean isReadAndWriteOutsideSessionLock() { + return true; + } + + @Override + public Optional synchronizedHandleRequest( + VaadinSession session, VaadinRequest request, + VaadinResponse response, String requestBody) + throws IOException, UnsupportedOperationException { UI uI = session.getService().findUI(request); if (uI == null) { // This should not happen but it will if the UI has been closed. We // really don't want to see it in the server logs though - commitJsonResponse(response, - VaadinService.createUINotFoundJSON(false)); - return true; + return Optional.of(() -> commitJsonResponse(response, + VaadinService.createUINotFoundJSON(false))); } StringWriter stringWriter = new StringWriter(); try { - getRpcHandler(session).handleRpc(uI, request.getReader(), request); + getRpcHandler(session).handleRpc(uI, requestBody, request); writeUidl(uI, stringWriter, false); } catch (JsonException e) { getLogger().error("Error writing JSON to response", e); // Refresh on client side - writeRefresh(response); - return true; + return Optional.of(() -> writeRefresh(response)); } catch (InvalidUIDLSecurityKeyException e) { getLogger().warn("Invalid security key received from {}", request.getRemoteHost()); // Refresh on client side - writeRefresh(response); - return true; + return Optional.of(() -> writeRefresh(response)); } catch (ResynchronizationRequiredException e) { // NOSONAR // Resync on the client side writeUidl(uI, stringWriter, true); @@ -131,8 +149,8 @@ public boolean synchronizedHandleRequest(VaadinSession session, stringWriter.close(); } - commitJsonResponse(response, stringWriter.toString()); - return true; + return Optional.of( + () -> commitJsonResponse(response, stringWriter.toString())); } private void writeRefresh(VaadinResponse response) throws IOException { diff --git a/flow-server/src/test/java/com/vaadin/flow/server/communication/ServerRpcHandlerTest.java b/flow-server/src/test/java/com/vaadin/flow/server/communication/ServerRpcHandlerTest.java index 9f5b699944c..3d7020cbbc9 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/communication/ServerRpcHandlerTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/communication/ServerRpcHandlerTest.java @@ -1,7 +1,6 @@ package com.vaadin.flow.server.communication; import java.io.IOException; -import java.io.Reader; import java.io.StringReader; import org.junit.Assert; @@ -98,12 +97,7 @@ public void handleRpc_resynchronize_throwsExceptionAndDirtiesTreeAndClearsDepend public void handleRpc_duplicateMessage_doNotThrow() throws InvalidUIDLSecurityKeyException, IOException { String msg = "{\"" + ApplicationConstants.CLIENT_TO_SERVER_ID + "\":1}"; - ServerRpcHandler handler = new ServerRpcHandler() { - @Override - protected String getMessage(Reader reader) throws IOException { - return msg; - }; - }; + ServerRpcHandler handler = new ServerRpcHandler(); ui = new UI(); ui.getInternals().setSession(session); @@ -111,23 +105,18 @@ protected String getMessage(Reader reader) throws IOException { MessageDigestUtil.sha256(msg)); // This invocation shouldn't throw. No other checks - handler.handleRpc(ui, Mockito.mock(Reader.class), request); + handler.handleRpc(ui, msg, request); } @Test(expected = UnsupportedOperationException.class) public void handleRpc_unexpectedMessage_throw() throws InvalidUIDLSecurityKeyException, IOException { - ServerRpcHandler handler = new ServerRpcHandler() { - @Override - protected String getMessage(Reader reader) throws IOException { - return "{\"" + ApplicationConstants.CLIENT_TO_SERVER_ID - + "\":1}"; - }; - }; + String msg = "{\"" + ApplicationConstants.CLIENT_TO_SERVER_ID + "\":1}"; + ServerRpcHandler handler = new ServerRpcHandler(); ui = new UI(); ui.getInternals().setSession(session); - handler.handleRpc(ui, Mockito.mock(Reader.class), request); + handler.handleRpc(ui, msg, request); } } diff --git a/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlRequestHandlerTest.java b/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlRequestHandlerTest.java index 94bfd930cd9..75e8b566512 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlRequestHandlerTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/communication/UidlRequestHandlerTest.java @@ -21,6 +21,7 @@ import java.io.OutputStream; import java.io.StringWriter; import java.util.Collections; +import java.util.Optional; import java.util.Properties; import org.junit.Assert; @@ -32,6 +33,7 @@ import com.vaadin.flow.server.DefaultDeploymentConfiguration; import com.vaadin.flow.server.HandlerHelper.RequestType; import com.vaadin.flow.server.MockVaadinContext; +import com.vaadin.flow.server.SynchronizedRequestHandler; import com.vaadin.flow.server.VaadinContext; import com.vaadin.flow.server.VaadinRequest; import com.vaadin.flow.server.VaadinResponse; @@ -40,7 +42,6 @@ import com.vaadin.flow.server.VaadinSession; import com.vaadin.flow.server.startup.ApplicationConfiguration; import com.vaadin.flow.shared.ApplicationConstants; - import elemental.json.JsonObject; import elemental.json.impl.JsonUtil; @@ -108,10 +109,11 @@ public void writeSessionExpired_whenUINotFound() throws IOException { when(service.findUI(request)).thenReturn(null); - boolean result = handler.synchronizedHandleRequest(session, request, - response); - Assert.assertTrue("Result should be true", result); - + Optional result = handler + .synchronizedHandleRequest(session, request, response, null); + Assert.assertTrue("ResponseWriter should be present", + result.isPresent()); + result.get().writeResponse(); String responseContent = CommunicationUtil .getStringWhenWriteString(outputStream);