Downloading a zip file returns a corrupt zip using servlets

Question

Hi All ,

I am trying to create a zip file using servlets but it returns me a corrupt zip file , here is the code for that in zipcontents function i am creating the zip , can someone help me out. Thanks in Advance.

public void doGet(HttpServletRequest req, HttpServletResponse res) throws ServletException,
    IOException {

    ByteArrayOutputStream bout = new ByteArrayOutputStream();
    res.setContentType("application/zip");
    res.setHeader("Content-Disposition", "attachment; filename=output.zip;");

    fsep = File.separator;
    rootDir = new File(getServletContext().getRealPath("Projects" + File.separator + "amrurta"));
    File list[] = rootDir.listFiles();
    zos = new ZipOutputStream(bout);
    zipContents(list, rootDir.getName() + fsep);
    zos.close();
    res.getWriter().println(bout.toString());
}

public void zipContents(File[] file, String dir) {
    // dir - directory in the zip file
    byte[] buffer = new byte[4096];
    try {

        for (int i = 0; i < file.length; i++) { // zip files
            if (file[i].isFile()) {
                fis = new FileInputStream(file[i]);
                zos.putNextEntry(new ZipEntry(dir + file[i].getName()));
                // shows how its stored
                // System.out.println(dir+file[i].getName());
                int bytes_read;
                while ((bytes_read = fis.read(buffer)) != -1)
                    zos.write(buffer, 0, bytes_read);

                fis.close();
            }
        } // for

        // create empty dir if theres no files inside
        if (file.length == 1)
            zos.putNextEntry(new ZipEntry(dir + fsep)); // this part is erroneous i think

        for (int i = 0; i < file.length; i++) { // zip directories
            if (file[i].isDirectory()) {
                File subList[] = file[i].listFiles();

                // for dir of varying depth
                File unparsedDir = file[i];
                String parsedDir = fsep + file[i].getName() + fsep; // last folder
                while (!unparsedDir.getParentFile().getName().equals(rootDir.getName())) {
                    unparsedDir = file[i].getParentFile();
                    parsedDir = fsep + unparsedDir.getName() + parsedDir;
                }
                parsedDir = rootDir.getName() + parsedDir; // add input_output as root

                zipContents(subList, parsedDir);
            }
        } // for

    } catch (IOException ioex) {
        ioex.printStackTrace();
    }
}

Answer 1

There are too much problems in the code. the major ones which springs in are:

1. The zos is been declared as servlet instance variable. This is not threadsafe. It is been shared among multiple requests. You risk that a subseuent request overwrites the previous one when it's not been finished.

2. The binary ZIP content is been converted to character data with bout.toString(). This will definitely corrupt binary data. You should write binary data as binary data using the usual InputStream#read()/OutputStream#write() loop.

3. The code does not call zos.closeEntry() at end of each entry.

I think #2 is the major cause. You don't need ByteArrayOutputStream. It's only an unnecessary memory hog. Just wrap the response.getOutputStream() in ZipOutputStream.

ZipOutputStream output = new ZipOutputStream(response.getOutputStream());
zipFiles(directory.listFiles(), output);
output.close();

Answer 2

One more possible reason is different JVM versions of application server and compiler which compiles servlet. Very rare issue but veeery hard to understand.