Prevent NPE on missing security key. (#18573)

Change-Id: I6d9eeb257a4a4889ce679a31a45133c3d0c90368

diff --git a/server/src/com/vaadin/server/communication/FileUploadHandler.java b/server/src/com/vaadin/server/communication/FileUploadHandler.java
index 576cbd84112d3b323bc26079ca8ea564fbf3b662..532c7fe95b0631d6f5a3bbdaf64fc9ed7267dcb9 100644 (file)
--- 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 2cb4c3bf4d461706287fdfc5b19d45201d22b2ae..286163541e4c2abb4028dc6e21851bc4013a8e38 100644 (file)
--- 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);
+    }
 }