From 09fd5d57ad3f322c1662f0d23c8f5890c416c1f9 Mon Sep 17 00:00:00 2001 From: Mikael Grankvist Date: Fri, 22 Nov 2024 12:37:48 +0200 Subject: [PATCH 1/2] feat: resend payload of reoccuring client request If the client request contains the previous id and exactly the same message content respond with the same payload as previously. Closes #20506 --- .../flow/component/internal/UIInternals.java | 21 ++++ .../communication/ServerRpcHandler.java | 21 +++- .../communication/UidlRequestHandler.java | 8 +- .../communication/ServerRpcHandlerTest.java | 6 +- .../communication/UidlRequestHandlerTest.java | 102 ++++++++++++++++-- 5 files changed, 144 insertions(+), 14 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java b/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java index fb6c0dadc88..1a5d6c8a4eb 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java @@ -215,6 +215,8 @@ public List getParameters() { private byte[] lastProcessedMessageHash = null; + private String lastRequestResponse = "for(;;);[{}]"; + private String contextRootRelativePath; private String appId; @@ -305,6 +307,25 @@ public void setLastProcessedClientToServerId( this.lastProcessedMessageHash = lastProcessedMessageHash; } + /** + * Sets the response created for the last UIDL request. + * + * @param lastRequestResponse + * The request that was sent for the last UIDL request. + */ + public void setLastRequestResponse(String lastRequestResponse) { + this.lastRequestResponse = lastRequestResponse; + } + + /** + * Returns the response created for the last UIDL request. + * + * @return The request that was sent for the last UIDL request. + */ + public String getLastRequestResponse() { + return lastRequestResponse; + } + /** * Gets the server sync id. *

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 291d887d4c3..7c52713daeb 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 @@ -236,6 +236,19 @@ public ResynchronizationRequiredException() { } } + /** + * Exception thrown then the client side re-sent the same request. + */ + public static class ResendPayloadException extends RuntimeException { + + /** + * Default constructor for the exception. + */ + public ResendPayloadException() { + super(); + } + } + /** * Reads JSON containing zero or more serialized RPC calls (including legacy * variable changes) and executes the calls. @@ -317,9 +330,11 @@ public void handleRpc(UI ui, String message, VaadinRequest request) * situation is most likely triggered by a timeout or such * causing a message to be resent. */ - getLogger().info( - "Ignoring old duplicate message from the client. Expected: " - + expectedId + ", got: " + requestId); + getLogger().debug( + "Received old duplicate message from the client. Expected: " + + expectedId + ", got: " + requestId + + ". Resending previous response."); + throw new ResendPayloadException(); } else if (rpcRequest.isUnloadBeaconRequest()) { getLogger().debug( "Ignoring unexpected message id from the client on UNLOAD request. " 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 8a90334aac1..f4b4efcd97d 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 @@ -39,6 +39,7 @@ import com.vaadin.flow.server.VaadinService; import com.vaadin.flow.server.VaadinSession; import com.vaadin.flow.server.communication.ServerRpcHandler.InvalidUIDLSecurityKeyException; +import com.vaadin.flow.server.communication.ServerRpcHandler.ResendPayloadException; import com.vaadin.flow.server.communication.ServerRpcHandler.ResynchronizationRequiredException; import com.vaadin.flow.server.dau.DAUUtils; import com.vaadin.flow.server.dau.DauEnforcementException; @@ -134,8 +135,10 @@ public Optional synchronizedHandleRequest( StringWriter stringWriter = new StringWriter(); try { - getRpcHandler(session).handleRpc(uI, requestBody, request); + getRpcHandler().handleRpc(uI, requestBody, request); writeUidl(uI, stringWriter, false); + } catch (ResendPayloadException e) { + stringWriter.write(uI.getInternals().getLastRequestResponse()); } catch (JsonException e) { getLogger().error("Error writing JSON to response", e); // Refresh on client side @@ -176,6 +179,7 @@ void writeUidl(UI ui, Writer writer, boolean resync) throws IOException { // some dirt to prevent cross site scripting String responseString = "for(;;);[" + uidl.toJson() + "]"; + ui.getInternals().setLastRequestResponse(responseString); writer.write(responseString); } @@ -208,7 +212,7 @@ public boolean handleSessionExpired(VaadinRequest request, return true; } - private ServerRpcHandler getRpcHandler(VaadinSession session) { + private ServerRpcHandler getRpcHandler() { ServerRpcHandler handler = rpcHandler.get(); if (handler == null) { rpcHandler.compareAndSet(null, createRpcHandler()); 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 0996a9768ff..8069d960406 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 @@ -99,9 +99,9 @@ public void handleRpc_resynchronize_throwsExceptionAndDirtiesTreeAndClearsDepend Mockito.verify(dependencyList).clearPendingSendToClient(); } - @Test - public void handleRpc_duplicateMessage_doNotThrow() - throws InvalidUIDLSecurityKeyException, IOException { + @Test(expected = ServerRpcHandler.ResendPayloadException.class) + public void handleRpc_duplicateMessage_throwsResendPayload() + throws InvalidUIDLSecurityKeyException { String msg = "{\"" + ApplicationConstants.CLIENT_TO_SERVER_ID + "\":1}"; ServerRpcHandler handler = new ServerRpcHandler(); 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 07440e1a9ef..f9a26c7194e 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 @@ -19,7 +19,6 @@ import java.io.IOException; import java.io.OutputStream; -import java.io.Reader; import java.io.StringWriter; import java.util.Collections; import java.util.Optional; @@ -31,6 +30,7 @@ import org.mockito.Mockito; import com.vaadin.flow.component.UI; +import com.vaadin.flow.function.DeploymentConfiguration; import com.vaadin.flow.server.DefaultDeploymentConfiguration; import com.vaadin.flow.server.HandlerHelper.RequestType; import com.vaadin.flow.server.MockVaadinContext; @@ -46,6 +46,7 @@ import com.vaadin.flow.server.startup.ApplicationConfiguration; import com.vaadin.flow.shared.ApplicationConstants; import com.vaadin.pro.licensechecker.dau.EnforcementException; +import com.vaadin.tests.util.MockUI; import elemental.json.JsonObject; import elemental.json.impl.JsonUtil; @@ -128,9 +129,73 @@ public void writeSessionExpired_whenUINotFound() throws IOException { responseContent); } + @Test + public void clientRequestsPreviousIdAndPayload_resendPreviousResponse() + throws IOException { + + UI ui = getUi(); + VaadinSession session = ui.getSession(); + VaadinService service = session.getService(); + DeploymentConfiguration conf = Mockito + .mock(DeploymentConfiguration.class); + Mockito.when(service.getDeploymentConfiguration()).thenReturn(conf); + Mockito.when(conf.isRequestTiming()).thenReturn(false); + + String requestBody = """ + { + "csrfToken": "d1f44a6f-bbe5-4493-a8a9-3f5f234a2a93", + "rpc": [ + { + "type": "mSync", + "node": 12, + "feature": 1, + "property": "value", + "value": "a" + }, + { + "type": "event", + "node": 12, + "event": "change", + "data": {} + } + ], + "syncId": 0, + "clientId": 0 + } + """; + Mockito.when(request.getService()).thenReturn(service); + Mockito.when(conf.isSyncIdCheckEnabled()).thenReturn(true); + + Optional result = handler + .synchronizedHandleRequest(session, request, response, + requestBody); + Assert.assertTrue("ResponseWriter should be present", + result.isPresent()); + result.get().writeResponse(); + String responseContent = CommunicationUtil + .getStringWhenWriteString(outputStream); + + // Init clean response + response = Mockito.mock(VaadinResponse.class); + outputStream = Mockito.mock(OutputStream.class); + Mockito.when(response.getOutputStream()).thenReturn(outputStream); + + result = handler.synchronizedHandleRequest(session, request, response, + requestBody); + Assert.assertTrue("ResponseWriter should be present", + result.isPresent()); + result.get().writeResponse(); + String resendResponseContent = CommunicationUtil + .getStringWhenWriteString(outputStream); + + // response shouldn't contain async + Assert.assertEquals("Server should send same content again", + responseContent, resendResponseContent); + } + @Test public void should_modifyUidl_when_MPR() throws Exception { - UI ui = mock(UI.class); + UI ui = getUi(); UidlRequestHandler handler = spy(new UidlRequestHandler()); StringWriter writer = new StringWriter(); @@ -151,7 +216,7 @@ public void should_modifyUidl_when_MPR() throws Exception { @Test public void should_changeURL_when_v7LocationProvided() throws Exception { - UI ui = mock(UI.class); + UI ui = getUi(); UidlRequestHandler handler = spy(new UidlRequestHandler()); StringWriter writer = new StringWriter(); @@ -172,7 +237,7 @@ public void should_changeURL_when_v7LocationProvided() throws Exception { @Test public void should_updateHash_when_v7LocationNotProvided() throws Exception { - UI ui = mock(UI.class); + UI ui = getUi(); UidlRequestHandler handler = spy(new UidlRequestHandler()); StringWriter writer = new StringWriter(); @@ -192,7 +257,7 @@ public void should_updateHash_when_v7LocationNotProvided() @Test public void should_not_modify_non_MPR_Uidl() throws Exception { - UI ui = mock(UI.class); + UI ui = getUi(); UidlRequestHandler handler = spy(new UidlRequestHandler()); StringWriter writer = new StringWriter(); @@ -217,7 +282,7 @@ public void should_not_modify_non_MPR_Uidl() throws Exception { @Test public void should_not_update_browser_history_if_no_hash_in_location() throws Exception { - UI ui = mock(UI.class); + UI ui = getUi(); UidlRequestHandler handler = spy(new UidlRequestHandler()); StringWriter writer = new StringWriter(); @@ -351,4 +416,29 @@ private JsonObject getUidlWithNoHashInLocation() { // @formatter:on } + /** + * Mock ui with session. + * + * @return + */ + private static UI getUi() { + VaadinService service = mock(VaadinService.class); + VaadinSession session = new VaadinSession(service) { + @Override + public boolean hasLock() { + return true; + } + + @Override + public VaadinService getService() { + return service; + } + }; + + UI ui = new MockUI(session); + + when(service.findUI(Mockito.any())).thenReturn(ui); + + return ui; + } } From 5e7c145e9f1af01349527240ea0e98afd2dc9990 Mon Sep 17 00:00:00 2001 From: Mikael Grankvist Date: Tue, 26 Nov 2024 10:04:15 +0200 Subject: [PATCH 2/2] Rename exception, remove initial value. --- .../com/vaadin/flow/component/internal/UIInternals.java | 2 +- .../flow/server/communication/ServerRpcHandler.java | 8 ++++---- .../flow/server/communication/UidlRequestHandler.java | 4 ++-- .../flow/server/communication/ServerRpcHandlerTest.java | 3 +-- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java b/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java index 1a5d6c8a4eb..7b58e5cc0f2 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java @@ -215,7 +215,7 @@ public List getParameters() { private byte[] lastProcessedMessageHash = null; - private String lastRequestResponse = "for(;;);[{}]"; + private String lastRequestResponse; private String contextRootRelativePath; 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 7c52713daeb..4612fa50bcc 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 @@ -237,14 +237,14 @@ public ResynchronizationRequiredException() { } /** - * Exception thrown then the client side re-sent the same request. + * Exception thrown when the client side re-sends the same request. */ - public static class ResendPayloadException extends RuntimeException { + public static class ClientResentPayloadException extends RuntimeException { /** * Default constructor for the exception. */ - public ResendPayloadException() { + public ClientResentPayloadException() { super(); } } @@ -334,7 +334,7 @@ public void handleRpc(UI ui, String message, VaadinRequest request) "Received old duplicate message from the client. Expected: " + expectedId + ", got: " + requestId + ". Resending previous response."); - throw new ResendPayloadException(); + throw new ClientResentPayloadException(); } else if (rpcRequest.isUnloadBeaconRequest()) { getLogger().debug( "Ignoring unexpected message id from the client on UNLOAD request. " 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 f4b4efcd97d..f47e19362af 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 @@ -39,7 +39,7 @@ import com.vaadin.flow.server.VaadinService; import com.vaadin.flow.server.VaadinSession; import com.vaadin.flow.server.communication.ServerRpcHandler.InvalidUIDLSecurityKeyException; -import com.vaadin.flow.server.communication.ServerRpcHandler.ResendPayloadException; +import com.vaadin.flow.server.communication.ServerRpcHandler.ClientResentPayloadException; import com.vaadin.flow.server.communication.ServerRpcHandler.ResynchronizationRequiredException; import com.vaadin.flow.server.dau.DAUUtils; import com.vaadin.flow.server.dau.DauEnforcementException; @@ -137,7 +137,7 @@ public Optional synchronizedHandleRequest( try { getRpcHandler().handleRpc(uI, requestBody, request); writeUidl(uI, stringWriter, false); - } catch (ResendPayloadException e) { + } catch (ClientResentPayloadException e) { stringWriter.write(uI.getInternals().getLastRequestResponse()); } catch (JsonException e) { getLogger().error("Error writing JSON to response", e); 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 8069d960406..6c8116a82ff 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; @@ -99,7 +98,7 @@ public void handleRpc_resynchronize_throwsExceptionAndDirtiesTreeAndClearsDepend Mockito.verify(dependencyList).clearPendingSendToClient(); } - @Test(expected = ServerRpcHandler.ResendPayloadException.class) + @Test(expected = ServerRpcHandler.ClientResentPayloadException.class) public void handleRpc_duplicateMessage_throwsResendPayload() throws InvalidUIDLSecurityKeyException { String msg = "{\"" + ApplicationConstants.CLIENT_TO_SERVER_ID + "\":1}";