Skip to content

Commit

Permalink
feat: validate user avatar uploads (#18096)
Browse files Browse the repository at this point in the history
* feat: validate user avatar uploads

(cherry picked from commit b9ebacb)
  • Loading branch information
netroms committed Oct 14, 2024
1 parent 65c4873 commit ea818e6
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ public void asyncSaveFileResource(FileResource fileResource, File file) {
@Override
@Transactional
public String asyncSaveFileResource(FileResource fileResource, byte[] bytes) {
validateFileResource(fileResource);

fileResource.setStorageStatus(FileResourceStorageStatus.PENDING);
fileResourceStore.save(fileResource);
entityManager.flush();
Expand All @@ -220,6 +222,8 @@ public String asyncSaveFileResource(FileResource fileResource, byte[] bytes) {
@Transactional
public String syncSaveFileResource(FileResource fileResource, byte[] bytes)
throws ConflictException {
validateFileResource(fileResource);

fileResource.setContentLength(bytes.length);
try {
fileResource.setContentMd5(ByteSource.wrap(bytes).hash(Hashing.md5()).toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,77 @@

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.jsontree.JsonString;
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 testSaveBadAvatarImageData() {
MockMultipartFile image =
new MockMultipartFile(
"file", "OU_profile_image.png", "image/png", "<<png data>>".getBytes());
HttpResponse response = POST_MULTIPART("/fileResources?domain=USER_AVATAR", image);
JsonString errorMessage =
response.content(HttpStatus.INTERNAL_SERVER_ERROR).getString("message");
assertEquals("Failed to resize image: src cannot be null", errorMessage.string());
}

@Test
void testSaveBadAvatarContentType() {
MockMultipartFile image =
new MockMultipartFile(
"file", "OU_profile_image.png", "image/tiff", "<<png data>>".getBytes());
HttpResponse response = POST_MULTIPART("/fileResources?domain=USER_AVATAR", image);
JsonString errorMessage = response.content(HttpStatus.CONFLICT).getString("message");
assertEquals(
"Invalid content type, valid content types are: image/jpeg,image/png,image/gif",
errorMessage.string());
}

@Test
void testSaveBadAvatarFileExtension() {
MockMultipartFile image =
new MockMultipartFile(
"file", "OU_profile_image.tiff", "image/png", "<<png data>>".getBytes());
HttpResponse response = POST_MULTIPART("/fileResources?domain=USER_AVATAR", image);
JsonString errorMessage = response.content(HttpStatus.CONFLICT).getString("message");
assertEquals(
"Wrong file extension, valid extensions are: jpg,jpeg,png,gif", errorMessage.string());
}

@Test
void testSaveBadAvatarFileSize() {
byte[] bytes = new byte[2_000_001];
MockMultipartFile image =
new MockMultipartFile("file", "OU_profile_image.png", "image/png", bytes);
HttpResponse response = POST_MULTIPART("/fileResources?domain=USER_AVATAR", image);
JsonString errorMessage = response.content(HttpStatus.CONFLICT).getString("message");
assertEquals(
"File size can't be bigger than 2000000, current file size 2000001", errorMessage.string());
}

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

@Test
void testSaveOrgUnitImage() {
MockMultipartFile image =
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
package org.hisp.dhis.webapi.controller;

import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.error;
import static org.hisp.dhis.webapi.utils.FileResourceUtils.resizeToDefaultIconSize;
import static org.hisp.dhis.webapi.utils.FileResourceUtils.resizeAvatarToDefaultSize;
import static org.hisp.dhis.webapi.utils.FileResourceUtils.resizeIconToDefaultSize;
import static org.hisp.dhis.webapi.utils.FileResourceUtils.validateCustomIconFile;

import com.google.common.base.MoreObjects;
Expand Down Expand Up @@ -169,9 +170,14 @@ public WebMessage saveFileResource(
@RequestParam(required = false) String uid)
throws IOException, ConflictException {
FileResource fileResource;

if (domain.equals(FileResourceDomain.ICON)) {
validateCustomIconFile(file);
fileResource = fileResourceUtils.saveFileResource(uid, resizeToDefaultIconSize(file), domain);
fileResource = fileResourceUtils.saveFileResource(uid, resizeIconToDefaultSize(file), domain);
} else if (domain.equals(FileResourceDomain.USER_AVATAR)) {
fileResourceUtils.validateUserAvatar(file);
fileResource =
fileResourceUtils.saveFileResource(uid, resizeAvatarToDefaultSize(file), domain);
} else {
fileResource = fileResourceUtils.saveFileResource(uid, file, domain);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.nio.file.Files;
import java.util.List;
import java.util.Objects;
import javax.annotation.Nonnull;
import javax.imageio.ImageIO;
import javax.servlet.http.HttpServletResponse;
import lombok.RequiredArgsConstructor;
Expand All @@ -62,7 +63,7 @@
import org.hisp.dhis.fileresource.FileResourceService;
import org.hisp.dhis.fileresource.ImageFileDimension;
import org.hisp.dhis.scheduling.JobRunner;
import org.imgscalr.Scalr;
import org.imgscalr.Scalr.Mode;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpHeaders;
import org.springframework.stereotype.Component;
Expand All @@ -87,9 +88,18 @@ public class FileResourceUtils {
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 long MAX_AVATAR_FILE_SIZE_IN_BYTES = 2_000_000;

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

/**
* Transfers the given multipart file content to a local temporary file.
*
Expand Down Expand Up @@ -243,40 +253,54 @@ public InputStream openStream() {
}
}

public void validateUserAvatar(@Nonnull MultipartFile file) {
validateContentType(file.getContentType(), ALLOWED_AVATAR_MIME_TYPES);
validateFileExtension(file.getOriginalFilename(), ALLOWED_AVATAR_FILE_EXTENSIONS);
validateFileSize(file, MAX_AVATAR_FILE_SIZE_IN_BYTES);
}

private void validateContentType(String contentType, @Nonnull 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));
}
}

public static void validateCustomIconFile(MultipartFile file) {
validateFileExtension(file.getOriginalFilename());
validateFileSize(file);
validateFileExtension(file.getOriginalFilename(), CUSTOM_ICON_VALID_ICON_EXTENSIONS);
validateFileSize(file, CUSTOM_ICON_FILE_SIZE_LIMIT_IN_BYTES);
}

private static void validateFileExtension(String fileName) {
if (getExtension(fileName) == null
|| !CUSTOM_ICON_VALID_ICON_EXTENSIONS.contains(getExtension(fileName))) {
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(",", CUSTOM_ICON_VALID_ICON_EXTENSIONS));
"Wrong file extension, valid extensions are: " + String.join(",", validExtension));
}
}

private static void validateFileSize(MultipartFile file) {
if (file.getSize() > CUSTOM_ICON_FILE_SIZE_LIMIT_IN_BYTES) {
private static void validateFileSize(@Nonnull 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",
CUSTOM_ICON_FILE_SIZE_LIMIT_IN_BYTES, file.getSize()));
maxFileSizeInBytes, file.getSize()));
}
}

public static MultipartFile resizeToDefaultIconSize(MultipartFile multipartFile)
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()),
Scalr.Mode.FIT_EXACT,
CUSTOM_ICON_TARGET_WIDTH,
CUSTOM_ICON_TARGET_HEIGHT);
ImageIO.read(multipartFile.getInputStream()), resizeMode, targetWidth, targetHeight);

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

ImageIO.write(
Expand All @@ -298,12 +322,24 @@ public static MultipartFile resizeToDefaultIconSize(MultipartFile multipartFile)
}

return new CommonsMultipartFile(fileItem);
} catch (IOException e) {
throw new IOException("Failed to resize image", e);
} 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ void shouldResizeImageTo48x48WhenCustomIconIsValid() throws IOException {
InputStream in = getClass().getResourceAsStream("/icon/test-image.png");
MockMultipartFile mockMultipartFile =
new MockMultipartFile("file", "test-image.png", "image/png", in);
MultipartFile file = FileResourceUtils.resizeToDefaultIconSize(mockMultipartFile);
MultipartFile file = FileResourceUtils.resizeIconToDefaultSize(mockMultipartFile);
BufferedImage bufferedImage = ImageIO.read(file.getInputStream());

Assertions.assertEquals(48, bufferedImage.getWidth());
Expand Down

0 comments on commit ea818e6

Please sign in to comment.