Reviewing the file download controller in spring rest

Question

In my project, so this controller is working perfectly, my objective is to read the log file from the server which is created on daily basis, so I have to read the log file which is heavier in size around maybe 2GB, so my question is that please review the below code and advise my is my code is smart enough to read such heavy log file, or should I need to do some modifications in it to read such file in chunks and also look if any modifications I need to introduce into from performance point of view or exceptional handling point of view and from REST practices point of view

@GetMapping("/download")
@ResponseStatus(HttpStatus.OK)
public ResponseEntity<Resource> download(@RequestParam String filename) {
    File file = new File(serverLocation + File.separator + filename + EXTENSION);

    HttpHeaders header = new HttpHeaders();
    header.add(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=" + filename + EXTENSION);
    header.add("Cache-Control", "no-cache, no-store, must-revalidate");
    header.add("Pragma", "no-cache");
    header.add("Expires", "0");

    Path path = get(file.getAbsolutePath());
    Resource resource = null;
    try {
        resource = new InputStreamResource(Files.newInputStream(path));
    } catch (IOException e) {
        LOGGER.error(String.format("Exception occurred while reading the log file%s", e));
    }
    return ResponseEntity.ok()
            .headers(header)            .contentLength(file.length())
            .contentType(MediaType.parseMediaType("application/octet-stream"))
            .body(resource);

}

Answer 1

In my opinion, there are two ways with Spring to do that; your way is one of them (using the Spring Resource), and the other is to inject the file bytes directly in the javax.servlet.http.HttpServletResponse.

Your version can be improved, since you can use the FileSystemResource instead of the InputStreamResource.

The InputStreamResource is good when there are no other options available, as stated in the Java documentation since there are lots of other implementations to use.

Should only be used if no specific Resource implementation is applicable[...]

The FileSystemResource will remove the need to open the file and make the code shorter.

HttpHeaders header = new HttpHeaders();
header.add(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=" + filename + EXTENSION);
header.add("Cache-Control", "no-cache, no-store, must-revalidate");
header.add("Pragma", "no-cache");
header.add("Expires", "0");

File file = new File(serverLocation + File.separator + filename + EXTENSION);
Resource resource = new FileSystemResource(file);

return ResponseEntity.ok()
.headers(header)
.contentLength(file.length())
.contentType(MediaType.parseMediaType("application/octet-stream"))
.body(resource);

The thing that concerns me with your code, is the potential Path Traversal vulnerability that you have when reading the file, from the parameter. If I pass the filename value of ..\..\..\system.log, this could potentially be a problem, because we could download file out of the scope of your application.

File file = new File(serverLocation + File.separator + filename + EXTENSION);

One way to prevent that is to compare the base path / location of the file with the serverLocation, if the path is different, you ignore the request.