Issue with Download using ZipOutputStream

[Anand Bhatt]

Hi ,

I have Download functionality in my application. The function download the file in .xls format and zipped it. So that user can save that zip file on this local machine.
Everything workign fine but the file which is zipped inside have no extension. Example I am expeting the user to download xyz.zip file which interns contains xyz.xls,but unfortunately file having no extension (downloaded as xyz) inside xyz.zip.I am using below statement to zip the file.

response.setContentType("application/zip");
response.setHeader("Content-Disposition", "attachment; filename="+xyz.zip+";");
ZipOutputStream outZip = new ZipOutputStream(response.getOutputStream());
FileInputStream in = new FileInputStream(xyz.xls);
outZip.putNextEntry(new ZipEntry(xyz.xls));

Please suggest,there is no error/exception found in logs.

[Joe Ess]

Can you show us your real code? There's a couple of problems in the code above that would not compile, much less cause a runtime problem.

[Anand Bhatt]

public void generateExcelFile( String filePath, String queryStr, String loginName)
	throws GenericDBAccessException, Exception
	{

		FileOutputStream fileOut = null;

		ResultSet rs = null;
		Connection conn = null;
		PreparedStatement pstmt = null;

		String[] sheetHeader = {"A", "B","C", "D", "E", 
				"F", "G", "H", "I", "J", 
		"K"};

		int rowCount = 0;

		
		try{
			
			if(filePath != null && filePath.length() > 0){		
								
								fileOut = new FileOutputStream (new File(filePath));
			}

			HSSFWorkbook wb=new HSSFWorkbook();
			HSSFSheet sheet = wb.createSheet("File Details");  
			HSSFRow row= null;
			HSSFCell cellx= null;
			


			HSSFFont font1 = wb.createFont();
			HSSFFont font2 = wb.createFont();	

			font1.setFontHeightInPoints((short)8);
			font1.setBoldweight(HSSFFont.BOLDWEIGHT_BOLD);
			font1.setColor(HSSFColor.BLACK.index);
			font1.setFontName("MS Sans Serif");

			font2.setFontHeightInPoints((short)8);
			font2.setColor(HSSFColor.BLACK.index);
			font2.setFontName("MS Sans Serif");

			HSSFCellStyle cs1 = wb.createCellStyle();
			cs1.setAlignment(HSSFCellStyle.ALIGN_CENTER);
			cs1.setWrapText(true);
			cs1.setFillPattern((short) HSSFCellStyle.SOLID_FOREGROUND);
			cs1.setFillForegroundColor((short)41) ;
			cs1.setBorderBottom(HSSFCellStyle.BORDER_THIN);
			cs1.setBorderLeft(HSSFCellStyle.BORDER_THIN);
			cs1.setBorderRight(HSSFCellStyle.BORDER_THIN);
			cs1.setBorderTop(HSSFCellStyle.BORDER_THIN);
			cs1.setFont(font1);
			//cs1.setDataFormat(format.getFormat("dd-MM-yyyy"));

			HSSFCellStyle cs2 = wb.createCellStyle();
			cs2.setAlignment(HSSFCellStyle.ALIGN_CENTER);
			cs2.setWrapText(true);
			cs2.setBorderBottom(HSSFCellStyle.BORDER_THIN);
			cs2.setBorderLeft(HSSFCellStyle.BORDER_THIN);
			cs2.setBorderRight(HSSFCellStyle.BORDER_THIN);
			cs2.setBorderTop(HSSFCellStyle.BORDER_THIN);
			cs2.setFont(font2);
			//cs2.setDataFormat(format.getFormat("#,##0.00"));

			//Style for the note at the top
			HSSFCellStyle cs3 = wb.createCellStyle();
			cs3.setAlignment(HSSFCellStyle.ALIGN_CENTER);
			cs3.setWrapText(true);
			cs3.setBorderBottom(HSSFCellStyle.BORDER_THIN);
			cs3.setBorderLeft(HSSFCellStyle.BORDER_THIN);
			cs3.setBorderRight(HSSFCellStyle.BORDER_THIN);
			cs3.setBorderTop(HSSFCellStyle.BORDER_THIN);
			
			HSSFFont font3 = wb.createFont();
			font3.setFontHeightInPoints((short)8);
			font3.setColor(HSSFColor.RED.index);
			font3.setFontName("MS Sans Serif");
			font3.setBoldweight(HSSFFont.BOLDWEIGHT_BOLD);
			
			cs3.setFont(font3);

			sheet.addMergedRegion(new Region(0,(short)0,1,(short)9));
			
			row = sheet.createRow(rowCount);
			cellx = row.createCell((short)0);
			cellx.setCellValue(" abc ");
			cellx.setCellStyle(cs3);
			rowCount++;

			row = sheet.createRow(rowCount);
			rowCount++;
			
			HSSFRow headerRow=sheet.createRow(rowCount);
			HSSFCell headerCell = null;
			for(int i=0; i < sheetHeader.length; i++){
				headerCell = headerRow.createCell((short) i);
				headerCell.setCellValue(sheetHeader[i]);
				headerCell.setCellStyle(cs1);
			}




			HSSFCell cell0 = null;
			HSSFCell cell1 = null;
			HSSFCell cell2 = null;
			HSSFCell cell3 = null;
			HSSFCell cell4 = null;
			HSSFCell cell5 = null;
			HSSFCell cell6 = null;
			HSSFCell cell7 = null;
			HSSFCell cell8 = null;
			HSSFCell cell9 = null;
			HSSFCell cell10 = null;


			conn = Connectionmanager.getConnection();

			pstmt = conn.prepareStatement(queryStr);
			
					
			rs = pstmt.executeQuery();

			while(rs.next()){

				rowCount++;
				row = sheet.createRow(rowCount);

				cell0 = row.createCell((short)  0);
				cell0.setCellValue(rs.getString("A"));
				cell0.setCellStyle(cs2);
				
				cell1 = row.createCell((short)  1);
				cell1.setCellValue(rs.getString("B"));
				cell1.setCellStyle(cs2);

				cell2 = row.createCell((short)  2);
				cell2.setCellValue(rs.getString("C"));
				cell2.setCellStyle(cs2);

				cell3 = row.createCell((short)  3);
				cell3.setCellValue(rs.getString("D"));
				cell3.setCellStyle(cs2);

				cell4 = row.createCell((short)  4);
				cell4.setCellValue(rs.getString("E"));
				cell4.setCellStyle(cs2);

				cell5 = row.createCell((short)  5);
				cell5.setCellValue(rs.getString("F"));
				cell5.setCellStyle(cs2);

				cell6 = row.createCell((short)  6);
				cell6.setCellValue(rs.getString("G"));
				cell6.setCellStyle(cs2);

				cell7 = row.createCell((short)  7);
				cell7.setCellValue(rs.getString("H"));
				cell7.setCellStyle(cs2);

				cell8 = row.createCell((short)  8);
				cell8.setCellValue(rs.getString("I"));
				cell8.setCellStyle(cs2);

				cell9 = row.createCell((short)  9);
				cell9.setCellValue(rs.getString("J"));
				cell9.setCellStyle(cs2);

				cell10 = row.createCell((short)  10);
				cell10.setCellValue(rs.getString("K"));
				cell10.setCellStyle(cs2);

			}


			wb.write(fileOut);


		} catch (SQLException sqe) {
			LOGGER.error("An SQLException has occured-->",sqe);
		sqe.printStackTrace();
			throw sqe;	
		} catch (Exception e) {
			LOGGER.error("An Exception has occured-->",e);
			e.printStackTrace();
			throw e;
		} finally {
			Connectionmanager.closeDBResources(_connection, pstmt, rs);
			try {
				if (rs != null) {rs.close();}		
				if (pstmt != null) {pstmt.close();}
				if (conn != null) {	conn.close();}
			} catch (Exception e) {
			}		
		}		
	}
}

[Paul Clapham]

I'm pretty sure Joe was asking for some real code which illustrated the problem you asked about. What you just posted may be real code, but it doesn't have anything to do with your problem.

[Anand Bhatt]

Paul,
This what responsible for deownloading and zipped the .xls sheet. apart from this there is no code.Just in one jsp I am calling this method which is implemented in struts Action class.

[Greg Charles]

Hi Anand,

You might want to check the code you posted. There's no mention of ZipOutputStream, or zipping at all. It also doesn't contain the sample lines you had in your original post, which look like they come from a servlet doGet() or doPost() method.

[Anand Bhatt]

real code to zip the file

public ActionForward downloadSheet(ActionMapping mapping, ActionForm form, HttpServletRequest request, HttpServletResponse response)
		  {
			String filePath = servlet.getServletContext().getRealPath("/");
			
			XYZActionForm XYZActionForm = (XYZActionForm)form; 
			HttpSession session = request.getSession();
			String searchClause = null;
			Integer totalRecordsCount;
				
			String loginName="loginname"
			
			String userName;
			try {
			userName = manager.getLoginUserName(loginName);
			XYZActionForm.setUserName(userName);
			searchClause = XYZActionForm.getSearchClause();			
			if(searchClause == null){
				searchClause = "";
			}
					
			totalRecordsCount = manager.getCountCount(searchClause,loginName);
			
			session.setAttribute("totalRecordsCount" , totalRecordsCount);
			
			manager.getEntireSheet(filePath, searchClause, loginName);

     		File file = new File(filePath+xyz.xls);
     		
     		   		
			if(file.exists()) {
				
				
				response.reset();
				response.setContentType("application/zip"); 
				response.setHeader("Content-Disposition", "attachment; filename="XYZ.zip+";"); 
				
				ZipOutputStream outZip;
			
					outZip = new ZipOutputStream(response.getOutputStream());
			
				FileInputStream in = new FileInputStream(file);
		
		        // Add ZIP entry to output stream.
		        outZip.putNextEntry(new ZipEntry(XYZ.xls));
		
		        // Transfer bytes from the file to the ZIP file
		        int len;
		        byte[] buf = new byte[4096];
				while ((len = in.read(buf)) > 0) {
		            outZip.write(buf, 0, len);
		        }
		
		        // Complete the entry
		        outZip.closeEntry();
		        in.close();
			outZip.flush() ;
		        outZip.close() ;
			file.delete();
			}else {
				 LOGGER.info("Download failed as no such file named '" );
			}	
			
			} catch (Exception e) {
					
					e.printStackTrace();
				}
			LOGGER.info("Download Charge Completed........");
			return null;
		}

[Rob Spoor]

Can you please UseCodeTags next time? Thanks.

[Anand Bhatt]

Let me brief on this

After download I can see zip inside the other zip file(but inner zip having no extension )

So what exactly happening is

XYZ.zip file downloaded which when extracted contains a file XYZ(no extension though) which than extracted usinh WinRar contains the actualxls file.

So why it is getting zipped twice is issue .

[Wouter Oet]

And you probably want to close your resources correctly (they don't get closed when an exception occurs).

[Anand Bhatt]

Hi Wouter,

Resources are closed properly and exception handling as well done where we expecting some excetion to be thrown.

The same code working when applicaion got hit from some machines but for few others it is not working.

[Wouter Oet]

No they're not. What happens when an exception is thrown at line 50? Then the methods calls which close the resources are skipped and a stacktrace is printed.

[Anand Bhatt]

Thanks Wouter for higlightinh that point,but i made modification to that code already so the updated code is somewhat:

while ((len = fin.read(buf)) > 0) {
	            outZip.write(buf, 0, len);
	        }
			outZip.finish();
		    outZip.close();
			fin.close();
			outZip.close();
			} catch(Exception e){
				LOGGER.error("An Exception occured whileZipping   the downloaded sheet  ...."+e.getLocalizedMessage());
				e.printStackTrace();
			} finally {
				[b]IOUtils.closeQuietly(fin);[/b]				[b]IOUtils.closeQuietly(outZip);[/b]		  	    //file.delete();
				LOGGER.info("Inside  sheet download finally block----> '");
			}

So bold part here doing that closing of resources now.

[Wouter Oet]

Only now you might close fin and outZip twice.

[Anand Bhatt]

OK. Wouter could you please suggest than what should I need to do here please. Keeping this code in mind.

[Rob Spoor]

Always close the outer-most stream. That should also close the stream it was wrapping. In this case it's very, very important to close the ZipOutputStream before you close the FileOutputStream, as the ZipOutputStream will write some extra bytes to close the ZIP file itself. If you close the FileOutputStream first those bytes will be missing and the ZIP file will be corrupt.

[Anand Bhatt]

Thanks Rob,
In below code snippet you are saying I need to close zipOutputStreamObject beore fileInputStreamObject?

try {
				zipOutputStreamObject = new ZipOutputStream(new FileOutputStream(XYZ.zip));
				zipOutputStreamObject.setLevel(Deflater.DEFAULT_COMPRESSION);
				fileInputStreamObject  = new FileInputStream(xyz_test.xls);
				zipOutputStreamObject.putNextEntry(new ZipEntry(xyz_test.xls));
				// Transfer bytes from the file to the ZIP file
				int len;
				while ((len = fileInputStreamObject.read(buf)) > 0) {
					zipOutputStreamObject.write(buf, 0, len);
				}
			
			
			} catch(Exception e){
				LOGGER.error("An Exception occured whileZipping   the downloaded sheet  ...."+e.getLocalizedMessage());
				e.printStackTrace();
			} finally {
				try {
					[b]zipOutputStreamObject.closeEntry();
					fileInputStreamObject.close();
					zipOutputStreamObject.close();[/b]				} catch (IOException ioe) {
					LOGGER.error("An Exception occured while closing  IO resources--->"+ ioe.getLocalizedMessage());
					ioe.printStackTrace();
				} 
		  	    //file.delete();
				LOGGER.info("Inside  sheet download finally block----> '");
			}

[Anand Bhatt]

Also I my code I havenot used any FileOutputStream ,which code you are referring .? Indeed it is FileInputStream

[Wouter Oet]

Getting closer but if zipOutputStreamObject.closeEntry() throws an exception when closing the other resources will not be closed.

[Rob Spoor]

Also, what did I tell you about closing the FileInputStream before closing the ZipOutputStream?

[Greg Charles]

Rob Spoor wrote:Also, what did I tell you about closing the FileInputStream before closing the ZipOutputStream?

Actually, you said not to close a FileOutputStream before you close a ZipOutputStream that wraps it. Anand asked you about that specifically a couple of posts ago because he's wrapping the servlet's output stream, not a FileOutputStream, but is using a FileInputStream to read the file. I don't think it matters when you close the Input Stream as long as you've finished reading from it. However, I also don't see anything that would cause the "double zip" issue Anand is seeing.

[Rob Spoor]

Ah yes, my apologies. I read fileOutputStreamObject instead of fileInputStreamObject.

[Anand Bhatt]

Hi Greg ,


} finally {
try {
zipOutputStreamObject.closeEntry();
fileInputStreamObject.close();
zipOutputStreamObject.close();[/b]
} catch (IOException ioe) {
LOGGER.error("An Exception occured while closing IO resources--->"+ ioe.getLocalizedMessage());
ioe.printStackTrace();
}


I can see that if I get any exception in zipOutputStreamObject.closeEntry(); than I have another catch block to handle it. So I am bit confused
what ele I need to do here.

[Anand Bhatt]

Thanks all for your valuable suggestoions.
I manged to get rid of this issue somehow.

[Greg Charles]

Hi Anand,

I think Wouter meant that if an exception happens at zipOutputStreamObject.closeEntry(), your program will jump immediately to the the catch block, bypassing the fileInputStreamObject.close() and zipOutputStreamObject.close(). You should really put each close into its own try-catch block or use one of those closeQuietly() utility methods. It's a bigger deal when you're closing something like a database connection or other scarce resources. I've seen server slow down and stop because there just aren't any db connections left ... and all because the close wasn't handled properly when an exception occurred.

I'm glad you solved your problem. Can you post the code here? Then we can mark the thread as resolved, and anyway I'm curious where the error was.

[Anand Bhatt]

Hi Greg,

The issue was with contentType
Instead of response.setContentType("application/zip"); I read some where in Microsoft site and tried below and isssue disappeared:
response.setContentType("application/octet-stream");
So it is some what specific for IE and I am still clueless about this issue.