Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: externalize links fails with ac scriptlets #2388

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -158,12 +158,20 @@ public boolean accepts(@NotNull String path, @NotNull SlingHttpServletRequest re
ResourceResolver resourceResolver = request.getResourceResolver();
String externalPath;
try {
if (vanityConfig == VanityConfig.MAPPING || vanityConfig == VanityConfig.ALWAYS) {
externalPath = externalizer.publishLink(resourceResolver, getPathOrVanityUrl(path, resourceResolver));
externalPath = LinkMaskingUtil.maskAndDo(path, maskedPath -> {
if (vanityConfig == VanityConfig.MAPPING || vanityConfig == VanityConfig.ALWAYS) {
return externalizer.publishLink(resourceResolver, getPathOrVanityUrl(maskedPath, resourceResolver));
} else {
return externalizer.publishLink(resourceResolver, maskedPath);
}
});
} catch (Exception ex) {
String message = "Failed to externalize url '{}': {}";
if (LOG.isDebugEnabled()) {
LOG.warn(message, path, ex.getMessage(), ex);

Check failure

Code scanning / CodeQL

Insertion of sensitive information into log files

This [potentially sensitive information](1) is written to a log file.
} else {
externalPath = externalizer.publishLink(resourceResolver, path);
LOG.warn(message, path, ex.getMessage());

Check failure

Code scanning / CodeQL

Insertion of sensitive information into log files

This [potentially sensitive information](1) is written to a log file.
}
} catch (Exception e) {
externalPath = path;
}
return externalPath;
Expand Down
Expand Up @@ -15,19 +15,12 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
package com.adobe.cq.wcm.core.components.internal.link;

import java.io.UnsupportedEncodingException;
import java.math.BigInteger;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.security.SecureRandom;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.HashMap;
import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.tuple.ImmutablePair;
Expand Down Expand Up @@ -60,7 +53,6 @@
public class LinkBuilderImpl implements LinkBuilder {

private static final Logger LOGGER = LoggerFactory.getLogger(LinkBuilderImpl.class);
private final static List<Pattern> PATTERNS = Collections.singletonList(Pattern.compile("(<%[=@].*?%>)"));
public static final String HTML_EXTENSION = ".html";

SlingHttpServletRequest request;
Expand Down Expand Up @@ -164,10 +156,7 @@ public LinkBuilderImpl(String url, SlingHttpServletRequest req, List<PathProcess
try {
// The link contain character sequences that are not well formatted and cannot be decoded, for example
// Adobe Campaign expressions like: /content/path/to/page.html?recipient=<%= recipient.id %>
Map<String, String> placeholders = new LinkedHashMap<>();
String maskedPath = mask(path, placeholders);
maskedPath = URLDecoder.decode(maskedPath, StandardCharsets.UTF_8.name());
path = unmask(maskedPath, placeholders);
path = LinkMaskingUtil.maskAndDo(path, maskedPath -> URLDecoder.decode(maskedPath, StandardCharsets.UTF_8.name()));
} catch (Exception ex) {
String message = "Failed to decode url '{}': {}";
if (LOGGER.isDebugEnabled()) {
Expand Down Expand Up @@ -310,72 +299,6 @@ private Pair<Page, String> resolvePage(@NotNull final Page page) {
return new ImmutablePair<>(resolved, getPageLinkURL(resolved));
}

/**
* Masks a given {@link String} by replacing all occurrences of {@link LinkBuilderImpl#PATTERNS} with a placeholder.
* The generated placeholders are put into the given {@link Map} and can be used to unmask a {@link String} later on.
* <p>
* For example the given original {@link String} {@code /path/to/page.html?r=<%= recipient.id %>} will be transformed to
* {@code /path/to/page.html?r=_abcd_} and the placeholder with the expression will be put into the given {@link Map}.
*
* @param original the original {@link String}
* @param placeholders a {@link Map} the generated placeholders will be put in
* @return the masked {@link String}
* @see LinkBuilderImpl#unmask(String, Map)
*/
private static String mask(String original, Map<String, String> placeholders) {
String masked = original;
for (Pattern pattern : PATTERNS) {
Matcher matcher = pattern.matcher(masked);
while (matcher.find()) {
String expression = matcher.group(1);
String placeholder = newPlaceholder(masked);
masked = masked.replaceFirst(Pattern.quote(expression), placeholder);
placeholders.put(placeholder, expression);
}
}
return masked;
}

/**
* Unmasks the given {@link String} by replacing the given placeholders with their original value.
* <p>
* For example the given masked {@link String} {@code /path/to/page.html?r=_abcd_} will be transformed to
* {@code /path/to/page.html?r=<%= recipient.id %>} by replacing each of the given {@link Map}s keys with the corresponding value.
*
* @param masked the masked {@link String}
* @param placeholders the {@link Map} of placeholders to replace
* @return the unmasked {@link String}
*/
private static String unmask(String masked, Map<String, String> placeholders) {
String unmasked = masked;
for (Map.Entry<String, String> placeholder : placeholders.entrySet()) {
unmasked = unmasked.replaceFirst(placeholder.getKey(), placeholder.getValue());
}
return unmasked;
}

/**
* Generate a new random placeholder that is not conflicting with any character sequence in the given {@link String}.
* <p>
* For example the given {@link String} {@code "foo"} a new random {@link String} will be returned that is not contained in the
* given {@link String}. In this example the following {@link String}s will never be returned "f", "fo", "foo", "o", "oo".
*
* @param str the given {@link String}
* @return the placeholder name
*/
private static String newPlaceholder(String str) {
SecureRandom random = new SecureRandom();
StringBuilder placeholderBuilder = new StringBuilder(5);

do {
placeholderBuilder.setLength(0);
placeholderBuilder
.append("_")
.append(new BigInteger(16, random).toString(16))
.append("_");
} while (str.contains(placeholderBuilder));

return placeholderBuilder.toString();
}

}
@@ -0,0 +1,131 @@
/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~ Copyright 2022 Adobe
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
package com.adobe.cq.wcm.core.components.internal.link;

import java.math.BigInteger;
import java.security.SecureRandom;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

class LinkMaskingUtil {

private final static List<Pattern> PATTERNS = Collections.singletonList(Pattern.compile("(<%[=@].*?%>)"));

private LinkMaskingUtil() {
super();
}

/**
* This functional interface is used to pass a callable to {@link LinkMaskingUtil#maskAndDo(String, Fn)} which has the semantics of a
* {@link java.util.function.Function} but allows to throw a checked exception.
*
* @param <T>
* @param <R>
*/
@FunctionalInterface
interface Fn<T, R> {
R apply(T t) throws Exception;
}

/**
* Masks the given {@link String}, calls the given {@link Fn} with it and unmasks the returned {@link String}.
*
* @param original a {@link String} to be masked and passed to the callable
* @param callable the {@link Fn} to process the masked original
* @return the unmasked return value of the given callable
* @throws Exception when the given callable throws ans exception
*/
static String maskAndDo(String original, Fn<String, String> callable) throws Exception {
// The link contain character sequences that are not well formatted and cannot be decoded, for example
// Adobe Campaign expressions like: /content/path/to/page.html?recipient=<%= recipient.id %>
Map<String, String> placeholders = new LinkedHashMap<>();
String masked = mask(original, placeholders);
masked = callable.apply(masked);
return unmask(masked, placeholders);
}

/**
* Masks a given {@link String} by replacing all occurrences of {@link LinkMaskingUtil#PATTERNS} with a placeholder.
* The generated placeholders are put into the given {@link Map} and can be used to unmask a {@link String} later on.
* <p>
* For example the given original {@link String} {@code /path/to/page.html?r=<%= recipient.id %>} will be transformed to
* {@code /path/to/page.html?r=_abcd_} and the placeholder with the expression will be put into the given {@link Map}.
*
* @param original the original {@link String}
* @param placeholders a {@link Map} the generated placeholders will be put in
* @return the masked {@link String}
* @see LinkMaskingUtil#unmask(String, Map)
*/
private static String mask(String original, Map<String, String> placeholders) {
String masked = original;
for (Pattern pattern : PATTERNS) {
Matcher matcher = pattern.matcher(masked);
while (matcher.find()) {
String expression = matcher.group(1);
String placeholder = newPlaceholder(masked);
masked = masked.replaceFirst(Pattern.quote(expression), placeholder);
placeholders.put(placeholder, expression);
}
}
return masked;
}

/**
* Unmasks the given {@link String} by replacing the given placeholders with their original value.
* <p>
* For example the given masked {@link String} {@code /path/to/page.html?r=_abcd_} will be transformed to
* {@code /path/to/page.html?r=<%= recipient.id %>} by replacing each of the given {@link Map}s keys with the corresponding value.
*
* @param masked the masked {@link String}
* @param placeholders the {@link Map} of placeholders to replace
* @return the unmasked {@link String}
*/
private static String unmask(String masked, Map<String, String> placeholders) {
String unmasked = masked;
for (Map.Entry<String, String> placeholder : placeholders.entrySet()) {
unmasked = unmasked.replaceFirst(placeholder.getKey(), placeholder.getValue());
}
return unmasked;
}

/**
* Generate a new random placeholder that is not conflicting with any character sequence in the given {@link String}.
* <p>
* For example the given {@link String} {@code "foo"} a new random {@link String} will be returned that is not contained in the
* given {@link String}. In this example the following {@link String}s will never be returned "f", "fo", "foo", "o", "oo".
*
* @param str the given {@link String}
* @return the placeholder name
*/
private static String newPlaceholder(String str) {
SecureRandom random = new SecureRandom();
StringBuilder placeholderBuilder = new StringBuilder(5);

do {
placeholderBuilder.setLength(0);
placeholderBuilder
.append("_")
.append(new BigInteger(16, random).toString(16))
.append("_");
} while (str.contains(placeholderBuilder));

return placeholderBuilder.toString();
}
}
Expand Up @@ -20,6 +20,9 @@
import org.apache.sling.servlethelpers.MockSlingHttpServletRequest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.AdditionalAnswers;
import org.mockito.ArgumentCaptor;
import org.mockito.internal.stubbing.answers.ReturnsArgumentAt;

import com.adobe.cq.wcm.core.components.context.CoreComponentTestContext;
import com.day.cq.commons.Externalizer;
Expand All @@ -30,10 +33,13 @@

import static com.adobe.cq.wcm.core.components.internal.link.LinkBuilderImpl.HTML_EXTENSION;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.RETURNS_DEFAULTS;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ExtendWith({AemContextExtension.class})
Expand All @@ -47,12 +53,28 @@ class DefaultPathProcessorTest {
void testExternalizeWithException() {
Externalizer externalizer = mock(Externalizer.class);
when(externalizer.publishLink(any(ResourceResolver.class), anyString())).thenThrow(IllegalArgumentException.class);
localContext.registerService(externalizer);
DefaultPathProcessor underTest = localContext.registerService(new DefaultPathProcessor());
localContext.registerService(Externalizer.class, externalizer);
DefaultPathProcessor underTest = localContext.registerInjectActivateService(new DefaultPathProcessor());
String path = PATH;
assertEquals(path, underTest.externalize(path, localContext.request()));
}

@Test
void testExternalizerIsCalledWithMaskedUrl() {
ArgumentCaptor<String> url = ArgumentCaptor.forClass(String.class);
Externalizer externalizer = mock(Externalizer.class);
when(externalizer.publishLink(any(ResourceResolver.class), anyString())).then(AdditionalAnswers.returnsSecondArg());
localContext.registerService(Externalizer.class, externalizer);
DefaultPathProcessor underTest = localContext.registerInjectActivateService(new DefaultPathProcessor());
String path = PATH + ".html?recipient=<%= recipient.id %>";

assertEquals(path, underTest.externalize(path, localContext.request()));

verify(externalizer).publishLink(any(ResourceResolver.class), url.capture());
assertFalse(url.getValue().contains("<%="));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would feel more confident if we asserted on the expected output and not on simply not having the original fragments. This would also catch situation where we might end up up with multiple masking/encoding.

assertFalse(url.getValue().contains("%>"));
}

@Test
void testMappingWithException() {
ResourceResolver resourceResolver = mock(ResourceResolver.class);
Expand Down