Skip to content

Commit

Permalink
feat: add more validation to file upload (#18112) (#18814) [DHIS2-17311]
Browse files Browse the repository at this point in the history
  • Loading branch information
@netroms
netroms authored Oct 14, 2024
1 parent ca12913 commit d60981c
Show file tree
Hide file tree
Showing 6 changed files with 207 additions and 23 deletions.
8 changes: 4 additions & 4 deletions ...ces/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/FileResourceBlocklist.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
* @author Lars Helge Overland
*/
public class FileResourceBlocklist {
private static final ImmutableSet<String> CONTENT_TYPES =
private static final ImmutableSet<String> BLOCKED_CONTENT_TYPES =
ImmutableSet.of(
// Web
"text/html",
Expand All @@ -54,7 +54,7 @@ public class FileResourceBlocklist {
"application/x-sh",
"application/x-csh");

private static final ImmutableSet<String> FILE_EXTENSIONS =
private static final ImmutableSet<String> BLOCKED_FILE_EXTENSIONS =
ImmutableSet.of(
// Web
"html",
Expand Down Expand Up @@ -93,11 +93,11 @@ public static boolean isValid(FileResource fileResource) {
return false;
}

if (CONTENT_TYPES.contains(fileResource.getContentType().toLowerCase())) {
if (BLOCKED_CONTENT_TYPES.contains(fileResource.getContentType().toLowerCase())) {
return false;
}

if (FILE_EXTENSIONS.contains(
if (BLOCKED_FILE_EXTENSIONS.contains(
FilenameUtils.getExtension(fileResource.getName().toLowerCase()))) {
return false;
}
Expand Down
21 changes: 14 additions & 7 deletions ...est-web-api/src/test/java/org/hisp/dhis/webapi/controller/FileResourceControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,32 +29,38 @@

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import org.hisp.dhis.feedback.ErrorCode;
import org.hisp.dhis.jsontree.JsonObject;
import org.hisp.dhis.web.HttpStatus;
import org.hisp.dhis.webapi.DhisControllerConvenienceTest;
import org.hisp.dhis.webapi.json.domain.JsonWebMessage;
import org.junit.jupiter.api.Test;
import org.springframework.core.io.ClassPathResource;
import org.springframework.mock.web.MockMultipartFile;

class FileResourceControllerTest extends DhisControllerConvenienceTest {

@Test
void testSaveOrgUnitImage() {
void testSaveOrgUnitImage() throws IOException {
File file = new ClassPathResource("file/dhis2.png").getFile();
MockMultipartFile image =
new MockMultipartFile(
"file", "OU_profile_image.png", "image/png", "<<png data>>".getBytes());
"file", "OU_profile_image.png", "image/png", Files.readAllBytes(file.toPath()));
HttpResponse response = POST_MULTIPART("/fileResources?domain=ORG_UNIT", image);
JsonObject savedObject =
response.content(HttpStatus.ACCEPTED).getObject("response").getObject("fileResource");
assertEquals("OU_profile_image.png", savedObject.getString("name").string());
}

@Test
void testSaveOrgUnitImageWithUid() {
void testSaveOrgUnitImageWithUid() throws IOException {
File file = new ClassPathResource("file/dhis2.png").getFile();
MockMultipartFile image =
new MockMultipartFile(
"file", "OU_profile_image.png", "image/png", "<<png data>>".getBytes());
"file", "OU_profile_image.png", "image/png", Files.readAllBytes(file.toPath()));
HttpResponse response = POST_MULTIPART("/fileResources?domain=ORG_UNIT&uid=0123456789a", image);
JsonObject savedObject =
response.content(HttpStatus.ACCEPTED).getObject("response").getObject("fileResource");
Expand All @@ -63,10 +69,11 @@ void testSaveOrgUnitImageWithUid() {
}

@Test
void testSaveOrgUnitImageWithUid_Update() {
void testSaveOrgUnitImageWithUid_Update() throws IOException {
File file = new ClassPathResource("file/dhis2.png").getFile();
MockMultipartFile image =
new MockMultipartFile(
"file", "OU_profile_image.png", "image/png", "<<png data>>".getBytes());
"file", "OU_profile_image.png", "image/png", Files.readAllBytes(file.toPath()));
HttpResponse response = POST_MULTIPART("/fileResources?domain=ORG_UNIT&uid=0123456789x", image);
JsonObject savedObject =
response.content(HttpStatus.ACCEPTED).getObject("response").getObject("fileResource");
Expand All @@ -76,7 +83,7 @@ void testSaveOrgUnitImageWithUid_Update() {
// now update the resource with a different image but the same UID
MockMultipartFile image2 =
new MockMultipartFile(
"file", "OU_profile_image2.png", "image/png", "<<png data>>".getBytes());
"file", "OU_profile_image2.png", "image/png", Files.readAllBytes(file.toPath()));

JsonWebMessage message =
assertWebMessage(
Expand Down
Binary file added dhis-2/dhis-test-web-api/src/test/resources/file/dhis2.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 6 additions & 0 deletions dhis-2/dhis-web-api/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,12 @@
<scope>provided</scope>
</dependency>

<!-- Image Processing -->
<dependency>
<groupId>org.imgscalr</groupId>
<artifactId>imgscalr-lib</artifactId>
</dependency>

<!-- Test -->
<dependency>
<groupId>org.hamcrest</groupId>
Expand Down
22 changes: 20 additions & 2 deletions ...-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/FileResourceController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.error;
import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.notFound;
import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.unauthorized;
import static org.hisp.dhis.webapi.utils.FileResourceUtils.resizeAvatarToDefaultSize;

import com.google.common.base.MoreObjects;
import java.io.IOException;
Expand Down Expand Up @@ -119,10 +120,12 @@ public void getFileResourceData(
}

response.setContentType(fileResource.getContentType());

response.setHeader(
HttpHeaders.CONTENT_LENGTH,
String.valueOf(fileResourceService.getFileResourceContentLength(fileResource)));
response.setHeader(HttpHeaders.CONTENT_DISPOSITION, "filename=" + fileResource.getName());

HeaderUtils.setSecurityHeaders(
response, dhisConfig.getProperty(ConfigurationKey.CSP_HEADER_VALUE));

Expand All @@ -143,8 +146,23 @@ public WebMessage saveFileResource(
@RequestParam MultipartFile file,
@RequestParam(defaultValue = "DATA_VALUE") FileResourceDomain domain,
@RequestParam(required = false) String uid)
throws WebMessageException, IOException {
FileResource fileResource = fileResourceUtils.saveFileResource(uid, file, domain);
throws IOException, WebMessageException {
FileResource fileResource;

if (domain.equals(FileResourceDomain.USER_AVATAR)) {
fileResourceUtils.validateUserAvatar(file);
fileResource =
fileResourceUtils.saveFileResource(uid, resizeAvatarToDefaultSize(file), domain);

} else if (domain.equals(FileResourceDomain.ORG_UNIT)) {
fileResourceUtils.validateOrgUnitImage(file);
fileResource =
fileResourceUtils.saveFileResource(
uid, fileResourceUtils.resizeOrgToDefaultSize(file), domain);

} else {
fileResource = fileResourceUtils.saveFileResource(uid, file, domain);
}

WebMessage webMessage = new WebMessage(Status.OK, HttpStatus.ACCEPTED);
webMessage.setResponse(new FileResourceWebMessageResponse(fileResource));
Expand Down
173 changes: 163 additions & 10 deletions dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/utils/FileResourceUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,43 +27,95 @@
*/
package org.hisp.dhis.webapi.utils;

import static org.apache.commons.io.FilenameUtils.getExtension;
import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.conflict;
import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.error;
import static org.hisp.dhis.external.conf.ConfigurationKey.CSP_HEADER_VALUE;
import static org.imgscalr.Scalr.resize;

import com.google.common.hash.Hashing;
import com.google.common.io.ByteSource;
import java.awt.image.BufferedImage;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.file.Files;
import java.util.List;
import java.util.Objects;
import javax.imageio.ImageIO;
import javax.servlet.http.HttpServletResponse;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.fileupload.FileItem;
import org.apache.commons.fileupload.disk.DiskFileItemFactory;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.input.NullInputStream;
import org.apache.commons.lang3.StringUtils;
import org.hisp.dhis.common.IllegalQueryException;
import org.hisp.dhis.dxf2.webmessage.WebMessageException;
import org.hisp.dhis.external.conf.DhisConfigurationProvider;
import org.hisp.dhis.feedback.ErrorCode;
import org.hisp.dhis.fileresource.FileResource;
import org.hisp.dhis.fileresource.FileResourceDomain;
import org.hisp.dhis.fileresource.FileResourceService;
import org.hisp.dhis.fileresource.ImageFileDimension;
import org.imgscalr.Scalr.Mode;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpHeaders;
import org.springframework.stereotype.Component;
import org.springframework.util.InvalidMimeTypeException;
import org.springframework.util.MimeTypeUtils;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.multipart.commons.CommonsMultipartFile;

/**
* @author Lars Helge Overland
*/
@Component
@Slf4j
public class FileResourceUtils {

@Autowired private FileResourceService fileResourceService;

private static final List<String> CUSTOM_ICON_VALID_ICON_EXTENSIONS = List.of("png");

private static final long CUSTOM_ICON_FILE_SIZE_LIMIT_IN_BYTES = 25_000_000;

private static final int CUSTOM_ICON_TARGET_HEIGHT = 48;
private static final int CUSTOM_ICON_TARGET_WIDTH = 48;

private static final int AVATAR_TARGET_HEIGHT = 200;
private static final int AVATAR_TARGET_WIDTH = 200;

private static final int ORGUNIT_TARGET_HEIGHT = 800;
private static final int ORGUNIT_TARGET_WIDTH = 800;

private static final long MAX_AVATAR_IMAGE_SIZE_IN_BYTES = 2_000_000;
private static final long MAX_ORGUNIT_IMAGE_SIZE_IN_BYTES = 8_000_000;

private static final List<String> ALLOWED_IMAGE_FILE_EXTENSIONS =
List.of("jpg", "jpeg", "png", "gif");
private static final List<String> ALLOWED_IMAGE_MIME_TYPES =
List.of("image/jpeg", "image/png", "image/gif");

private static class MultipartFileByteSource extends ByteSource {
private MultipartFile file;

public MultipartFileByteSource(MultipartFile file) {
this.file = file;
}

@Override
public InputStream openStream() {
try {
return file.getInputStream();
} catch (IOException ioe) {
return new NullInputStream(0);
}
}
}

/**
* Transfers the given multipart file content to a local temporary file.
*
Expand Down Expand Up @@ -197,20 +249,121 @@ public FileResource saveFileResource(String uid, MultipartFile file, FileResourc
// Inner classes
// -------------------------------------------------------------------------

private class MultipartFileByteSource extends ByteSource {
private MultipartFile file;
// private class MultipartFileByteSource extends ByteSource {
// private MultipartFile file;
//
// public MultipartFileByteSource(MultipartFile file) {
// this.file = file;
// }
//
// @Override
// public InputStream openStream() throws IOException {
// try {
// return file.getInputStream();
// } catch (IOException ioe) {
// return new NullInputStream(0);
// }
// }
// }

public MultipartFileByteSource(MultipartFile file) {
this.file = file;
public void validateOrgUnitImage(MultipartFile file) {
validateContentType(file.getContentType(), ALLOWED_IMAGE_MIME_TYPES);
validateFileExtension(file.getOriginalFilename(), ALLOWED_IMAGE_FILE_EXTENSIONS);
validateFileSize(file, MAX_ORGUNIT_IMAGE_SIZE_IN_BYTES);
}

public void validateUserAvatar(MultipartFile file) {
validateContentType(file.getContentType(), ALLOWED_IMAGE_MIME_TYPES);
validateFileExtension(file.getOriginalFilename(), ALLOWED_IMAGE_FILE_EXTENSIONS);
validateFileSize(file, MAX_AVATAR_IMAGE_SIZE_IN_BYTES);
}

private void validateContentType(String contentType, List<String> validExtensions) {
if (contentType == null) {
throw new IllegalQueryException("Invalid content type, content type is NULL");
}
contentType = contentType.split(";")[0].trim();
if (!validExtensions.contains(contentType)) {
throw new IllegalQueryException(
"Invalid content type, valid content types are: " + String.join(",", validExtensions));
}
}

@Override
public InputStream openStream() throws IOException {
try {
return file.getInputStream();
} catch (IOException ioe) {
return new NullInputStream(0);
public static void validateCustomIconFile(MultipartFile file) {
validateFileExtension(file.getOriginalFilename(), CUSTOM_ICON_VALID_ICON_EXTENSIONS);
validateFileSize(file, CUSTOM_ICON_FILE_SIZE_LIMIT_IN_BYTES);
}

private static void validateFileExtension(String fileName, List<String> validExtension) {
if (getExtension(fileName) == null || !validExtension.contains(getExtension(fileName))) {
throw new IllegalQueryException(
"Wrong file extension, valid extensions are: " + String.join(",", validExtension));
}
}

private static void validateFileSize(MultipartFile file, long maxFileSizeInBytes) {
if (file.getSize() > maxFileSizeInBytes) {
throw new IllegalQueryException(
String.format(
"File size can't be bigger than %d, current file size %d",
maxFileSizeInBytes, file.getSize()));
}
}

public static MultipartFile resizeImageToCustomSize(
MultipartFile multipartFile, int targetWidth, int targetHeight, Mode resizeMode)
throws IOException {
File tmpFile = null;

try {
BufferedImage resizedImage =
resize(
ImageIO.read(multipartFile.getInputStream()), resizeMode, targetWidth, targetHeight);

tmpFile = Files.createTempFile("org.hisp.dhis", ".tmp").toFile();

ImageIO.write(
resizedImage,
Objects.requireNonNull(getExtension(multipartFile.getOriginalFilename())),
tmpFile);

FileItem fileItem =
new DiskFileItemFactory()
.createItem(
"file",
Files.probeContentType(tmpFile.toPath()),
false,
multipartFile.getOriginalFilename());

try (InputStream in = new FileInputStream(tmpFile);
OutputStream out = fileItem.getOutputStream()) {
in.transferTo(out);
}

return new CommonsMultipartFile(fileItem);
} catch (Exception e) {
throw new IOException("Failed to resize image: " + e.getMessage());
} finally {
if (tmpFile != null && tmpFile.exists()) {
Files.delete(tmpFile.toPath());
}
}
}

public static MultipartFile resizeIconToDefaultSize(MultipartFile multipartFile)
throws IOException {
return resizeImageToCustomSize(
multipartFile, CUSTOM_ICON_TARGET_WIDTH, CUSTOM_ICON_TARGET_HEIGHT, Mode.FIT_EXACT);
}

public static MultipartFile resizeAvatarToDefaultSize(MultipartFile multipartFile)
throws IOException {
return resizeImageToCustomSize(
multipartFile, AVATAR_TARGET_WIDTH, AVATAR_TARGET_HEIGHT, Mode.AUTOMATIC);
}

public MultipartFile resizeOrgToDefaultSize(MultipartFile multipartFile) throws IOException {
return resizeImageToCustomSize(
multipartFile, ORGUNIT_TARGET_WIDTH, ORGUNIT_TARGET_HEIGHT, Mode.AUTOMATIC);
}
}

0 comments on commit d60981c

Please sign in to comment.