diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a7f3ed4b..cb1827bf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,27 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] -## [v2.1.6] +## [v2.1.10] + +### Fixed + +- 0652 - Fixed over-escaping of static rendition paths (stemming from #639) +- 0653 - 6.5 only - Bundle does not start on 6.5.9 +- 0654 - 6.5 only - ExternalRenditionDispatcher expression evaluation does not resolve to correct asset resource +- 0655 - 6.5 only - Erring HTTP POST is made on all page loads + +## [v2.1.8] ### Fixed - 0643 - Site title shows up the component policy title if not authored on header component policy -- 0639 - Correct issue introduced in 2.1.0 via #639 where URLs were being over-escaped breaking External Redirect Disaptching - + +## [v2.1.6] + +### Fixed + +- 0639 - Fixed regression introduced in #639 that over-escaped URLs + ## [v2.1.4] ### Fixed diff --git a/all/pom.xml b/all/pom.xml index 2342dfa92..9ad244428 100644 --- a/all/pom.xml +++ b/all/pom.xml @@ -22,7 +22,7 @@ com.adobe.aem.commons assetshare - 2.1.5-SNAPSHOT + 2.1.9-SNAPSHOT ../pom.xml diff --git a/core/pom.xml b/core/pom.xml index 8a69c7c09..120537cb3 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -19,7 +19,7 @@ com.adobe.aem.commons assetshare - 2.1.5-SNAPSHOT + 2.1.9-SNAPSHOT ../pom.xml assetshare.core @@ -57,6 +57,8 @@ Import-Package: javax.annotation;version=0.0.0, \ com.day.cq.search.eval;version="[1.4.0,2)", \ org.apache.commons.io;version="[2.6.0,4)", \ org.apache.commons.lang3;version="[3.9.0,4)", \ + org.osgi.framework;version="[1.9.0,2)", \ + org.apache.sling.api.wrappers;version="[2.6.4,3)", \ * -conditionalpackage: com.adobe.acs.commons.util.*, -exportcontents: ${packages;VERSIONED} diff --git a/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/AssetRendition.java b/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/AssetRendition.java index cc9a5577c..9714d835d 100644 --- a/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/AssetRendition.java +++ b/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/AssetRendition.java @@ -21,11 +21,13 @@ import com.adobe.aem.commons.assetshare.util.UrlUtil; import com.day.text.Text; +import org.apache.commons.lang3.StringUtils; import org.apache.http.client.utils.URIUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.net.URI; +import java.net.URISyntaxException; import java.util.Optional; /** @@ -40,16 +42,21 @@ public class AssetRendition { private String mimeType; public AssetRendition(URI uri, Long size, String mimeType) { - setBinaryUri(uri); - setSize(size); - setMimeType(mimeType); + this(uri.toString(), size, mimeType); + } public AssetRendition(String uri, Long size, String mimeType) { + URI cleanURI = null; - uri = UrlUtil.escape(uri, true); + try { + cleanURI = cleanURI(uri.toString()); + } catch (URISyntaxException e) { + log.warn("Unable to clean the URI [ {} ], using it as is.", uri, e); + cleanURI = URI.create(uri); + } - setBinaryUri(URI.create(uri)); + setBinaryUri(cleanURI); setSize(size); setMimeType(mimeType); } @@ -84,6 +91,12 @@ public void setMimeType(String mimeType) { this.mimeType = mimeType; } + private URI cleanURI(String uri) throws URISyntaxException { + uri = StringUtils.replace(uri, " ", "%20"); + uri = StringUtils.replace(uri, "/_jcr_content", "/jcr:content"); + + return new URI(uri); + } } diff --git a/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/impl/AssetRenditionsImpl.java b/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/impl/AssetRenditionsImpl.java index 653e1b37f..df8a98895 100644 --- a/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/impl/AssetRenditionsImpl.java +++ b/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/impl/AssetRenditionsImpl.java @@ -78,7 +78,7 @@ public Map getOptions(final Map mappin @Override public String evaluateExpression(final SlingHttpServletRequest request, String expression) { - final AssetModel assetModel = request.adaptTo(AssetModel.class); + final AssetModel assetModel = modelFactory.createModel(request.getResource(), AssetModel.class); return evaluateExpression(assetModel, new AssetRenditionParameters(request).getRenditionName(), expression); } diff --git a/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/AbstractRenditionDispatcherImpl.java b/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/AbstractRenditionDispatcherImpl.java index a34d27cad..6c83d0bb7 100644 --- a/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/AbstractRenditionDispatcherImpl.java +++ b/core/src/main/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/AbstractRenditionDispatcherImpl.java @@ -92,7 +92,8 @@ public String cleanURI(String uri) throws URISyntaxException { .filter(nvp -> !QUERY_PARAM_SUGGESTED_EXTENSION.equals(nvp.getName())) .forEach(nvp -> newURI.addParameter(nvp.getName(), nvp.getValue()));; + // Common pattern in DM URLS; This escaping probably is not necessary, but keeping for + // now as it would take a closer look to remove (and it breaks tests) return StringUtils.replace(newURI.toString(), "%24", "$"); } - } \ No newline at end of file diff --git a/core/src/main/java/com/adobe/aem/commons/assetshare/util/UrlUtil.java b/core/src/main/java/com/adobe/aem/commons/assetshare/util/UrlUtil.java index 8a2786e18..371ed0eab 100644 --- a/core/src/main/java/com/adobe/aem/commons/assetshare/util/UrlUtil.java +++ b/core/src/main/java/com/adobe/aem/commons/assetshare/util/UrlUtil.java @@ -62,22 +62,25 @@ public static final String escape(final String unescaped, final boolean preventD // URL cannot handle spaces, so convert them to %20; escapePath(..) will handle converting them back for later escaping tmp = StringUtils.replace(tmp, " ", "%20"); - try { - final URL url = new URL(tmp); - String path = escapePath(url.getPath()); + if (!isPath(unescaped)) { + try { + final URL url = new URL(tmp); + String path = escapePath(url.getPath()); - if (StringUtils.isNotBlank(url.getQuery())) { - path += "?" + url.getQuery(); - } + if (StringUtils.isNotBlank(url.getQuery())) { + path += "?" + url.getQuery(); + } - // Reconstruct the URL using the escaped path - return new URL(url.getProtocol(), url.getHost(), url.getPort(), path).toString(); + // Reconstruct the URL using the escaped path + return new URL(url.getProtocol(), url.getHost(), url.getPort(), path).toString(); - } catch (MalformedURLException e) { - // Treat as internal path - log.debug("Could not evaluate unescaped string [ {} ] as a URL. Falling back to escape as path.", e); - return escapePath(tmp); + } catch (MalformedURLException e) { + // Treat as internal path + log.debug("Could not evaluate unescaped string [ {} ] as a URL. Falling back to escape as path.", unescaped, e); + } } + + return escapePath(tmp); } /** @@ -117,4 +120,20 @@ private static String escapePath(String path) { path = Text.escapePath(path); return path; } + + /** + * @param candidate the candidate URI to path to check + * @return true if it looks like a path, and false it looks like a URL + */ + private static boolean isPath(String candidate) { + if (StringUtils.isNotBlank(candidate)) { + if (candidate.matches("^[a-zA-Z0-9]+://.+")) { + return false; + } else if (StringUtils.startsWith(candidate, "//")) { + return false; + } + } + + return true; + } } diff --git a/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/AssetRenditionTest.java b/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/AssetRenditionTest.java index f8dccb11b..181cc46f4 100644 --- a/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/AssetRenditionTest.java +++ b/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/AssetRenditionTest.java @@ -16,14 +16,17 @@ public void getBinaryUri() { assetRendition = new AssetRendition("/content/dam/test asset.jpg", 123L, "image/jpeg"); assertEquals("/content/dam/test%20asset.jpg", assetRendition.getBinaryUri().toString()); + assetRendition = new AssetRendition("/content/dam/test asset.jpg/jcr:content/renditions.cq5dam.web.1.2.png", 123L, "image/jpeg"); + assertEquals("/content/dam/test%20asset.jpg/jcr:content/renditions.cq5dam.web.1.2.png", assetRendition.getBinaryUri().toString()); + assetRendition = new AssetRendition("/content/dam/test+with+plus.jpg", 123L, "image/jpeg"); - assertEquals("/content/dam/test%2bwith%2bplus.jpg", assetRendition.getBinaryUri().toString()); + assertEquals("/content/dam/test+with+plus.jpg", assetRendition.getBinaryUri().toString()); assetRendition = new AssetRendition("https://test.com/content/dam/test asset.jpg", 123L, "image/jpeg"); assertEquals("https://test.com/content/dam/test%20asset.jpg", assetRendition.getBinaryUri().toString()); assetRendition = new AssetRendition("https://smartimaging.scene7.com/is/image/DynamicMediaNA/test (test):Medium", 123L, "image/jpeg"); - assertEquals("https://smartimaging.scene7.com/is/image/DynamicMediaNA/test%20(test)%3aMedium", assetRendition.getBinaryUri().toString()); + assertEquals("https://smartimaging.scene7.com/is/image/DynamicMediaNA/test%20(test):Medium", assetRendition.getBinaryUri().toString()); assetRendition = new AssetRendition("https://smartimaging.scene7.com/is/image/DynamicMediaNA/test (test)?$grayscale$", 123L, "image/jpeg"); assertEquals("https://smartimaging.scene7.com/is/image/DynamicMediaNA/test%20(test)?$grayscale$", assetRendition.getBinaryUri().toString()); diff --git a/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/ExternalRedirectRenditionDispatcherImplTest.java b/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/ExternalRedirectRenditionDispatcherImplTest.java index 44d654710..b8f28bfba 100644 --- a/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/ExternalRedirectRenditionDispatcherImplTest.java +++ b/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/ExternalRedirectRenditionDispatcherImplTest.java @@ -65,7 +65,6 @@ public class ExternalRedirectRenditionDispatcherImplTest { public void setUp() throws Exception { ctx.load().json(getClass().getResourceAsStream("ExternalRedirectRenditionDispatcherImplTest.json"), "/content/dam"); ctx.currentResource("/content/dam/test.png"); - doReturn(DamUtil.resolveToAsset(ctx.resourceResolver().getResource("/content/dam/test.png"))).when(assetResolver).resolveAsset(ctx.request()); ctx.registerService(MimeTypeService.class, mimeTypeService); ctx.registerService(ExpressionEvaluator.class, new ExpressionEvaluatorImpl()); @@ -161,7 +160,6 @@ public void dispatch() throws IOException, ServletException { assetRenditionDispatcher.dispatch(ctx.request(), ctx.response()); assertEquals(301, ctx.response().getStatus()); - //assertEquals("http://test.scene7.com/is/image/testing/test?%24greyscale%24", ctx.response().getHeader("Location")); } @Test @@ -175,7 +173,6 @@ public void dispatch_WithSpacesInPath() throws IOException, ServletException { final AssetRenditionDispatcher assetRenditionDispatcher = ctx.getService(AssetRenditionDispatcher.class); - doReturn(DamUtil.resolveToAsset(ctx.resourceResolver().getResource("/content/dam/test with spaces.png"))).when(assetResolver).resolveAsset(ctx.request()); ctx.currentResource("/content/dam/test with spaces.png"); ctx.requestPathInfo().setExtension("rendition"); ctx.requestPathInfo().setSuffix("testing/download/asset.rendition"); @@ -198,7 +195,6 @@ public void dispatch_WithHostAndSpacesInPath() throws IOException, ServletExcept final AssetRenditionDispatcher assetRenditionDispatcher = ctx.getService(AssetRenditionDispatcher.class); - doReturn(DamUtil.resolveToAsset(ctx.resourceResolver().getResource("/content/dam/test with spaces.png"))).when(assetResolver).resolveAsset(ctx.request()); ctx.currentResource("/content/dam/test with spaces.png"); ctx.requestPathInfo().setExtension("rendition"); ctx.requestPathInfo().setSuffix("testing/download/asset.rendition"); diff --git a/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/InternalRedirectRenditionDispatcherImplTest.java b/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/InternalRedirectRenditionDispatcherImplTest.java index dad6fa6f6..667e8745f 100644 --- a/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/InternalRedirectRenditionDispatcherImplTest.java +++ b/core/src/test/java/com/adobe/aem/commons/assetshare/content/renditions/impl/dispatchers/InternalRedirectRenditionDispatcherImplTest.java @@ -78,7 +78,6 @@ public void setUp() throws Exception { ctx.load().json(getClass().getResourceAsStream("InternalRedirectRenditionDispatcherImplTest.json"), "/content/dam"); ctx.currentResource("/content/dam/test.png"); - doReturn(DamUtil.resolveToAsset(ctx.resourceResolver().getResource("/content/dam/test.png"))).when(assetResolver).resolveAsset(ctx.request()); ctx.registerService(MimeTypeService.class, mimeTypeService); ctx.registerService(ExpressionEvaluator.class, new ExpressionEvaluatorImpl()); @@ -206,7 +205,6 @@ public void dispatch_WithSpacesInPath() throws IOException, ServletException { final AssetRenditionDispatcher assetRenditionDispatcher = ctx.getService(AssetRenditionDispatcher.class); - doReturn(DamUtil.resolveToAsset(ctx.resourceResolver().getResource("/content/dam/test with spaces.png"))).when(assetResolver).resolveAsset(ctx.request()); ctx.currentResource("/content/dam/test with spaces.png"); ctx.requestPathInfo().setExtension("rendition"); ctx.requestPathInfo().setSuffix("testing/download/asset.rendition"); diff --git a/dispatcher/pom.xml b/dispatcher/pom.xml index 45b469be4..526032b4c 100644 --- a/dispatcher/pom.xml +++ b/dispatcher/pom.xml @@ -7,7 +7,7 @@ com.adobe.aem.commons assetshare - 2.1.5-SNAPSHOT + 2.1.9-SNAPSHOT ../pom.xml diff --git a/pom.xml b/pom.xml index 58367b422..321606e7f 100644 --- a/pom.xml +++ b/pom.xml @@ -18,7 +18,7 @@ com.adobe.aem.commons assetshare pom - 2.1.5-SNAPSHOT + 2.1.9-SNAPSHOT Asset Share Commons - Reactor Project asset-share-commons diff --git a/ui.apps.structure/pom.xml b/ui.apps.structure/pom.xml index 6d0201db9..2864f25d7 100644 --- a/ui.apps.structure/pom.xml +++ b/ui.apps.structure/pom.xml @@ -8,7 +8,7 @@ com.adobe.aem.commons assetshare - 2.1.5-SNAPSHOT + 2.1.9-SNAPSHOT ../pom.xml diff --git a/ui.apps/pom.xml b/ui.apps/pom.xml index 9a1f75c11..7353bd758 100644 --- a/ui.apps/pom.xml +++ b/ui.apps/pom.xml @@ -23,7 +23,7 @@ com.adobe.aem.commons assetshare - 2.1.5-SNAPSHOT + 2.1.9-SNAPSHOT ../pom.xml diff --git a/ui.apps/src/main/content/jcr_root/apps/asset-share-commons/components/modals/downloads/clientlibs/site/js/downloads.js b/ui.apps/src/main/content/jcr_root/apps/asset-share-commons/components/modals/downloads/clientlibs/site/js/downloads.js index 300dc04ee..44be93149 100644 --- a/ui.apps/src/main/content/jcr_root/apps/asset-share-commons/components/modals/downloads/clientlibs/site/js/downloads.js +++ b/ui.apps/src/main/content/jcr_root/apps/asset-share-commons/components/modals/downloads/clientlibs/site/js/downloads.js @@ -49,17 +49,21 @@ jQuery((function($, ns, semanticModal, download) { } function updateBadge() { - $.post(DOWNLOADS_URL, getModalData()).then(function(htmlResponse) { - var count = ns.Elements.element('downloads-modal', htmlResponse).data('asset-share-downloads-count') || 0; - ns.Elements.element("downloads-count").text(count); - }); + if (DOWNLOADS_URL) { + $.post(DOWNLOADS_URL, getModalData()).then(function(htmlResponse) { + var count = ns.Elements.element('downloads-modal', htmlResponse).data('asset-share-downloads-count') || 0; + ns.Elements.element("downloads-count").text(count); + }); + } } function update() { - $.post(DOWNLOADS_URL, getModalData()).then(function(htmlResponse) { - ns.Elements.update(htmlResponse, WHEN_DOWNLOADS_UPDATED); - init(); - }); + if (DOWNLOADS_URL) { + $.post(DOWNLOADS_URL, getModalData()).then(function(htmlResponse) { + ns.Elements.update(htmlResponse, WHEN_DOWNLOADS_UPDATED); + init(); + }); + } } function show(e) { diff --git a/ui.content.sample/pom.xml b/ui.content.sample/pom.xml index a624abbbc..30bd1ce4c 100644 --- a/ui.content.sample/pom.xml +++ b/ui.content.sample/pom.xml @@ -23,7 +23,7 @@ com.adobe.aem.commons assetshare - 2.1.5-SNAPSHOT + 2.1.9-SNAPSHOT ../pom.xml diff --git a/ui.content/pom.xml b/ui.content/pom.xml index 2419ecd4d..9d8463372 100644 --- a/ui.content/pom.xml +++ b/ui.content/pom.xml @@ -22,7 +22,7 @@ com.adobe.aem.commons assetshare - 2.1.5-SNAPSHOT + 2.1.9-SNAPSHOT ../pom.xml diff --git a/ui.frontend.theme.dark/pom.xml b/ui.frontend.theme.dark/pom.xml index 15173c4b8..7939da4f2 100644 --- a/ui.frontend.theme.dark/pom.xml +++ b/ui.frontend.theme.dark/pom.xml @@ -23,7 +23,7 @@ com.adobe.aem.commons assetshare - 2.1.5-SNAPSHOT + 2.1.9-SNAPSHOT ../pom.xml diff --git a/ui.frontend.theme.light/pom.xml b/ui.frontend.theme.light/pom.xml index 24e37b4cb..dbb03fbe0 100644 --- a/ui.frontend.theme.light/pom.xml +++ b/ui.frontend.theme.light/pom.xml @@ -23,7 +23,7 @@ com.adobe.aem.commons assetshare - 2.1.5-SNAPSHOT + 2.1.9-SNAPSHOT ../pom.xml