Display error for unsupported compression file type (#4286)

Closes #4260.

main/src/com/google/refine/commands/importing/ImportingControllerCommand.java

@@ -65,7 +65,11 @@ public class ImportingControllerCommand extends Command {
         if (controller != null) {
         	response.setCharacterEncoding("UTF-8");
             response.setHeader("Content-Type", "application/json");
+            try {
                 controller.doPost(request, response);
+            } catch (IOException e) {
+                HttpUtilities.respond(response, "error", e.getMessage());
+            }
         } else {
             HttpUtilities.respond(response, "error", "No such import controller");
         }

main/src/com/google/refine/importing/ImportingUtilities.java

@@ -57,6 +57,7 @@ import java.util.Properties;
 import java.util.stream.Collectors;
 import java.util.zip.GZIPInputStream;
 import java.util.zip.ZipEntry;
+import java.util.zip.ZipException;
 import java.util.zip.ZipInputStream;
 
 import javax.servlet.ServletException;
@@ -67,6 +68,7 @@ import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
 import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
 import org.apache.commons.compress.compressors.bzip2.BZip2CompressorInputStream;
 import org.apache.commons.fileupload.FileItem;
+import org.apache.commons.fileupload.FileUploadException;
 import org.apache.commons.fileupload.ProgressListener;
 import org.apache.commons.fileupload.disk.DiskFileItemFactory;
 import org.apache.commons.fileupload.servlet.ServletFileUpload;
@@ -136,7 +138,7 @@ public class ImportingUtilities {
             JSONUtilities.safePut(config, "state", "error");
             JSONUtilities.safePut(config, "error", "Error uploading data");
             JSONUtilities.safePut(config, "errorDetails", e.getLocalizedMessage());
-            return;
+            throw new IOException(e.getMessage());
         }
         
         ArrayNode fileSelectionIndexes = ParsingUtilities.mapper.createArrayNode();
@@ -173,7 +175,7 @@ public class ImportingUtilities {
         File rawDataDir,
         ObjectNode retrievalRecord,
         final Progress progress
-    ) throws Exception {
+    ) throws IOException, FileUploadException {
         ArrayNode fileRecords = ParsingUtilities.mapper.createArrayNode();
         JSONUtilities.safePut(retrievalRecord, "files", fileRecords);
         
@@ -526,7 +528,8 @@ public class ImportingUtilities {
         abstract public void savedMore();
         abstract public boolean isCanceled();
     }
-    static public long saveStreamToFile(InputStream stream, File file, SavingUpdate update) throws IOException {
+
+    static private long saveStreamToFile(InputStream stream, File file, SavingUpdate update) throws IOException {
         long length = 0;
         FileOutputStream fos = new FileOutputStream(file);
         try {
@@ -542,13 +545,15 @@ public class ImportingUtilities {
                 }
             }
             return length;
+        } catch (ZipException e) {
+            throw new IOException("Compression format not supported, " + e.getMessage());
         } finally {
             fos.close();
         }
     }
     
     static public boolean postProcessRetrievedFile(
-            File rawDataDir, File file, ObjectNode fileRecord, ArrayNode fileRecords, final Progress progress) {
+            File rawDataDir, File file, ObjectNode fileRecord, ArrayNode fileRecords, final Progress progress) throws IOException {
         
         String mimeType = JSONUtilities.getString(fileRecord, "declaredMimeType", null);
         String contentEncoding = JSONUtilities.getString(fileRecord, "declaredEncoding", null);
@@ -631,13 +636,13 @@ public class ImportingUtilities {
     }
     
     // FIXME: This is wasteful of space and time. We should try to process on the fly
-    static public boolean explodeArchive(
+    static private boolean explodeArchive(
         File rawDataDir,
         InputStream archiveIS,
         ObjectNode archiveFileRecord,
         ArrayNode fileRecords,
         final Progress progress
-    ) {
+    ) throws IOException {
         if (archiveIS instanceof TarArchiveInputStream) {
             TarArchiveInputStream tis = (TarArchiveInputStream) archiveIS;
             try {
@@ -670,7 +675,6 @@ public class ImportingUtilities {
             return true;
         } else if (archiveIS instanceof ZipInputStream) {
             ZipInputStream zis = (ZipInputStream) archiveIS;
-            try {
             ZipEntry ze;
             while (!progress.isCanceled() && (ze = zis.getNextEntry()) != null) {
                 if (!ze.isDirectory()) {
@@ -693,10 +697,6 @@ public class ImportingUtilities {
                     JSONUtilities.append(fileRecords, fileRecord2);
                 }
             }
-            } catch (IOException e) {
-                // TODO: what to do?
-                e.printStackTrace();
-            }
             return true;
         }
         return false;

main/tests/server/src/com/google/refine/importing/ImportingUtilitiesTests.java

@@ -26,10 +26,9 @@
  ******************************************************************************/
 package com.google.refine.importing;
 
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
-import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.*;
-import static org.testng.Assert.assertTrue;
-import static org.testng.Assert.fail;
 
 import java.io.File;
 import java.io.IOException;
@@ -45,12 +44,12 @@ import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
 
 import org.apache.commons.collections.IteratorUtils;
+import org.apache.commons.fileupload.FileUploadBase;
 import org.apache.commons.io.FileUtils;
 import org.apache.http.HttpEntity;
 import org.apache.http.entity.ContentType;
 import org.apache.http.entity.mime.MultipartEntityBuilder;
 import org.apache.http.entity.mime.content.StringBody;
-import org.mockito.Mockito;
 import org.testng.Assert;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
@@ -69,6 +68,7 @@ import com.google.refine.util.TestUtils;
 import javax.servlet.ReadListener;
 import javax.servlet.ServletInputStream;
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 public class ImportingUtilitiesTests extends ImporterTest {
 
@@ -133,7 +133,7 @@ public class ImportingUtilitiesTests extends ImporterTest {
         entity.writeTo(os);
         ByteArrayInputStream is = new ByteArrayInputStream(os.toByteArray());
 
-        HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
+        HttpServletRequest req = mock(HttpServletRequest.class);
         when(req.getContentType()).thenReturn(entity.getContentType().getValue());
         when(req.getParameter("download")).thenReturn(url.toString());
         when(req.getMethod()).thenReturn("POST");
@@ -284,4 +284,40 @@ public class ImportingUtilitiesTests extends ImporterTest {
         assertEquals(importOptions.get("archiveFileName").asText(), "movies.zip");
     }
 
+    @Test
+    public void importUnsupportedZipFile() throws IOException{
+        String filename = "unsupportedPPMD.zip";
+        String filepath = ClassLoader.getSystemResource(filename).getPath();
+        // Make a copy in our data directory where it's expected
+        File tmp = File.createTempFile("openrefine-test-unsupportedPPMD", ".zip", job.getRawDataDir());
+        tmp.deleteOnExit();
+        FileUtils.copyFile(new File(filepath), tmp);
+
+        Progress dummyProgress = new Progress() {
+            @Override
+            public void setProgress(String message, int percent) {}
+
+            @Override
+            public boolean isCanceled() {
+                return false;
+            }
+        };
+
+        ArrayNode fileRecords = ParsingUtilities.mapper.createArrayNode();
+        ObjectNode fileRecord = ParsingUtilities.mapper.createObjectNode();
+        JSONUtilities.safePut(fileRecord, "origin", "upload");
+        JSONUtilities.safePut(fileRecord, "declaredEncoding", "UTF-8");
+        JSONUtilities.safePut(fileRecord, "declaredMimeType", "application/x-zip-compressed");
+        JSONUtilities.safePut(fileRecord, "fileName", filename);
+        JSONUtilities.safePut(fileRecord, "location", tmp.getName());
+
+        HttpServletRequest request = mock(HttpServletRequest.class);
+        HttpServletResponse response = mock(HttpServletResponse.class);
+
+        assertThrows(IOException.class, () -> ImportingUtilities.postProcessRetrievedFile(job.getRawDataDir(), tmp, fileRecord, fileRecords, dummyProgress));
+        assertThrows(FileUploadBase.InvalidContentTypeException.class, () -> ImportingUtilities.retrieveContentFromPostRequest(request, new Properties(), job.getRawDataDir(), fileRecord, dummyProgress));
+        assertThrows(IOException.class, () -> ImportingUtilities.loadDataAndPrepareJob(request, response, new Properties(), job, fileRecord));
+
+    }
+
 }