Skip to content

Commit

Permalink
feat: add more validation to file upload (#18112) (#18909)
Browse files Browse the repository at this point in the history
  • Loading branch information
@netroms
netroms authored Oct 25, 2024
1 parent 825d92f commit ed0217f
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 36 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 Set<String> CONTENT_TYPES =
private static final Set<String> BLOCKED_CONTENT_TYPES =
Set.of(
// Web
"text/html",
Expand All @@ -54,7 +54,7 @@ public class FileResourceBlocklist {
"application/x-sh",
"application/x-csh");

private static final Set<String> FILE_EXTENSIONS =
private static final Set<String> BLOCKED_FILE_EXTENSIONS =
Set.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
17 changes: 10 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 @@ -101,10 +101,11 @@ void testSaveGoodAvatar() throws IOException {
}

@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");
Expand All @@ -119,10 +120,11 @@ void testSaveOrgUnitImage() {
}

@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 @@ -131,10 +133,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 @@ -144,7 +147,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
10 changes: 10 additions & 0 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 @@ -144,10 +144,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 @@ -174,10 +176,18 @@ public WebMessage saveFileResource(
if (domain.equals(FileResourceDomain.ICON)) {
validateCustomIconFile(file);
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 if (domain.equals(FileResourceDomain.ORG_UNIT)) {
fileResourceUtils.validateOrgUnitImage(file);
fileResource =
fileResourceUtils.saveFileResource(
uid, fileResourceUtils.resizeOrgToDefaultSize(file), domain);

} else {
fileResource = fileResourceUtils.saveFileResource(uid, file, domain);
}
Expand Down
61 changes: 36 additions & 25 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 @@ -93,13 +93,34 @@ public class FileResourceUtils {
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 int ORGUNIT_TARGET_HEIGHT = 800;
private static final int ORGUNIT_TARGET_WIDTH = 800;

private static final List<String> ALLOWED_AVATAR_FILE_EXTENSIONS =
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_AVATAR_MIME_TYPES =
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 @@ -232,31 +253,16 @@ public FileResource saveFileResource(String uid, MultipartFile file, FileResourc
return fileResource;
}

// -------------------------------------------------------------------------
// Inner classes
// -------------------------------------------------------------------------

private 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);
}
}
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(@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);
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, @Nonnull List<String> validExtensions) {
Expand Down Expand Up @@ -342,4 +348,9 @@ public static MultipartFile resizeAvatarToDefaultSize(MultipartFile multipartFil
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 ed0217f

Please sign in to comment.