BREAKING: Refactors file validation

terrestris · May 5, 2021 · 051a311 · 051a311
1 parent feb4163
commit 051a311
Show file tree

Hide file tree

Showing 7 changed files with 115 additions and 86 deletions.
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/controller/BaseFileController.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/controller/BaseFileController.java
@@ -33,14 +33,10 @@
 import org.springframework.web.multipart.MultipartFile;
 import org.springframework.web.server.ResponseStatusException;
 
-import java.io.ByteArrayInputStream;
-import java.io.InputStream;
 import java.util.List;
 import java.util.Optional;
 import java.util.UUID;
 
-import static de.terrestris.shogun.lib.util.FileUtil.convertToFile;
-
 public abstract class BaseFileController<T extends BaseFileService<?, S>, S extends File> {
 
     protected final Logger LOG = LogManager.getLogger(getClass());
@@ -175,7 +171,7 @@ public S add(MultipartFile uploadedFile) {
 
         try {
 
-            service.isValidType(uploadedFile.getContentType());
+            service.isValid(uploadedFile);
 
             S persistedFile = service.create(uploadedFile);
 
@@ -218,7 +214,7 @@ public S addToFileSystem(MultipartFile uploadedFile) {
 
         try {
 
-            service.isValidType(uploadedFile.getContentType());
+            service.isValid(uploadedFile);
 
             S persistedFile = service.create(uploadedFile, true);
 

diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/service/BaseFileService.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/service/BaseFileService.java
@@ -18,20 +18,66 @@
 
 import de.terrestris.shogun.lib.model.File;
 import de.terrestris.shogun.lib.repository.BaseFileRepository;
+import de.terrestris.shogun.properties.UploadProperties;
+import org.apache.commons.io.FileUtils;
+import org.apache.tika.config.TikaConfig;
+import org.apache.tika.exception.TikaException;
+import org.apache.tika.io.TikaInputStream;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.mime.MediaType;
+import org.apache.tomcat.util.http.fileupload.impl.InvalidContentTypeException;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
 import org.springframework.security.access.prepost.PostAuthorize;
+import org.springframework.util.PatternMatchUtils;
 import org.springframework.web.multipart.MultipartFile;
 
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.List;
 import java.util.Optional;
 import java.util.UUID;
 
 public abstract class BaseFileService<T extends BaseFileRepository<S, Long> & JpaSpecificationExecutor<S>, S extends File> extends BaseService<T, S> implements IBaseFileService<T, S> {
 
+    @Autowired
+    private UploadProperties uploadProperties;
+
     @PostAuthorize("hasRole('ROLE_ADMIN') or hasPermission(returnObject.orElse(null), 'READ')")
     public Optional<S> findOne(UUID fileUuid) {
         return repository.findByFileUuid(fileUuid);
     }
 
     public abstract S create(MultipartFile uploadFile, Boolean writeToSystem) throws Exception;
 
+    public void isValid(MultipartFile file) throws Exception {
+        if (file == null) {
+            throw new Exception("Given file is null.");
+        } else if (file.isEmpty()) {
+            throw new Exception("Given file is empty.");
+        }
+        this.isValidType(file.getContentType());
+        this.verifyContentType(file);
+    }
+
+    public void verifyContentType(MultipartFile file) throws IOException, TikaException {
+        String contentType = file.getContentType();
+        String name = file.getName();
+        Metadata metadata = new Metadata();
+        metadata.set(Metadata.RESOURCE_NAME_KEY, name);
+        TikaConfig tika = new TikaConfig();
+        MediaType mediaType = tika.getDetector().detect(TikaInputStream.get(file.getBytes()), metadata);
+        if (!mediaType.toString().equals(contentType)) {
+            throw new IOException("Mediatype validation failed. Passed content type is " + contentType + " but detected mediatype is " + mediaType);
+        }
+    }
+
+    public void isValidType(String contentType) throws InvalidContentTypeException {
+        List<String> supportedContentTypes = getSupportedContentTypes();
+        boolean isMatch = PatternMatchUtils.simpleMatch(supportedContentTypes.toArray(new String[supportedContentTypes.size()]), contentType);
+        if (!isMatch) {
+            throw new InvalidContentTypeException("Unsupported content type for upload!");
+        }
+    }
+
 }
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/service/FileService.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/service/FileService.java
@@ -23,15 +23,14 @@
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.tomcat.util.http.fileupload.impl.InvalidContentTypeException;
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import org.springframework.util.PatternMatchUtils;
 import org.springframework.web.multipart.MultipartFile;
 
-import java.io.ByteArrayInputStream;
-import java.io.FileOutputStream;
-import java.io.InputStream;
-import java.io.OutputStream;
+import java.io.*;
 import java.util.List;
 import java.util.UUID;
 
@@ -51,8 +50,6 @@ public class FileService extends BaseFileService<FileRepository, File> {
      */
     public File create(MultipartFile uploadFile) throws Exception {
 
-        FileUtil.validateFile(uploadFile);
-
         byte[] fileByteArray = FileUtil.fileToByteArray(uploadFile);
 
         File file = new File();
@@ -89,7 +86,6 @@ public File create(MultipartFile uploadFile, Boolean writeToSystem) throws Excep
             throw new Exception("Could not upload file. fileName is null.");
         }
 
-        FileUtil.validateFile(uploadFile);
         File file = new File();
         file.setFileType(uploadFile.getContentType());
         file.setFileName(fileName);
@@ -125,33 +121,18 @@ public File create(MultipartFile uploadFile, Boolean writeToSystem) throws Excep
         return this.repository.save(savedFile);
     }
 
-    /**
-     *
-     * @param contentType
-     * @throws InvalidContentTypeException
-     */
-    public void isValidType(String contentType) throws InvalidContentTypeException {
+    @Override
+    public List<String> getSupportedContentTypes() {
         if (uploadProperties == null) {
-            throw new InvalidContentTypeException("No properties for the upload found. " +
-                "Please check your application.yml");
+            throw new NoSuchBeanDefinitionException("No properties for the upload found. Please check your application.yml");
         }
-
         if (uploadProperties.getFile() == null) {
-            throw new InvalidContentTypeException("No properties for the file upload found. " +
-                "Please check your application.yml");
+            throw new NoSuchBeanDefinitionException("No properties for the file upload found. Please check your application.yml");
         }
-
         if (uploadProperties.getFile().getSupportedContentTypes() == null) {
-            throw new InvalidContentTypeException("No list of supported content types for the file upload found. " +
+            throw new NoSuchBeanDefinitionException("No list of supported content types for the file upload found. " +
                 "Please check your application.yml");
         }
-
-        List<String> supportedContentTypes = uploadProperties.getFile().getSupportedContentTypes();
-
-        boolean isMatch = PatternMatchUtils.simpleMatch(supportedContentTypes.toArray(new String[supportedContentTypes.size()]), contentType);
-
-        if (!isMatch) {
-            throw new InvalidContentTypeException("Unsupported content type for upload!");
-        }
+        return uploadProperties.getFile().getSupportedContentTypes();
     }
 }
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/service/IBaseFileService.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/service/IBaseFileService.java
@@ -18,10 +18,14 @@
 
 import de.terrestris.shogun.lib.model.File;
 import de.terrestris.shogun.lib.repository.BaseFileRepository;
+import org.apache.tika.exception.TikaException;
 import org.apache.tomcat.util.http.fileupload.impl.InvalidContentTypeException;
+import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
 import org.springframework.web.multipart.MultipartFile;
 
+import java.io.IOException;
+import java.util.List;
 import java.util.Optional;
 import java.util.UUID;
 
@@ -31,5 +35,36 @@ public interface IBaseFileService<T extends BaseFileRepository<S, Long> & JpaSpe
 
     S create(MultipartFile uploadFile) throws Exception;
 
+    /**
+     * Verifies if the content type of the file matches its content.
+     *
+     * @param file
+     * @throws IOException
+     * @throws TikaException
+     */
+    void verifyContentType(MultipartFile file) throws IOException, TikaException;
+
+    /**
+     * Checks if the given contentType is included in the content type whitelist configured via UploadProperties.
+     *
+     * @param contentType
+     * @throws InvalidContentTypeException
+     */
     void isValidType(String contentType) throws InvalidContentTypeException;
+
+    /**
+     * Checks if the file is not null or empty and has a valid content type.
+     *
+     * @param file
+     * @throws Exception
+     */
+    void isValid(MultipartFile file) throws Exception;
+
+    /**
+     * Get the list of supported content types. Should be implement by real class.
+     *
+     * @return
+     * @throws NoSuchBeanDefinitionException
+     */
+    List<String> getSupportedContentTypes() throws NoSuchBeanDefinitionException;
 }
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/service/ImageFileService.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/service/ImageFileService.java
@@ -23,17 +23,20 @@
 import de.terrestris.shogun.properties.UploadProperties;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.tika.config.TikaConfig;
+import org.apache.tika.exception.TikaException;
+import org.apache.tika.io.TikaInputStream;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.mime.MediaType;
 import org.apache.tomcat.util.http.fileupload.impl.InvalidContentTypeException;
+import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import org.springframework.util.PatternMatchUtils;
 import org.springframework.web.multipart.MultipartFile;
 
 import java.awt.*;
-import java.io.ByteArrayInputStream;
-import java.io.FileOutputStream;
-import java.io.InputStream;
-import java.io.OutputStream;
+import java.io.*;
 import java.util.List;
 import java.util.UUID;
 
@@ -45,8 +48,6 @@ public class ImageFileService extends BaseFileService<ImageFileRepository, Image
 
     public ImageFile create(MultipartFile uploadFile) throws Exception {
 
-        FileUtil.validateFile(uploadFile);
-
         byte[] fileByteArray = FileUtil.fileToByteArray(uploadFile);
 
         ImageFile file = new ImageFile();
@@ -71,31 +72,6 @@ public ImageFile create(MultipartFile uploadFile) throws Exception {
         return savedFile;
     }
 
-    public void isValidType(String contentType) throws InvalidContentTypeException {
-        if (uploadProperties == null) {
-            throw new InvalidContentTypeException("No properties for the upload found. " +
-                "Please check your application.yml");
-        }
-
-        if (uploadProperties.getImage() == null) {
-            throw new InvalidContentTypeException("No properties for the image file upload found. " +
-                "Please check your application.yml");
-        }
-
-        if (uploadProperties.getImage().getSupportedContentTypes() == null) {
-            throw new InvalidContentTypeException("No list of supported content types for the image file upload found. " +
-                "Please check your application.yml");
-        }
-
-        List<String> supportedContentTypes = uploadProperties.getImage().getSupportedContentTypes();
-
-        boolean isMatch = PatternMatchUtils.simpleMatch(supportedContentTypes.toArray(new String[supportedContentTypes.size()]), contentType);
-
-        if (!isMatch) {
-            throw new InvalidContentTypeException("Unsupported content type for upload!");
-        }
-    }
-
     @Override
     public ImageFile create(MultipartFile uploadFile, Boolean writeToSystem) throws Exception {
         if (!writeToSystem) {
@@ -111,7 +87,6 @@ public ImageFile create(MultipartFile uploadFile, Boolean writeToSystem) throws
             throw new Exception("Could not upload file. fileName is null.");
         }
 
-        FileUtil.validateFile(uploadFile);
         byte[] fileByteArray = FileUtil.fileToByteArray(uploadFile);
         ImageFile file = new ImageFile();
         file.setFileType(uploadFile.getContentType());
@@ -156,4 +131,18 @@ public ImageFile create(MultipartFile uploadFile, Boolean writeToSystem) throws
         savedFile.setPath(path);
         return this.repository.save(savedFile);
     }
+
+    public List<String> getSupportedContentTypes() {
+        if (uploadProperties == null) {
+            throw new NoSuchBeanDefinitionException("No properties for the upload found. Please check your application.yml");
+        }
+        if (uploadProperties.getImage() == null) {
+            throw new NoSuchBeanDefinitionException("No properties for the imagefile upload found. Please check your application.yml");
+        }
+        if (uploadProperties.getImage().getSupportedContentTypes() == null) {
+            throw new NoSuchBeanDefinitionException("No list of supported content types for the imagefile upload found. " +
+                "Please check your application.yml");
+        }
+        return uploadProperties.getImage().getSupportedContentTypes();
+    }
 }
diff --git a/shogun-lib/src/main/java/de/terrestris/shogun/lib/util/FileUtil.java b/shogun-lib/src/main/java/de/terrestris/shogun/lib/util/FileUtil.java
@@ -67,24 +67,4 @@ public static File convertToFile(MultipartFile multipartFile) throws IOException
         return file;
     }
 
-    public static void validateFile(MultipartFile file) throws Exception {
-        if (file == null) {
-            throw new Exception("Given file is null.");
-        } else if (file.isEmpty()) {
-            throw new Exception("Given file is empty.");
-        }
-
-        String name = file.getName();
-        String contentType = file.getContentType();
-        Metadata metadata = new Metadata();
-        metadata.set(Metadata.RESOURCE_NAME_KEY, name);
-        TikaConfig tika = new TikaConfig();
-        MediaType mediaType = tika.getDetector().detect(TikaInputStream.get(file.getBytes()), metadata);
-
-        if (!mediaType.toString().equals(contentType)) {
-            throw new Exception("Mediatype validation failed. Passed content type is " + contentType + " but detected mediatype is " + mediaType);
-        }
-
-    }
-
 }
diff --git a/shogun-lib/src/test/java/de/terrestris/shogun/lib/service/FileServiceTest.java b/shogun-lib/src/test/java/de/terrestris/shogun/lib/service/FileServiceTest.java
@@ -25,6 +25,7 @@
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 
+import java.io.File;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -53,6 +54,7 @@ public void initializeConfig(List<String> supportedContentTypes) {
     @Test
     public void isValidType_failsIfNoConfigIsGiven() {
         // initializeConfig hasn't been called.
+        File mockFile = new File("/");
         assertThrows(InvalidContentTypeException.class, () -> fileService.isValidType("application/zip"));
     }