Downloading and unzipping file in another thread giving corrupted archive

Question

I'm trying to download a .zip archive from a given url and unzip it to a selected location. Since the download is quite large, I tried to do it in a different thread so it doesn't freeze the entire thing, but it didn't really turn out to be a complete success... Normally, I would have just create a runnable and thread object, but now, since I'm using javafx, it gives some error. I searched online and I had to use Platform.runLater() instead. Here's my code:

private void startTask(Label st, ListView<String> view, HashMap<String, String> hash)
{

    Platform.runLater(new Runnable() {
        @Override
        public void run() {
            runTask(st,view,hash);
        }
    });
}

private void runTask(Label st, ListView<String> view, HashMap<String, String> hash){
    String link = hash.get(view.getSelectionModel().getSelectedItem());
    File file = new File("temp.zip");
    try {
        FileUtils.copyURLToFile(new URL(link), file);
        byte[] buffer = new byte[1024];
        ZipInputStream zis = new ZipInputStream(new FileInputStream(file));
        ZipEntry zipEntry = zis.getNextEntry();
        File newFile;
        FileOutputStream fos;
        String fileName;
        while(zipEntry != null){

            fileName = zipEntry.getName();
            if(fileName.contains("##TEMP##")) continue;
            if(fileName.contains("MACOSX")) continue;
            newFile = new File(fileName);
            fos = new FileOutputStream(newFile);
            int len;
            while ((len = zis.read(buffer)) > 0) {
                fos.write(buffer, 0, len);
            }
            fos.close();
            zipEntry = zis.getNextEntry();
        }
        zis.closeEntry();
        zis.close();
    } catch (IOException | NullPointerException e) {
        e.printStackTrace();
    }
    st.setText("Status: Ready");


}

And here's the result: A 39ko temp.zip file that appears to be corrupted. It doesn't get unzipped (Obviously). I tried many links from multiple domains, still same problem...

EDIT: Tried a different method... Same error.. New Code:

        URL url = new URL(link);
        HttpURLConnection connection = (HttpURLConnection) url.openConnection();
        connection.setRequestMethod("GET");
        InputStream in = connection.getInputStream();
        FileOutputStream out = new FileOutputStream("download.zip");
        copy(in, out, 1024);
        out.close();
public static void copy(InputStream input, OutputStream output, int bufferSize) throws IOException {
    byte[] buf = new byte[bufferSize];
    int n = input.read(buf);
    while (n >= 0) {
        output.write(buf, 0, n);
        n = input.read(buf);
    }
    output.flush();
}

I commented the unzipping part, still the same... Conclusion: downloading is the issue.

What makes you think the problem has anything to do with JavaFX? Does the exact same code work if run in a console application? — Joe C, Jan 23 '18 at 22:09
I'm not saying it has anything to do with javafx... I said I had to use `Platform.runLater()` so nobody asks me why I did. It most likely does the exact same thing in console. — Etienne Poulin, Jan 23 '18 at 22:11
From the link, no, I've downloaded it first and opened it. Plus it's supposed to be 2mb+. — Etienne Poulin, Jan 23 '18 at 22:15
@Sedrick https://stackoverflow.com/questions/17850191/why-am-i-getting-java-lang-illegalstateexception-not-on-fx-application-thread I had that error, and every thread I found was saying that. — Etienne Poulin, Jan 23 '18 at 22:29
Yea, you do because of `st.setText("Status: Ready");`. I didn't see that at first. — SedJ601, Jan 23 '18 at 22:30
you do a if(fileName.contains()) check at the top of the while loop which can continue, but zipEntry = zis.getNextEntry(); is at the bottom of the loop, so zis.getNextEntry() won't be called. is it getting stuck in an infinite loop? — slipperyseal, Jan 23 '18 at 22:41
Another problem you have is that two of these threads can't execute concurrently, because of the fixed filename `test.zip`. You need to make it dynamic. If you don't need the actual .zip file it would be more efficient to unzip directly from the URL connection's input stream. — user207421, Jan 23 '18 at 22:49
Tried a different method and added it to post... Is it better or worst ? — Etienne Poulin, Jan 23 '18 at 23:17
It's basically the same. `flush()` before `close()` is redundant. Is the actual ZIP file itself OK? Openable with WinZip? Or is it maybe an HTML page? — user207421, Jan 23 '18 at 23:31
A side note: Don't download and unzip the file in a `runLater()` task. Do this in a background thread. Only after downloading and unzipping the file, that task should invoke `runLater()` to update the UI. The JavaFX application thread is where all GUI events are handled. Using it to perform network IO and other unreliable, potentially long duration tasks will make the UI unresponsive. — erickson, Jan 23 '18 at 23:35
So if you can't open it with WinZip it's being corrupted somehow during the download, or it's corrupt at source, or at least not a ZIP file. Nothing to do with your unzip code. Nothing wrong with your downloading code either, in either version. Have you inspected it for HTML? — user207421, Jan 23 '18 at 23:55
@EJP You were right... I was so convinced that it was a zip file that I didn't even consider the possibility of it being an html file... The ending of the url I downloaded it from was `.zip` so I assumed it was a zip file... Any idea on how to download the zip from that ? Because using the browser downloads it directly. — Etienne Poulin, Jan 24 '18 at 00:11
It must contain a redirection of some kind, say a `meta refresh`, or some JavaScript. Not much you can do about that really. — user207421, Jan 24 '18 at 00:51

slipperyseal · Accepted Answer · 2018-01-23T22:51:24.043

You do fileName.contains() checks at the top of the while loop which can continue, returning to the start of the while loop. But zipEntry = zis.getNextEntry() is at the bottom of the loop, so it won't be called in these cases. It looks like it is getting stuck in an infinite loop

     while(zipEntry != null) {
        fileName = zipEntry.getName();
        if(fileName.contains("##TEMP##")) continue;
        if(fileName.contains("MACOSX")) continue;

        // unzipping code

        zipEntry = zis.getNextEntry(); // never happens when filename  contains ##TEMP## or MACOSX 
    }

Moving the getNextEntry() code to the top of the loop and remove the first call from the variable declaration..

     ZipEntry zipEntry;

     while((zipEntry = zis.getNextEntry()) != null) {
        fileName = zipEntry.getName();
        if(fileName.contains("##TEMP##")) continue;
        if(fileName.contains("MACOSX")) continue;

        // unzipping code
    }

Tried that, still doesn't work.. I even tried it without the `fileName.contains()` but still no luck... — Etienne Poulin, Jan 23 '18 at 22:54
can you put some logging in to see what it is doing at various points in the code. my concern is that you pass the filename from the zip directly into new File(fileName); it might not be able to create the file at that path. Its also a big security risk. — slipperyseal, Jan 23 '18 at 23:02

Downloading and unzipping file in another thread giving corrupted archive

1 Answers1