You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by dk...@apache.org on 2021/07/04 02:40:23 UTC
[sling-whiteboard] branch master updated: Improving test coverage
of stransform servlet and fixing behavior to return a 400 on an invalid
output format
This is an automated email from the ASF dual-hosted git repository.
dklco pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
The following commit(s) were added to refs/heads/master by this push:
new cc38ef4 Improving test coverage of stransform servlet and fixing behavior to return a 400 on an invalid output format
cc38ef4 is described below
commit cc38ef4982ec9e8d0a52903f415f826477c3b9d5
Author: Dan Klco <kl...@adobe.com>
AuthorDate: Sat Jul 3 22:40:11 2021 -0400
Improving test coverage of stransform servlet and fixing behavior to return a 400 on an invalid output format
---
.../sling/commons/thumbnails/OutputFileFormat.java | 26 ++++----
.../internal/DynamicTransformServlet.java | 14 ++--
.../thumbnails/internal/TransformServlet.java | 9 ++-
.../internal/DynamicTransformServletTest.java | 12 ++++
.../thumbnails/internal/TransformServletTest.java | 75 +++++++++++++++++++---
5 files changed, 107 insertions(+), 29 deletions(-)
diff --git a/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/OutputFileFormat.java b/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/OutputFileFormat.java
index 5941a31..9d4d0bc 100644
--- a/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/OutputFileFormat.java
+++ b/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/OutputFileFormat.java
@@ -16,15 +16,13 @@
*/
package org.apache.sling.commons.thumbnails;
+import com.google.common.net.MediaType;
+
import org.apache.commons.lang3.StringUtils;
import org.apache.sling.api.SlingHttpServletRequest;
+import org.jetbrains.annotations.NotNull;
import org.osgi.annotation.versioning.ProviderType;
-import java.util.Optional;
-
-import com.google.common.base.Enums;
-import com.google.common.net.MediaType;
-
/**
* Enumeration of the valid output formats for the thumbnail generator.
*/
@@ -39,9 +37,8 @@ public enum OutputFileFormat {
* @return the format for the suffix
*/
public static OutputFileFormat forRequest(SlingHttpServletRequest request) {
- String suffixExtension = StringUtils.substringAfterLast(request.getRequestPathInfo().getSuffix(), ".")
- .toUpperCase();
- return Enums.getIfPresent(OutputFileFormat.class, suffixExtension).or(OutputFileFormat.JPEG);
+ return forValue(StringUtils.substringAfterLast(request.getRequestPathInfo().getSuffix(), "."));
+
}
/**
@@ -50,9 +47,16 @@ public enum OutputFileFormat {
* @param request the requested format
* @return the format requested
*/
- public static OutputFileFormat forValue(String format) {
- return Enums.getIfPresent(OutputFileFormat.class,
- Optional.ofNullable(format).map(String::toUpperCase).orElse("JPEG")).or(OutputFileFormat.JPEG);
+ public static OutputFileFormat forValue(@NotNull String format) {
+ format = format.toUpperCase();
+ if ("JPG".equals(format)) {
+ format = "JPEG";
+ }
+ try {
+ return Enum.valueOf(OutputFileFormat.class, format);
+ } catch (IllegalArgumentException | NullPointerException e) {
+ throw new BadRequestException("Could not get valid extension from: " + format);
+ }
}
private String mimeType;
diff --git a/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/internal/DynamicTransformServlet.java b/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/internal/DynamicTransformServlet.java
index 9c040f6..054bb60 100644
--- a/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/internal/DynamicTransformServlet.java
+++ b/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/internal/DynamicTransformServlet.java
@@ -20,6 +20,7 @@ import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.List;
+import java.util.Optional;
import javax.servlet.Servlet;
import javax.servlet.ServletException;
@@ -67,13 +68,16 @@ public class DynamicTransformServlet extends SlingAllMethodsServlet {
throws ServletException, IOException {
log.trace("doPost");
- Resource resource = request.getResourceResolver().getResource(request.getParameter("resource"));
+ try {
- OutputFileFormat format = OutputFileFormat.forValue(request.getParameter("format"));
- response.setHeader("Content-Disposition", "filename=" + resource.getName());
- response.setContentType(format.getMimeType());
+ Resource resource = request.getResourceResolver()
+ .getResource(Optional.ofNullable(request.getParameter("resource"))
+ .orElseThrow(() -> new BadRequestException("Parameter resource must be supplied")));
- try {
+ OutputFileFormat format = OutputFileFormat
+ .forValue(Optional.ofNullable(request.getParameter("format")).orElse("jpeg"));
+ response.setHeader("Content-Disposition", "filename=" + resource.getName());
+ response.setContentType(format.getMimeType());
ObjectMapper objectMapper = new ObjectMapper();
List<TransformationHandlerConfigImpl> transformations = parsePostBody(request, objectMapper);
diff --git a/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/internal/TransformServlet.java b/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/internal/TransformServlet.java
index 21c7f7e..77e8201 100644
--- a/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/internal/TransformServlet.java
+++ b/org.apache.sling.commons.thumbnail/src/main/java/org/apache/sling/commons/thumbnails/internal/TransformServlet.java
@@ -152,7 +152,7 @@ public class TransformServlet extends SlingAllMethodsServlet {
log.debug("Using existing rendition {}", name);
IOUtils.copy(rendition.adaptTo(InputStream.class), response.getOutputStream());
} else {
- ByteArrayOutputStream baos = transform(request, response, format, transformation);
+ ByteArrayOutputStream baos = transform(request, response, transformation);
Resource file = ResourceUtil.getOrCreateResource(serviceResolver,
request.getResource().getPath() + "/" + expectedPath,
Collections.singletonMap(JcrConstants.JCR_PRIMARYTYPE, JcrConstants.NT_FILE),
@@ -165,16 +165,15 @@ public class TransformServlet extends SlingAllMethodsServlet {
}
} else {
log.debug("Sending transformation to response....");
- transform(request, response, format, transformation);
+ transform(request, response, transformation);
}
}
private ByteArrayOutputStream transform(SlingHttpServletRequest request, SlingHttpServletResponse response,
- String format, Transformation transformation) throws IOException {
+ Transformation transformation) throws IOException {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
- transformer.transform(request.getResource(), transformation, OutputFileFormat.valueOf(format.toUpperCase()),
- baos);
+ transformer.transform(request.getResource(), transformation, OutputFileFormat.forRequest(request), baos);
IOUtils.copy(new ByteArrayInputStream(baos.toByteArray()), response.getOutputStream());
return baos;
}
diff --git a/org.apache.sling.commons.thumbnail/src/test/java/org/apache/sling/commons/thumbnails/internal/DynamicTransformServletTest.java b/org.apache.sling.commons.thumbnail/src/test/java/org/apache/sling/commons/thumbnails/internal/DynamicTransformServletTest.java
index ea36641..384ff62 100644
--- a/org.apache.sling.commons.thumbnail/src/test/java/org/apache/sling/commons/thumbnails/internal/DynamicTransformServletTest.java
+++ b/org.apache.sling.commons.thumbnail/src/test/java/org/apache/sling/commons/thumbnails/internal/DynamicTransformServletTest.java
@@ -88,6 +88,18 @@ public class DynamicTransformServletTest {
}
@Test
+ public void testNoResource() throws IOException, ServletException {
+
+ context.request().addRequestParameter("format", "png");
+ context.request().setContent(
+ "[{\"handlerType\":\"sling/commons/thumbnails/transformers/crop\",\"properties\":{\"position\":\"CENTER\",\"width\":1000,\"height\":1000}}]"
+ .getBytes());
+ dts.doPost(context.request(), context.response());
+
+ assertEquals(400, context.response().getStatus());
+ }
+
+ @Test
public void testNoFormat() throws IOException, ServletException {
context.request().addRequestParameter("resource", "/content/apache/sling-apache-org/index/apache.png");
diff --git a/org.apache.sling.commons.thumbnail/src/test/java/org/apache/sling/commons/thumbnails/internal/TransformServletTest.java b/org.apache.sling.commons.thumbnail/src/test/java/org/apache/sling/commons/thumbnails/internal/TransformServletTest.java
index b8c4daa..523278b 100644
--- a/org.apache.sling.commons.thumbnail/src/test/java/org/apache/sling/commons/thumbnails/internal/TransformServletTest.java
+++ b/org.apache.sling.commons.thumbnail/src/test/java/org/apache/sling/commons/thumbnails/internal/TransformServletTest.java
@@ -18,7 +18,10 @@ package org.apache.sling.commons.thumbnails.internal;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.io.IOException;
@@ -28,6 +31,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import javax.servlet.RequestDispatcher;
import javax.servlet.ServletException;
import org.apache.sling.api.resource.LoginException;
@@ -44,24 +48,25 @@ import org.apache.sling.commons.thumbnails.internal.providers.ImageThumbnailProv
import org.apache.sling.commons.thumbnails.internal.providers.PdfThumbnailProvider;
import org.apache.sling.commons.thumbnails.internal.transformers.CropHandler;
import org.apache.sling.commons.thumbnails.internal.transformers.ResizeHandler;
+import org.apache.sling.servlethelpers.MockRequestDispatcherFactory;
import org.apache.sling.testing.mock.sling.junit.SlingContext;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.mockito.Mockito;
import org.osgi.framework.BundleContext;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class TransformServletTest {
- private static final Logger log = LoggerFactory.getLogger(TransformServletTest.class);
-
private TransformServlet ts;
@Rule
public final SlingContext context = new SlingContext();
+ private Resource resource;
+
+ private RequestDispatcher dispatcher;
+
@Before
public void init() throws IllegalAccessException, LoginException {
ContextHelper.initContext(context);
@@ -83,7 +88,7 @@ public class TransformServletTest {
TransformationImpl transformation = new TransformationImpl(handlers, "test");
- Resource resource = Mockito.mock(Resource.class);
+ resource = Mockito.mock(Resource.class);
Mockito.when(resource.getPath()).thenReturn("/conf");
Mockito.when(resource.adaptTo(Mockito.any())).thenReturn(transformation);
Mockito.when(resolver.findResources(Mockito.anyString(), Mockito.anyString())).thenAnswer((ans) -> {
@@ -110,17 +115,21 @@ public class TransformServletTest {
when(thumbnailSupport.getPersistableTypes()).thenReturn(Collections.emptySet());
when(thumbnailSupport.getSupportedTypes()).thenReturn(Collections.singleton("nt:file"));
when(thumbnailSupport.getMetaTypePropertyPath("nt:file")).thenReturn("jcr:content/jcr:mimeType");
+ when(thumbnailSupport.getServletErrorResourcePath()).thenReturn("/content/error");
TransformerImpl transformer = new TransformerImpl(providers, thumbnailSupport, th);
ts = new TransformServlet(thumbnailSupport, transformer, tsu, new TransformationCache(tsu),
mock(BundleContext.class));
+ MockRequestDispatcherFactory dispatcherFactory = mock(MockRequestDispatcherFactory.class);
+ dispatcher = mock(RequestDispatcher.class);
+ when(dispatcherFactory.getRequestDispatcher(anyString(), any())).thenReturn(dispatcher);
+ context.request().setRequestDispatcherFactory(dispatcherFactory);
+
}
@Test
public void testValid() throws IOException, ServletException {
- log.info("testContentTypes");
-
context.currentResource("/content/apache/sling-apache-org/index/apache.png");
context.requestPathInfo().setSuffix("/test.png");
context.requestPathInfo().setExtension("transform");
@@ -131,8 +140,58 @@ public class TransformServletTest {
}
@Test
+ public void testUnsupportedOutput() throws IOException, ServletException {
+
+ context.currentResource("/content/apache/sling-apache-org/index/apache.png");
+ context.requestPathInfo().setSuffix("/test.webp");
+ context.requestPathInfo().setExtension("transform");
+
+ ts.doGet(context.request(), context.response());
+
+ assertEquals(400, context.response().getStatus());
+ }
+
+ @Test
+ public void testUnexpectedException() throws IOException, ServletException {
+
+ Resource throwy = mock(Resource.class);
+ when(throwy.getResourceType()).thenThrow(new RuntimeException());
+ context.currentResource(throwy);
+ context.requestPathInfo().setSuffix("/test.jpg");
+ context.requestPathInfo().setExtension("transform");
+
+ ts.doGet(context.request(), context.response());
+
+ assertEquals(500, context.response().getStatus());
+ assertEquals("image/jpeg", context.response().getContentType());
+
+ verify(dispatcher).forward(any(), any());
+ }
+
+ @Test
+ public void testInvalidConfig() throws ServletException, IOException {
+ List<TransformationHandlerConfig> handlers = new ArrayList<>();
+ Map<String, Object> crop = new HashMap<>();
+ crop.put(CropHandler.PN_POSITION, "center");
+ crop.put(ResizeHandler.PN_WIDTH, -1);
+ crop.put(ResizeHandler.PN_HEIGHT, 200);
+ handlers.add(new TransformationHandlerConfigImpl(CropHandler.RESOURCE_TYPE, crop));
+
+ TransformationImpl transformation = new TransformationImpl(handlers, "test");
+
+ Mockito.when(resource.adaptTo(Mockito.any())).thenReturn(transformation);
+
+ context.currentResource("/content/apache/sling-apache-org/index/apache.png");
+ context.requestPathInfo().setSuffix("/test.png");
+ context.requestPathInfo().setExtension("transform");
+
+ ts.doGet(context.request(), context.response());
+
+ assertEquals(400, context.response().getStatus());
+ }
+
+ @Test
public void testInvalid() throws IOException, ServletException {
- log.info("testContentTypes");
context.currentResource("/content/apache/sling-apache-org/index/apache.png");
context.requestPathInfo().setSuffix("/te.png");