Servlet to let a user download a file

Question

Is the code below an efficient way to get the zip file from the server? The code works fine but I'm wondering if I should throw some exceptions for the following conditions:

1. if(param1 == null || param2 == null)

2. } else if ("".equals(param1) || "".equals(param2)) {

The way I call this servlet from a web browser is as follows:

https://myserver.com/DownloadFileFromServer/DownloadFileServlet?filename=my_users_files_1560866090.zip&user=TAN

Eventually, I'll call the above URL from my user interface when a user would have clicked a Download button, maybe using Javascript's setTimeout() method. The reason I plan on calling it again and again for sometime is because the file might not be available on the server and I would have to keep on checkiing it again and again. Do I need to handle this part in my servlet code somehow? I mean, I would keep on callling the above URL until I get the file from the server.

@WebServlet("/DownloadFileServlet")
public class DownloadFileServlet extends HttpServlet {

    final String[][] contentTypes = {{"zip" ,"application/zip"},{"csv" ,"text/csv"},{"pdf","application/pdf"}};
    private static final long serialVersionUID = 1L;


    protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {


        String param1 = request.getParameter("filename");
        String param2 = request.getParameter("user");

        System.out.println("File Name Retrieved is:"+param1);
        System.out.println("User Name Retrieved is:"+param2);

        final String FILE_LOCATION = "/srv/my_users/param2/";


        if(param1 == null || param2 == null) {
            // The Request Parameters Were Not Present In The Query String. Do Something Or Exception Handling !!
            System.out.println("Request Parameter Not Found in first if!");
        } else if ("".equals(param1) || "".equals(param2)) {
            // The Request Parameters Were Present In The Query String But Has No Value. Do Something Or  Exception Handling !!
            System.out.println("Request Parameter Not Found in second if!");
        } else {

        /*if (param1 != null && param2 !=null) {*/
            String fileName = (String) param1;
            String contentType = getContentType(fileName.split("\\.")[1]);
            File file = new File(FILE_LOCATION + "/" +fileName);
            response.setContentType(contentType);
            response.addHeader("Content-Disposition","attachment; filename=" + fileName);
            response.setContentLength((int) file.length());
            ServletOutputStream servletOutputStream = response.getOutputStream();
            BufferedInputStream bufferedInputStream = new BufferedInputStream(new FileInputStream(file));
            int bytesRead = bufferedInputStream.read();
            while (bytesRead != -1) {
                servletOutputStream.write(bytesRead);
                bytesRead = bufferedInputStream.read();
            }
            if (servletOutputStream != null) servletOutputStream.close();
            if(bufferedInputStream != null) bufferedInputStream.close();

        }

        //response.getWriter().append("Served at: ").append(request.getContextPath());
    }


    private String getContentType(String fileType) {
        String returnType = null;
        for(int i = 0; i < contentTypes.length; i++) {
            if(fileType.equals(contentTypes[i][0])) returnType = contentTypes[i][1];
        }
        return returnType;
    }

}

Answer 1

If the parameters are required then they need to be checked and their invalidity must be reported to the caller. There is a HTTP status code for it: 400/BAD REQUEST.

The file transfer is done one byte at a time. This is very inefficient. This kind of transer is usually done in 4096 byte blocks. There are libraries for this task too. For example Apache Commons IO IOUtil.copy(InputStream, OutputStream).

The content type mappings are inefficient. A Map would be a natural fit for the use case instead of a 2D array. But this is something you don't need to do yourself: https://stackoverflow.com/questions/51438/getting-a-files-mime-type-in-java

Calling the servlet repeatedly is called polling. A polling service should be quick and lightweight to not cause excessive load. The client should implement waiting between the polling. The servlet doesn't really need to know about the polling interval. If the servlet should implement throttling of "too eager clients", then you need to implement authentication to identify clients. Depending on the infrastructure you have, you might want to use a firewall or an API gateway for throttling.

By creating the file path by concatenating input from a URL parameter to a path constant you allow the caller to add directories to the path name. E.g. a file name "../../some/sensitive/file.txt" might reveal data you want to keep hidden. It is safer to get a file list from the directory with File.list(FileNameFilter) and use the filter find the file that matches the requested file. Other approach is to sanitize the input, but in mny opinion this is a much more robust solution. If there are a lot of files in the directory and there are a lot of requiests, you might want to cache the file list for a few seconds.

Closing the servlet output stream may have unwanted side effects, so you should just leave it to the servlet container.