Skip to content

Commit 8f4f590

Browse files
committed
Fix multivalue DS component property reading
Allow more tenant-aware OSGi services by default
1 parent 32d3aa8 commit 8f4f590

File tree

2 files changed

+74
-51
lines changed

2 files changed

+74
-51
lines changed

src/main/java/biz/netcentric/osgi/bnd/NamespaceValidatorsPlugin.java

Lines changed: 69 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,15 @@
1818

1919
import java.io.IOException;
2020
import java.io.InputStream;
21+
import java.util.ArrayList;
2122
import java.util.Arrays;
2223
import java.util.Collection;
2324
import java.util.LinkedList;
25+
import java.util.List;
2426
import java.util.Map;
2527
import java.util.Map.Entry;
28+
import java.util.Objects;
29+
import java.util.StringTokenizer;
2630
import java.util.regex.Pattern;
2731
import java.util.stream.Collectors;
2832

@@ -107,13 +111,19 @@ public class NamespaceValidatorsPlugin implements VerifierPlugin, Plugin {
107111
private static final Collection<Pattern> ALLOWED_TENANT_SPECIFIC_SERVICES;
108112

109113
static {
114+
// list those service interfaces which fully support multi-tenancy or are known to almost never clash
110115
ALLOWED_TENANT_SPECIFIC_SERVICES = new LinkedList<>();
111-
// TODO: conditionally add depending on the multi-tenancy property is being validated
112116
SERVLET_INTERFACES.forEach(
113117
iface -> ALLOWED_TENANT_SPECIFIC_SERVICES.add(Pattern.compile(Pattern.quote(iface))));
114118
FILTER_INTERFACES.forEach(iface -> ALLOWED_TENANT_SPECIFIC_SERVICES.add(Pattern.compile(Pattern.quote(iface))));
115119
ALLOWED_TENANT_SPECIFIC_SERVICES.add(
116120
Pattern.compile(Pattern.quote("org.apache.sling.api.adapter.AdapterFactory")));
121+
ALLOWED_TENANT_SPECIFIC_SERVICES.add(
122+
Pattern.compile(Pattern.quote("org.apache.sling.rewriter.TransformerFactory")));
123+
ALLOWED_TENANT_SPECIFIC_SERVICES.add(
124+
Pattern.compile(Pattern.quote("com.adobe.granite.workflow.exec.WorkflowProcess")));
125+
ALLOWED_TENANT_SPECIFIC_SERVICES.add(
126+
Pattern.compile(Pattern.quote("com.day.cq.workflow.exec.WorkflowProcess")));
117127
ALLOWED_TENANT_SPECIFIC_SERVICES.add(Pattern.compile(Pattern.quote(AUTHENTICATION_HANDLER_INTERFACE)));
118128
}
119129

@@ -475,12 +485,12 @@ private void validateDSComponentXML(String path, InputStream xmlStream)
475485
boolean isAuthenticationHandlerComponent =
476486
isComponentImplementingInterface(root, AUTHENTICATION_HANDLER_INTERFACE);
477487

478-
Map<String, String> properties = getComponentProperties(root);
488+
Map<String, Collection<String>> properties = getComponentProperties(root);
479489

480490
// Validate service interfaces if pattern is configured
481491
if (config.allowedServiceClassPatterns() != null
482492
&& !config.allowedServiceClassPatterns().isEmpty()) {
483-
validateServiceInterfaces(componentName, root);
493+
validateServiceProviders(componentName, root);
484494
}
485495

486496
// Validate Sling servlet properties if this is a servlet component and patterns are configured
@@ -519,9 +529,9 @@ private boolean isComponentImplementingInterface(Element componentElement, Strin
519529
}
520530

521531
/**
522-
* Validates service interfaces against the configured pattern.
532+
* Validates service provider classes against the configured patterns.
523533
*/
524-
private void validateServiceInterfaces(String componentName, Element componentElement) {
534+
private void validateServiceProviders(String componentName, Element componentElement) {
525535
NodeList serviceElements = componentElement.getElementsByTagName(DS_SERVICE_ELEMENT);
526536
for (int i = 0; i < serviceElements.getLength(); i++) {
527537
Element serviceElement = (Element) serviceElements.item(i);
@@ -548,33 +558,47 @@ private void validateServiceInterfaces(String componentName, Element componentEl
548558
}
549559

550560
/**
551-
* Retrieves all properties of a DS component as a map of property name to value.
552-
* Only handles properties with a 'value' attribute (not text content or multi-valued properties).
561+
* Retrieves all properties of a DS component as a map of property name to collection of values.
562+
* Handles properties with a 'value' attribute (comma-separated values) and supports multi-valued properties.
553563
*/
554-
private Map<String, String> getComponentProperties(Element componentElement) {
555-
Map<String, String> properties = new java.util.HashMap<>();
564+
private Map<String, Collection<String>> getComponentProperties(Element componentElement) {
565+
Map<String, Collection<String>> properties = new java.util.HashMap<>();
556566
NodeList propertyElements = componentElement.getElementsByTagName(DS_PROPERTY_ELEMENT);
557567
for (int i = 0; i < propertyElements.getLength(); i++) {
558568
Element propertyElement = (Element) propertyElements.item(i);
559569
String propertyName = propertyElement.getAttribute(DS_PROPERTY_NAME_ATTRIBUTE);
560-
String propertyValue = propertyElement.getAttribute(DS_PROPERTY_VALUE_ATTRIBUTE);
561-
if (propertyName != null && !propertyName.isEmpty() && propertyValue != null && !propertyValue.isEmpty()) {
562-
properties.put(propertyName, propertyValue);
570+
Objects.requireNonNull(propertyName, "Property name in DS component cannot be null");
571+
String propertyValue = null;
572+
if (propertyElement.hasAttribute(DS_PROPERTY_VALUE_ATTRIBUTE)) {
573+
propertyValue = propertyElement.getAttribute(DS_PROPERTY_VALUE_ATTRIBUTE);
574+
}
575+
List<String> valueList = new ArrayList<>();
576+
if (propertyValue != null) {
577+
valueList.add(propertyValue);
578+
} else {
579+
// If no 'value' attribute, check for text content (could be multi-line)
580+
StringTokenizer tokener = new StringTokenizer(propertyElement.getTextContent(), "\r\n");
581+
while (tokener.hasMoreTokens()) {
582+
String value = tokener.nextToken().trim();
583+
if (!value.isEmpty()) {
584+
valueList.add(value);
585+
}
586+
}
563587
}
588+
properties.put(propertyName, valueList);
564589
}
565590
return properties;
566591
}
567592

568593
/**
569594
* Validates servlet properties against configured patterns considering both Sling servlets and OSGi HTTP (Servlet) Whiteboard servlets.
570595
*/
571-
private void validateServletProperties(String componentName, Map<String, String> properties) {
596+
private void validateServletProperties(String componentName, Map<String, Collection<String>> properties) {
572597
// Validate sling.servlet.paths
573598
if (properties.containsKey(SLING_SERVLET_PATHS)
574599
&& config.allowedSlingServletPathsPatterns() != null
575600
&& !config.allowedSlingServletPathsPatterns().isEmpty()) {
576-
String[] paths = properties.get(SLING_SERVLET_PATHS).split(",");
577-
for (String path : paths) {
601+
for (String path : properties.get(SLING_SERVLET_PATHS)) {
578602
String trimmedPath = path.trim();
579603
if (config.allowedSlingServletPathsPatterns().stream()
580604
.noneMatch(pattern -> pattern.matcher(trimmedPath).matches())) {
@@ -591,9 +615,7 @@ private void validateServletProperties(String componentName, Map<String, String>
591615
// Validate sling.servlet.resourceTypes
592616
if (properties.containsKey(SLING_SERVLET_RESOURCE_TYPES)
593617
&& config.allowedSlingServletResourceTypesPatterns() != null) {
594-
String[] resourceTypes =
595-
properties.get(SLING_SERVLET_RESOURCE_TYPES).split(",");
596-
for (String resourceType : resourceTypes) {
618+
for (String resourceType : properties.get(SLING_SERVLET_RESOURCE_TYPES)) {
597619
String trimmedResourceType = resourceType.trim();
598620
if (config.allowedSlingServletResourceTypesPatterns().stream()
599621
.noneMatch(
@@ -611,48 +633,48 @@ private void validateServletProperties(String componentName, Map<String, String>
611633
// Validate sling.servlet.resourceSuperType
612634
if (properties.containsKey(SLING_SERVLET_RESOURCE_SUPER_TYPE)
613635
&& config.allowedSlingServletResourceSuperTypePatterns() != null) {
614-
String propertyValue = properties.get(SLING_SERVLET_RESOURCE_SUPER_TYPE);
615-
if (config.allowedSlingServletResourceSuperTypePatterns().stream()
616-
.noneMatch(pattern -> pattern.matcher(propertyValue).matches())) {
617-
reporter.error(
618-
"Sling servlet component \"%s\" has resource super type \"%s\" which does not match any of the allowed patterns [%s]",
619-
componentName,
620-
propertyValue,
621-
config.allowedSlingServletResourceSuperTypePatterns().stream()
622-
.map(Pattern::pattern)
623-
.collect(Collectors.joining(",")));
636+
for (String propertyValue : properties.get(SLING_SERVLET_RESOURCE_SUPER_TYPE)) {
637+
if (config.allowedSlingServletResourceSuperTypePatterns().stream()
638+
.noneMatch(pattern -> pattern.matcher(propertyValue).matches())) {
639+
reporter.error(
640+
"Sling servlet component \"%s\" has resource super type \"%s\" which does not match any of the allowed patterns [%s]",
641+
componentName,
642+
propertyValue,
643+
config.allowedSlingServletResourceSuperTypePatterns().stream()
644+
.map(Pattern::pattern)
645+
.collect(Collectors.joining(",")));
646+
}
624647
}
625648
}
626649
// Validate osgi.http.whiteboard.servlet.pattern
627650
if (properties.containsKey(HTTP_WHITEBOARD_SERVLET_PATTERN)
628651
&& config.allowedHttpWhiteboardServletPatternPatterns() != null
629652
&& !config.allowedHttpWhiteboardServletPatternPatterns().isEmpty()) {
630-
String propertyValue = properties.get(HTTP_WHITEBOARD_SERVLET_PATTERN);
631-
if (config.allowedHttpWhiteboardServletPatternPatterns().stream()
632-
.noneMatch(pattern -> pattern.matcher(propertyValue).matches())) {
633-
reporter.error(
634-
"Servlet component \"%s\" has OSGi HTTP/Servlet whiteboard servlet pattern \"%s\" which does not match any of the allowed patterns [%s]",
635-
componentName,
636-
propertyValue,
637-
config.allowedHttpWhiteboardServletPatternPatterns().stream()
638-
.map(Pattern::pattern)
639-
.collect(Collectors.joining(",")));
653+
for (String propertyValue : properties.get(HTTP_WHITEBOARD_SERVLET_PATTERN)) {
654+
if (config.allowedHttpWhiteboardServletPatternPatterns().stream()
655+
.noneMatch(pattern -> pattern.matcher(propertyValue).matches())) {
656+
reporter.error(
657+
"Servlet component \"%s\" has OSGi HTTP/Servlet whiteboard servlet pattern \"%s\" which does not match any of the allowed patterns [%s]",
658+
componentName,
659+
propertyValue,
660+
config.allowedHttpWhiteboardServletPatternPatterns().stream()
661+
.map(Pattern::pattern)
662+
.collect(Collectors.joining(",")));
663+
}
640664
}
641665
}
642666
}
643667

644668
/**
645669
* Validates AuthenticationHandler path against configured patterns.
646670
*/
647-
private void validateAuthenticationHandlerPath(String componentName, Map<String, String> properties) {
671+
private void validateAuthenticationHandlerPath(String componentName, Map<String, Collection<String>> properties) {
648672
if (config.allowedSlingAuthenticationHandlerPathPatterns() == null
649673
|| config.allowedSlingAuthenticationHandlerPathPatterns().isEmpty()) {
650674
return;
651675
}
652676
if (properties.containsKey(AUTH_HANDLER_PATH_PROPERTY)) {
653-
String propertyValue = properties.get(AUTH_HANDLER_PATH_PROPERTY);
654-
String[] paths = propertyValue.split(",");
655-
for (String path : paths) {
677+
for (String path : properties.get(AUTH_HANDLER_PATH_PROPERTY)) {
656678
String trimmedPath = path.trim();
657679
if (config.allowedSlingAuthenticationHandlerPathPatterns().stream()
658680
.noneMatch(pattern -> pattern.matcher(trimmedPath).matches())) {
@@ -671,13 +693,12 @@ private void validateAuthenticationHandlerPath(String componentName, Map<String,
671693
/**
672694
* Validates filter patterns for Sling and OSGi HTTP/Servlet Whiteboard filters.
673695
*/
674-
private void validateFilterPatterns(String componentName, Map<String, String> properties) {
696+
private void validateFilterPatterns(String componentName, Map<String, Collection<String>> properties) {
675697
// Validate sling.filter.pattern
676698
if (properties.containsKey(SLING_FILTER_PATTERN)
677699
&& config.allowedSlingFilterPatternPatterns() != null
678700
&& !config.allowedSlingFilterPatternPatterns().isEmpty()) {
679-
String[] patterns = properties.get(SLING_FILTER_PATTERN).split(",");
680-
for (String pattern : patterns) {
701+
for (String pattern : properties.get(SLING_FILTER_PATTERN)) {
681702
String trimmedPattern = pattern.trim();
682703
if (config.allowedSlingFilterPatternPatterns().stream()
683704
.noneMatch(p -> p.matcher(trimmedPattern).matches())) {
@@ -691,12 +712,11 @@ private void validateFilterPatterns(String componentName, Map<String, String> pr
691712
}
692713
}
693714
}
694-
// Validate sling.filter.pattern
715+
// Validate sling.filter.resourceTypes
695716
if (properties.containsKey(SLING_FILTER_RESOURCE_TYPES)
696717
&& config.allowedSlingFilterResourceTypesPatterns() != null
697718
&& !config.allowedSlingFilterResourceTypesPatterns().isEmpty()) {
698-
String[] patterns = properties.get(SLING_FILTER_RESOURCE_TYPES).split(",");
699-
for (String pattern : patterns) {
719+
for (String pattern : properties.get(SLING_FILTER_RESOURCE_TYPES)) {
700720
String trimmedPattern = pattern.trim();
701721
if (config.allowedSlingFilterResourceTypesPatterns().stream()
702722
.noneMatch(p -> p.matcher(trimmedPattern).matches())) {
@@ -714,8 +734,7 @@ private void validateFilterPatterns(String componentName, Map<String, String> pr
714734
if (properties.containsKey(HTTP_WHITEBOARD_FILTER_PATTERN)
715735
&& config.allowedHttpWhiteboardFilterPatternPatterns() != null
716736
&& !config.allowedHttpWhiteboardFilterPatternPatterns().isEmpty()) {
717-
String[] patterns = properties.get(HTTP_WHITEBOARD_FILTER_PATTERN).split(",");
718-
for (String pattern : patterns) {
737+
for (String pattern : properties.get(HTTP_WHITEBOARD_FILTER_PATTERN)) {
719738
String trimmedPattern = pattern.trim();
720739
if (config.allowedHttpWhiteboardFilterPatternPatterns().stream()
721740
.noneMatch(p -> p.matcher(trimmedPattern).matches())) {

src/test/java/biz/netcentric/osgi/bnd/NamespaceValidatorsPluginTest.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ void testDSComponentServiceValidation_WildcardPattern_StarWildcard() throws Exce
241241
+ " <implementation class=\"com.mycompany.impl.InvalidComponentImpl\"/>\n"
242242
+ " <service>\n"
243243
+ " <provide interface=\"org.apache.sling.api.SlingService\"/>\n"
244+
+ " <provide interface=\"org.apache.sling.rewriter.TransformerFactory\"/>\n"
244245
+ " </service>\n"
245246
+ "</component>";
246247

@@ -682,7 +683,10 @@ void testAuthenticationHandlerPath_MultiplePaths() throws Exception {
682683
+ " <service>\n"
683684
+ " <provide interface=\"org.apache.sling.auth.core.spi.AuthenticationHandler\"/>\n"
684685
+ " </service>\n"
685-
+ " <property name=\"path\" value=\"/auth,/notallowed\"/>\n"
686+
+ " <property name=\"path\">\n"
687+
+ " /auth\n"
688+
+ " /notallowed\n"
689+
+ " </property>\n"
686690
+ "</component>";
687691
jar.putResource("OSGI-INF/AuthHandler.xml", new EmbeddedResource(dsXml.getBytes(), 0));
688692

0 commit comments

Comments
 (0)