spring-guides / gs-uploading-files

Uploading Files :: Learn how to build a Spring application that accepts multi-part file uploads.
http://spring.io/guides/gs/uploading-files/
Apache License 2.0
469 stars 503 forks source link

Leak in 'complete' FileSystemStorageService.java ( #43

Closed kevinmichaelchen closed 6 years ago

kevinmichaelchen commented 6 years ago

https://github.com/spring-guides/gs-uploading-files/blob/master/complete/src/main/java/hello/storage/FileSystemStorageService.java

I believe we need to close the InputStream.

Instead of

 @Override
    public void store(MultipartFile file) {
        String filename = StringUtils.cleanPath(file.getOriginalFilename());
        try {
            if (file.isEmpty()) {
                throw new StorageException("Failed to store empty file " + filename);
            }
            if (filename.contains("..")) {
                // This is a security check
                throw new StorageException(
                        "Cannot store file with relative path outside current directory "
                                + filename);
            }
            Files.copy(file.getInputStream(), this.rootLocation.resolve(filename),
                    StandardCopyOption.REPLACE_EXISTING);
        }
        catch (IOException e) {
            throw new StorageException("Failed to store file " + filename, e);
        }
}

it should be

 @Override
    public void store(MultipartFile file) {
        String filename = StringUtils.cleanPath(file.getOriginalFilename());
        InputStream inputStream = null;
        try {
            if (file.isEmpty()) {
                throw new StorageException("Failed to store empty file " + filename);
            }
            if (filename.contains("..")) {
                // This is a security check
                throw new StorageException(
                        "Cannot store file with relative path outside current directory "
                                + filename);
            }
            inputStream = file.getInputStream();
            Files.copy(inputStream, this.rootLocation.resolve(filename),
                    StandardCopyOption.REPLACE_EXISTING);
        }
        catch (IOException e) {
            throw new StorageException("Failed to store file " + filename, e);
        }
        finally {
            if (inputStream != null) inputStream.close();
        }
}