diff --git a/genie-web/src/main/java/com/netflix/genie/web/apis/rest/v3/controllers/GenieExceptionMapper.java b/genie-web/src/main/java/com/netflix/genie/web/apis/rest/v3/controllers/GenieExceptionMapper.java index f3a84c89c0c..d02094c561b 100644 --- a/genie-web/src/main/java/com/netflix/genie/web/apis/rest/v3/controllers/GenieExceptionMapper.java +++ b/genie-web/src/main/java/com/netflix/genie/web/apis/rest/v3/controllers/GenieExceptionMapper.java @@ -30,6 +30,7 @@ import com.netflix.genie.common.internal.exceptions.unchecked.GenieJobNotFoundException; import com.netflix.genie.common.internal.exceptions.unchecked.GenieJobSpecificationNotFoundException; import com.netflix.genie.common.internal.exceptions.unchecked.GenieRuntimeException; +import com.netflix.genie.web.exceptions.checked.IllegalAttachmentFileNameException; import com.netflix.genie.web.exceptions.checked.AttachmentTooLargeException; import com.netflix.genie.web.exceptions.checked.IdAlreadyExistsException; import com.netflix.genie.web.exceptions.checked.JobNotFoundException; @@ -138,6 +139,8 @@ public ResponseEntity handleGenieCheckedException(final G return new ResponseEntity<>(e, HttpStatus.BAD_REQUEST); } else if (e instanceof AttachmentTooLargeException) { return new ResponseEntity<>(e, HttpStatus.PAYLOAD_TOO_LARGE); + } else if (e instanceof IllegalAttachmentFileNameException) { + return new ResponseEntity<>(e, HttpStatus.BAD_REQUEST); } else { return new ResponseEntity<>(e, HttpStatus.INTERNAL_SERVER_ERROR); } diff --git a/genie-web/src/main/java/com/netflix/genie/web/exceptions/checked/IllegalAttachmentFileNameException.java b/genie-web/src/main/java/com/netflix/genie/web/exceptions/checked/IllegalAttachmentFileNameException.java new file mode 100644 index 00000000000..93e6fe9fe92 --- /dev/null +++ b/genie-web/src/main/java/com/netflix/genie/web/exceptions/checked/IllegalAttachmentFileNameException.java @@ -0,0 +1,43 @@ +package com.netflix.genie.web.exceptions.checked; + +/** + * Exception thrown when the attachment filename is illegal. + * + * @author bhou + */ +public class IllegalAttachmentFileNameException extends SaveAttachmentException { + /** + * Constructor. + */ + public IllegalAttachmentFileNameException() { + super(); + } + + /** + * Constructor. + * + * @param message The detail message + */ + public IllegalAttachmentFileNameException(final String message) { + super(message); + } + + /** + * Constructor. + * + * @param message The detail message + * @param cause The root cause of this exception + */ + public IllegalAttachmentFileNameException(final String message, final Throwable cause) { + super(message, cause); + } + + /** + * Constructor. + * + * @param cause The root cause of this exception + */ + public IllegalAttachmentFileNameException(final Throwable cause) { + super(cause); + } +} diff --git a/genie-web/src/main/java/com/netflix/genie/web/services/impl/LocalFileSystemAttachmentServiceImpl.java b/genie-web/src/main/java/com/netflix/genie/web/services/impl/LocalFileSystemAttachmentServiceImpl.java index 19301375871..2e9af11dce6 100644 --- a/genie-web/src/main/java/com/netflix/genie/web/services/impl/LocalFileSystemAttachmentServiceImpl.java +++ b/genie-web/src/main/java/com/netflix/genie/web/services/impl/LocalFileSystemAttachmentServiceImpl.java @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; +import com.netflix.genie.web.exceptions.checked.IllegalAttachmentFileNameException; import com.netflix.genie.web.exceptions.checked.AttachmentTooLargeException; import com.netflix.genie.web.exceptions.checked.SaveAttachmentException; import com.netflix.genie.web.properties.AttachmentServiceProperties; @@ -93,6 +94,11 @@ public Set saveAttachments( final long attachmentSize = attachment.contentLength(); final String filename = attachment.getFilename(); + if (filename != null && filename.contains("/")) { + throw new IllegalAttachmentFileNameException("Attachment filename " + filename + " is illegal. " + + "It should not contain /"); + } + if (attachmentSize > this.attachmentServiceProperties.getMaxSize().toBytes()) { throw new AttachmentTooLargeException("Attachment is too large: " + filename); } diff --git a/genie-web/src/test/groovy/com/netflix/genie/web/exceptions/checked/GenieWebCheckedExceptionsSpec.groovy b/genie-web/src/test/groovy/com/netflix/genie/web/exceptions/checked/GenieWebCheckedExceptionsSpec.groovy index e103242a9fa..be424812995 100644 --- a/genie-web/src/test/groovy/com/netflix/genie/web/exceptions/checked/GenieWebCheckedExceptionsSpec.groovy +++ b/genie-web/src/test/groovy/com/netflix/genie/web/exceptions/checked/GenieWebCheckedExceptionsSpec.groovy @@ -66,6 +66,7 @@ class GenieWebCheckedExceptionsSpec extends Specification { exceptionClass | _ AgentLaunchException | _ AttachmentTooLargeException | _ + IllegalAttachmentFileNameException | _ IdAlreadyExistsException | _ JobDirectoryManifestNotFoundException | _ JobNotArchivedException | _ diff --git a/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/LocalFileSystemAttachmentServiceImplSpec.groovy b/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/LocalFileSystemAttachmentServiceImplSpec.groovy index 839dafbb322..bab74453cb8 100644 --- a/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/LocalFileSystemAttachmentServiceImplSpec.groovy +++ b/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/LocalFileSystemAttachmentServiceImplSpec.groovy @@ -18,11 +18,13 @@ package com.netflix.genie.web.services.impl import com.google.common.collect.Sets +import com.netflix.genie.web.exceptions.checked.IllegalAttachmentFileNameException import com.netflix.genie.web.exceptions.checked.AttachmentTooLargeException import com.netflix.genie.web.exceptions.checked.SaveAttachmentException import com.netflix.genie.web.properties.AttachmentServiceProperties import org.apache.commons.io.FileUtils import org.apache.commons.lang3.RandomStringUtils +import org.mockito.Mockito import org.springframework.core.io.FileSystemResource import org.springframework.core.io.Resource import org.springframework.util.unit.DataSize @@ -151,4 +153,17 @@ class LocalFileSystemAttachmentServiceImplSpec extends Specification { then: thrown(SaveAttachmentException) } + + def "reject attachments with illegal filename"() { + Set attachments = Sets.newHashSet() + Resource attachment = Mock(Resource) + Mockito.doReturn("../../../root/breakout.file").when(attachment).getFilename() + attachments.add(attachment) + + when: + service.saveAttachments(null, attachments) + + then: + thrown(IllegalAttachmentFileNameException) + } }