From e7fda93b300b11d9507f25917306e1f3d57cb27c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Sauli=20T=C3=A4hk=C3=A4p=C3=A4=C3=A4?= Date: Sat, 15 Aug 2015 23:14:14 +0300 Subject: [PATCH] Prevent NPE on missing security key. (#18573) Change-Id: I6d9eeb257a4a4889ce679a31a45133c3d0c90368 --- .../communication/FileUploadHandler.java | 2 +- .../communication/FileUploadHandlerTest.java | 120 +++++++++++++++--- 2 files changed, 104 insertions(+), 18 deletions(-) diff --git a/server/src/com/vaadin/server/communication/FileUploadHandler.java b/server/src/com/vaadin/server/communication/FileUploadHandler.java index 576cbd8411..532c7fe95b 100644 --- a/server/src/com/vaadin/server/communication/FileUploadHandler.java +++ b/server/src/com/vaadin/server/communication/FileUploadHandler.java @@ -269,7 +269,7 @@ public class FileUploadHandler implements RequestHandler { streamVariable = uI.getConnectorTracker().getStreamVariable( connectorId, variableName); String secKey = uI.getConnectorTracker().getSeckey(streamVariable); - if (!secKey.equals(parts[3])) { + if (secKey == null || !secKey.equals(parts[3])) { // TODO Should rethink error handling return true; } diff --git a/server/tests/src/com/vaadin/server/communication/FileUploadHandlerTest.java b/server/tests/src/com/vaadin/server/communication/FileUploadHandlerTest.java index 2cb4c3bf4d..286163541e 100644 --- a/server/tests/src/com/vaadin/server/communication/FileUploadHandlerTest.java +++ b/server/tests/src/com/vaadin/server/communication/FileUploadHandlerTest.java @@ -15,50 +15,136 @@ */ package com.vaadin.server.communication; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import org.junit.Before; import org.junit.Test; -import org.mockito.Mockito; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import com.vaadin.server.ClientConnector; +import com.vaadin.server.ServletPortletHelper; +import com.vaadin.server.StreamVariable; import com.vaadin.server.VaadinRequest; +import com.vaadin.server.VaadinResponse; +import com.vaadin.server.VaadinSession; +import com.vaadin.ui.ConnectorTracker; +import com.vaadin.ui.UI; -/** - * Tests whether we get infinite loop if InputStream is already read (#10096) - */ public class FileUploadHandlerTest { private FileUploadHandler handler; - private VaadinRequest request; + @Mock private VaadinResponse response; + @Mock private StreamVariable streamVariable; + @Mock private ClientConnector clientConnector; + @Mock private VaadinRequest request; + @Mock private UI ui; + @Mock private ConnectorTracker connectorTracker; + @Mock private VaadinSession session; + @Mock private OutputStream responseOutput; + + private int uiId = 123; + private final String connectorId = "connectorId"; + private final String variableName = "name"; + private final String expectedSecurityKey = "key"; @Before public void setup() throws Exception { + MockitoAnnotations.initMocks(this); + handler = new FileUploadHandler(); - InputStream inputStream = new InputStream() { - private int counter = 0; + + mockRequest(); + mockConnectorTracker(); + mockUi(); + + when(clientConnector.isConnectorEnabled()).thenReturn(true); + when(streamVariable.getOutputStream()).thenReturn(mock(OutputStream.class)); + when(response.getOutputStream()).thenReturn(responseOutput); + } + + private void mockConnectorTracker() { + when(connectorTracker.getSeckey(streamVariable)).thenReturn(expectedSecurityKey); + when(connectorTracker.getStreamVariable(connectorId, variableName)).thenReturn(streamVariable); + when(connectorTracker.getConnector(connectorId)).thenReturn(clientConnector); + } + + private void mockRequest() throws IOException { + when(request.getPathInfo()).thenReturn("/" + ServletPortletHelper.UPLOAD_URL_PREFIX + uiId + "/"+ connectorId + "/" + variableName + "/" + expectedSecurityKey); + when(request.getInputStream()).thenReturn(createInputStream("foobar")); + when(request.getHeader("Content-Length")).thenReturn("6"); + when(request.getContentType()).thenReturn("foobar"); + } + + private InputStream createInputStream(final String content) { + return new InputStream() { + int counter = 0; + byte[] msg = content.getBytes(); @Override public int read() throws IOException { - counter++; - if (counter > 6) { - throw new RuntimeException( - "-1 is ignored by FileUploadHandler"); + if(counter > msg.length + 1) { + throw new AssertionError("-1 was ignored by FileUploadHandler."); + } + + if(counter >= msg.length) { + counter++; + return -1; } - return -1; - } + return msg[counter++]; + } }; - request = Mockito.mock(VaadinRequest.class); - Mockito.when(request.getInputStream()).thenReturn(inputStream); - Mockito.when(request.getHeader("Content-Length")).thenReturn("211"); } + private void mockUi() { + when(ui.getConnectorTracker()).thenReturn(connectorTracker); + when(session.getUIById(uiId)).thenReturn(ui); + } + + /** + * Tests whether we get infinite loop if InputStream is already read (#10096) + */ @Test(expected = IOException.class) - public void testStreamEnded() throws IOException { + public void exceptionIsThrownOnUnexpectedEnd() throws IOException { + when(request.getInputStream()).thenReturn(createInputStream("")); + when(request.getHeader("Content-Length")).thenReturn("1"); + handler.doHandleSimpleMultipartFileUpload(null, request, null, null, null, null, null); + } + + @Test + public void responseIsSentOnCorrectSecurityKey() throws IOException { + when(connectorTracker.getSeckey(streamVariable)).thenReturn(expectedSecurityKey); + + handler.handleRequest(session, request, response); + verify(responseOutput).close(); } + @Test + public void responseIsNotSentOnIncorrectSecurityKey() throws IOException { + when(connectorTracker.getSeckey(streamVariable)).thenReturn("another key expected"); + + handler.handleRequest(session, request, response); + + verifyZeroInteractions(responseOutput); + } + + @Test + public void responseIsNotSentOnMissingSecurityKey() throws IOException { + when(connectorTracker.getSeckey(streamVariable)).thenReturn(null); + + handler.handleRequest(session, request, response); + + verifyZeroInteractions(responseOutput); + } } -- 2.39.5