From ec5e0d06631507f540dc887efc0544cbd4a0c469 Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Tue, 24 Jul 2018 11:13:50 +0100 Subject: [PATCH] REPO-3720, MNT-19825 ImageMagick docker transformer does not take arbitrary command line options. --- .../transformer/ImageMagickController.java | 18 ++++++++++++++++-- .../transformer/ImageMagickControllerTest.java | 17 +++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/alfresco-docker-imagemagick/src/main/java/org/alfresco/transformer/ImageMagickController.java b/alfresco-docker-imagemagick/src/main/java/org/alfresco/transformer/ImageMagickController.java index 76088155..ef493414 100644 --- a/alfresco-docker-imagemagick/src/main/java/org/alfresco/transformer/ImageMagickController.java +++ b/alfresco-docker-imagemagick/src/main/java/org/alfresco/transformer/ImageMagickController.java @@ -146,7 +146,19 @@ public class ImageMagickController extends AbstractTransformerController @RequestParam(value = "resizeHeight", required = false) Integer resizeHeight, @RequestParam(value = "resizePercentage", required = false) Boolean resizePercentage, @RequestParam(value = "allowEnlargement", required = false) Boolean allowEnlargement, - @RequestParam(value = "maintainAspectRatio", required = false) Boolean maintainAspectRatio) + @RequestParam(value = "maintainAspectRatio", required = false) Boolean maintainAspectRatio, + + // The commandOptions parameter is supported in ACS 6.0.1 because there may be + // custom renditions that use it. However the Transform service should + // not support it as it provides the option to specify arbitrary command + // options or even the option to run something else on the command line. + // All Transform service options should be checked as is done for the other + // request parameters. Setting this option in the rendition's + // ImageTransformationOptions object is being deprecated for the point where + // The Transform service is being used for all transforms. In the case of + // ACS 6.0, this is relatively safe as it requires an AMP to be installed + // which supplies the commandOptions. + @RequestParam(value = "commandOptions", required = false) String commandOptions) { if (cropGravity != null) { @@ -265,7 +277,9 @@ public class ImageMagickController extends AbstractTransformerController ? "["+startPage+']' : "["+startPage+'-'+endPage+']'; - String options = args.toString(); + String options = + (commandOptions == null || "".equals(commandOptions.trim()) ? "" : commandOptions + ' ') + + args.toString(); executeTransformCommand(options, sourceFile, pageRange, targetFile, timeout); return createAttachment(targetFilename, targetFile, testDelay); diff --git a/alfresco-docker-imagemagick/src/test/java/org/alfresco/transformer/ImageMagickControllerTest.java b/alfresco-docker-imagemagick/src/test/java/org/alfresco/transformer/ImageMagickControllerTest.java index 5cf5025a..08feba17 100644 --- a/alfresco-docker-imagemagick/src/test/java/org/alfresco/transformer/ImageMagickControllerTest.java +++ b/alfresco-docker-imagemagick/src/test/java/org/alfresco/transformer/ImageMagickControllerTest.java @@ -147,4 +147,21 @@ public class ImageMagickControllerTest extends AbstractTransformerControllerTest .andExpect(content().bytes(expectedTargetFileBytes)) .andExpect(header().string("Content-Disposition", "attachment; filename*= UTF-8''quick."+targetExtension)); } + + @Test + public void deprecatedCommandOptionsTest() throws Exception + { + // Example of why the commandOptions parameter is a bad idea. + expectedOptions = "( horrible command / ); -resize 321x654>"; + mockMvc.perform(MockMvcRequestBuilders.fileUpload("/transform") + .file(sourceFile) + .param("targetExtension", targetExtension) + .param("thumbnail", "false") + .param("resizeWidth", "321") + .param("resizeHeight", "654") + .param("commandOptions", "( horrible command / );")) + .andExpect(status().is(200)) + .andExpect(content().bytes(expectedTargetFileBytes)) + .andExpect(header().string("Content-Disposition", "attachment; filename*= UTF-8''quick."+targetExtension)); + } }