Skip to content

Commit

Permalink
SITES-15109 - RFH - Applying Async in clientlibraries not working on …
Browse files Browse the repository at this point in the history
…customer instance (#2570)

* added request attribute based injection for categories and resourceTypes fields as a safety mechanism to reduce the risk of unintended injections on these two fields
 * updated unit tests

Co-authored-by: Vlad <vlad@bailescu.ro>
Co-authored-by: bcenusa <bcenusa@adobe.com>
  • Loading branch information
3 people committed Aug 17, 2023
1 parent 342bbbd commit e8fc23a
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 7 deletions.
Expand Up @@ -47,6 +47,7 @@
import org.apache.sling.models.annotations.DefaultInjectionStrategy;
import org.apache.sling.models.annotations.Model;
import org.apache.sling.models.annotations.injectorspecific.OSGiService;
import org.apache.sling.models.annotations.injectorspecific.RequestAttribute;
import org.apache.sling.models.annotations.injectorspecific.Self;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
Expand Down Expand Up @@ -81,6 +82,18 @@ public class ClientLibrariesImpl implements ClientLibraries {
@Named(OPTION_RESOURCE_TYPES)
Object resourceTypes;

@Inject
@Named(OPTION_CATEGORIES)
private Object categories;

// The two fields bellow are injected from request attributes only to be able to detect unintended injections on
// the corresponding fields above.
@RequestAttribute(name = OPTION_RESOURCE_TYPES)
private Object raResourceTypes;

@RequestAttribute(name = OPTION_CATEGORIES)
private Object raCategories;

@Inject
@Named(OPTION_FILTER_REGEX)
String filterRegex;
Expand All @@ -90,18 +103,12 @@ public class ClientLibrariesImpl implements ClientLibraries {
@Default(booleanValues = OPTION_INHERITED_DEFAULT)
boolean inherited;

@Inject
@Named(OPTION_CATEGORIES)
private Object categories;

@Inject
@Named(OPTION_ASYNC)
@Nullable
private boolean async;

@Inject
@Named(OPTION_DEFER)
@Nullable
private boolean defer;

@Inject
Expand Down Expand Up @@ -132,13 +139,19 @@ public class ClientLibrariesImpl implements ClientLibraries {
@PostConstruct
protected void initModel() {
resourceTypeSet = Utils.getStrings(resourceTypes);
if (resourceTypeSet.isEmpty()) {
resourceTypeSet = Utils.getStrings(raResourceTypes);
}
if (StringUtils.isNotEmpty(filterRegex)) {
pattern = Pattern.compile(filterRegex);
}

Set<String> categoriesSet = Utils.getStrings(categories);
if (categoriesSet.isEmpty()) {
categoriesSet = getCategoriesFromComponents();
categoriesSet = Utils.getStrings(raCategories);
if (categoriesSet.isEmpty()) {
categoriesSet = getCategoriesFromComponents();
}
}

categoriesArray = categoriesSet.toArray(new String[0]);
Expand Down
Expand Up @@ -34,6 +34,7 @@
import org.apache.sling.api.resource.ResourceResolverFactory;
import org.apache.sling.api.scripting.SlingBindings;
import org.apache.sling.models.factory.ModelFactory;
import org.apache.sling.testing.mock.sling.servlet.MockSlingHttpServletRequest;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand Down Expand Up @@ -355,6 +356,47 @@ void testGetCategoriesWithInjectedCategories() {
assertEquals(includes.toString(), clientlibs.getJsAndCssIncludes());
}

@Test
void testGetCategoriesWithAdverseInjectedCategories() {
// invalid injection
Map<String,Object> slingAttributes = new HashMap<>();
slingAttributes.put(ClientLibraries.OPTION_CATEGORIES, new Object());

// valid injection
Map<String,Object> requestAttributes = new HashMap<>();
requestAttributes.put(ClientLibraries.OPTION_CATEGORIES, TEASER_CATEGORY + "," + ACCORDION_CATEGORY);

ClientLibraries clientlibs = getClientLibrariesUnderTest(ROOT_PAGE, slingAttributes, requestAttributes);

//valid injection should be used
StringBuilder includes = new StringBuilder();
includes.append(jsIncludes.get(TEASER_CATEGORY));
includes.append(jsIncludes.get(ACCORDION_CATEGORY));
includes.append(cssIncludes.get(TEASER_CATEGORY));
includes.append(cssIncludes.get(ACCORDION_CATEGORY));
assertEquals(includes.toString(), clientlibs.getJsAndCssIncludes());
}

@Test
void testGetCategoriesWithAdverseInjectedCategoriesBackwardsCompatibility() {
// original honored injection
Map<String,Object> slingAttributes = new HashMap<>();
slingAttributes.put(ClientLibraries.OPTION_CATEGORIES, TEASER_CATEGORY + "," + ACCORDION_CATEGORY);

// new ignored injection
Map<String,Object> requestAttributes = new HashMap<>();
requestAttributes.put(ClientLibraries.OPTION_CATEGORIES, TEASER_CATEGORY);

ClientLibraries clientlibs = getClientLibrariesUnderTest(ROOT_PAGE, slingAttributes, requestAttributes);

StringBuilder includes = new StringBuilder();
includes.append(jsIncludes.get(TEASER_CATEGORY));
includes.append(jsIncludes.get(ACCORDION_CATEGORY));
includes.append(cssIncludes.get(TEASER_CATEGORY));
includes.append(cssIncludes.get(ACCORDION_CATEGORY));
assertEquals(includes.toString(), clientlibs.getJsAndCssIncludes());
}

@Test
void testGetCategoriesWithInjectedResourceType() {
Map<String,Object> attributes = new HashMap<>();
Expand All @@ -368,6 +410,44 @@ void testGetCategoriesWithInjectedResourceType() {
assertEquals(includes.toString(), clientlibs.getJsAndCssIncludes());
}

@Test
void testGetCategoriesWithAdverseInjectedResourceType() {
// invalid injection
Map<String,Object> slingAttributes = new HashMap<>();
slingAttributes.put(ClientLibraries.OPTION_RESOURCE_TYPES, new Object());
// valid injection
Map<String,Object> requestAttributes = new HashMap<>();
requestAttributes.put(ClientLibraries.OPTION_RESOURCE_TYPES, new HashSet<String>() {{
add("core/wcm/components/accordion/v1/accordion");
}});
ClientLibraries clientlibs = getClientLibrariesUnderTest(ROOT_PAGE, slingAttributes, requestAttributes);
StringBuilder includes = new StringBuilder();
includes.append(jsIncludes.get(ACCORDION_CATEGORY));
includes.append(cssIncludes.get(ACCORDION_CATEGORY));
assertEquals(includes.toString(), clientlibs.getJsAndCssIncludes());
}

@Test
void testGetCategoriesWithAdverseInjectedResourceTypeBackwardsCompatibility() {
// original honored injection
Map<String,Object> slingAttributes = new HashMap<>();
slingAttributes.put(ClientLibraries.OPTION_RESOURCE_TYPES, new HashSet<String>() {{
add("core/wcm/components/accordion/v1/accordion");
}});
// new ignored injection
Map<String,Object> requestAttributes = new HashMap<>();
requestAttributes.put(ClientLibraries.OPTION_RESOURCE_TYPES, new HashSet<String>() {{
add("core/wcm/components/accordion/v1/accordion");
add("core/wcm/components/carousel/v3/carousel");
}});

ClientLibraries clientlibs = getClientLibrariesUnderTest(ROOT_PAGE, slingAttributes, requestAttributes);
StringBuilder includes = new StringBuilder();
includes.append(jsIncludes.get(ACCORDION_CATEGORY));
includes.append(cssIncludes.get(ACCORDION_CATEGORY));
assertEquals(includes.toString(), clientlibs.getJsAndCssIncludes());
}

@Test
void testGetCategoriesWithInjectedResourceTypesAndInheritance() {
Map<String,Object> attributes = new HashMap<>();
Expand Down Expand Up @@ -528,4 +608,24 @@ private ClientLibraries getClientLibrariesUnderTest(String path, Map<String,Obje
return null;
}

private ClientLibraries getClientLibrariesUnderTest(String path, Map<String,Object> slingAttributes, Map<String,Object> requestAttributes) {
Resource resource = context.currentResource(path);
if (resource != null) {
MockSlingHttpServletRequest request = context.request();
if (slingAttributes != null) {
SlingBindings slingBindings = (SlingBindings) request.getAttribute(SlingBindings.class.getName());
for (Map.Entry<String,Object> entry : slingAttributes.entrySet()) {
slingBindings.put(entry.getKey(), entry.getValue());
}
request.setAttribute(SlingBindings.class.getName(), slingBindings);
for (Map.Entry<String,Object> entry : requestAttributes.entrySet()) {
request.setAttribute(entry.getKey(), entry.getValue());
}
}
request.setResource(resource);
return request.adaptTo(ClientLibraries.class);
}
return null;
}

}

0 comments on commit e8fc23a

Please sign in to comment.