You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2022/02/04 10:05:53 UTC
[sling-org-apache-sling-security] branch master updated: SLING-11121 : Avoid registering service in activate method
This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-security.git
The following commit(s) were added to refs/heads/master by this push:
new 2e2f735 SLING-11121 : Avoid registering service in activate method
2e2f735 is described below
commit 2e2f7350e3fb8a9a9f25a2db0d001b9b062081fa
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Fri Feb 4 11:05:43 2022 +0100
SLING-11121 : Avoid registering service in activate method
---
.../security/impl/ContentDispositionFilter.java | 19 +++--
.../apache/sling/security/impl/ReferrerFilter.java | 98 ++++++++--------------
.../impl/ContentDispositionFilterTest.java | 30 +++----
.../sling/security/impl/ReferrerFilterTest.java | 15 +---
4 files changed, 60 insertions(+), 102 deletions(-)
diff --git a/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java b/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java
index 4962495..82d4389 100644
--- a/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java
+++ b/src/main/java/org/apache/sling/security/impl/ContentDispositionFilter.java
@@ -44,7 +44,14 @@ import org.osgi.service.metatype.annotations.Designate;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-@Component(property={"sling.filter.scope=request", "sling.filter.scope=forward", "service.ranking:Integer=25000"})
+@Component(
+ service = Filter.class,
+ property={
+ "sling.filter.scope=request",
+ "sling.filter.scope=forward",
+ "service.ranking:Integer=25000"
+ }
+)
@Designate(ocd=ContentDispositionFilterConfiguration.class)
public class ContentDispositionFilter implements Filter {
@@ -56,21 +63,21 @@ public class ContentDispositionFilter implements Filter {
/**
* Set of paths
*/
- Set<String> contentDispositionPaths;
+ final Set<String> contentDispositionPaths;
/**
* Array of prefixes of paths
*/
- private String[] contentDispositionPathsPfx;
+ private final String[] contentDispositionPathsPfx;
Set<String> contentDispositionExcludedPaths;
- private Map<String, Set<String>> contentTypesMapping;
+ private final Map<String, Set<String>> contentTypesMapping;
- private boolean enableContentDispositionAllPaths;
+ private final boolean enableContentDispositionAllPaths;
@Activate
- private void activate(ContentDispositionFilterConfiguration configuration) {
+ public ContentDispositionFilter(final ContentDispositionFilterConfiguration configuration) {
Set<String> paths = new HashSet<String>();
List<String> pfxs = new ArrayList<String>();
diff --git a/src/main/java/org/apache/sling/security/impl/ReferrerFilter.java b/src/main/java/org/apache/sling/security/impl/ReferrerFilter.java
index 1ca775f..38b3cdd 100644
--- a/src/main/java/org/apache/sling/security/impl/ReferrerFilter.java
+++ b/src/main/java/org/apache/sling/security/impl/ReferrerFilter.java
@@ -27,10 +27,8 @@ import java.net.SocketException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Dictionary;
import java.util.Enumeration;
import java.util.HashSet;
-import java.util.Hashtable;
import java.util.List;
import java.util.Set;
import java.util.regex.Pattern;
@@ -41,12 +39,8 @@ import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.Constants;
-import org.osgi.framework.ServiceRegistration;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
-import org.osgi.service.component.annotations.Deactivate;
import org.osgi.service.http.whiteboard.HttpWhiteboardConstants;
import org.osgi.service.http.whiteboard.Preprocessor;
import org.osgi.service.metatype.annotations.AttributeDefinition;
@@ -58,7 +52,10 @@ import org.slf4j.LoggerFactory;
@Component(
service = Preprocessor.class,
property = {
- HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_SELECT + "=(" + HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_NAME + "=*)"
+ HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_SELECT + "=(" + HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_NAME + "=*)",
+ "felix.webconsole.label=slingreferrerfilter",
+ "felix.webconsole.title=Sling Referrer Filter",
+ "felix.webconsole.configprinter.modes=always"
}
)
@Designate(ocd = ReferrerFilter.Config.class)
@@ -152,21 +149,19 @@ public class ReferrerFilter implements Preprocessor {
/**
* Do we allow empty referrer?
*/
- private boolean allowEmpty;
+ private final boolean allowEmpty;
/** Allowed uri referrers */
- private URL[] allowedUriReferrers;
+ private final URL[] allowedUriReferrers;
/** Allowed regexp referrers */
- private Pattern[] allowedRegexReferrers;
+ private final Pattern[] allowedRegexReferrers;
/** Methods to be filtered. */
- private String[] filterMethods;
+ private final String[] filterMethods;
/** Paths to be excluded */
- private Pattern[] excludedRegexUserAgents;
-
- private ServiceRegistration<Object> configPrinterRegistration;
+ private final Pattern[] excludedRegexUserAgents;
/**
* Create a default list of referrers
@@ -253,7 +248,7 @@ public class ReferrerFilter implements Preprocessor {
}
@Activate
- protected void activate(final BundleContext context, Config config) {
+ public ReferrerFilter(final Config config) {
this.allowEmpty = config.allow_empty();
this.allowedRegexReferrers = createRegexPatterns(config.allow_hosts_regexp());
this.excludedRegexUserAgents = createRegexPatterns(config.exclude_agents_regexp());
@@ -264,40 +259,23 @@ public class ReferrerFilter implements Preprocessor {
}
this.allowedUriReferrers = createReferrerUrls(allowUriReferrers);
- this.filterMethods = config.filter_methods();
- if (this.filterMethods != null
- &&this.filterMethods.length == 1
- && (this.filterMethods[0] == null || this.filterMethods[0].trim().length() == 0)) {
- this.filterMethods = null;
- }
- if ( this.filterMethods != null ) {
- for(int i=0; i<filterMethods.length; i++) {
- filterMethods[i] = filterMethods[i].toUpperCase();
+ String[] methods = config.filter_methods();
+ if ( methods != null ) {
+ final List<String> values = new ArrayList<>();
+ for(final String m : methods) {
+ if ( m != null && m.trim().length() > 0 ) {
+ values.add(m.trim().toUpperCase());
+ }
+ }
+ if ( values.isEmpty() ) {
+ methods = null;
+ } else {
+ methods = values.toArray(new String[values.size()]);
}
}
- this.configPrinterRegistration = registerConfigPrinter(context);
+ this.filterMethods = methods;
}
- @Deactivate
- protected void deactivate() {
- this.configPrinterRegistration.unregister();
- }
-
- private ServiceRegistration<Object> registerConfigPrinter(BundleContext bundleContext) {
- final ConfigurationPrinter cfgPrinter = new ConfigurationPrinter();
- final Dictionary<String, String> serviceProps = new Hashtable<>();
- serviceProps.put(Constants.SERVICE_DESCRIPTION,
- "Apache Sling Referrer Filter Configuration Printer");
- serviceProps.put(Constants.SERVICE_VENDOR, "The Apache Software Foundation");
- serviceProps.put("felix.webconsole.label", "slingreferrerfilter");
- serviceProps.put("felix.webconsole.title", "Sling Referrer Filter");
- serviceProps.put("felix.webconsole.configprinter.modes", "always");
-
- return bundleContext.registerService(Object.class,
- cfgPrinter, serviceProps);
- }
-
-
private boolean isModification(final HttpServletRequest req) {
final String method = req.getMethod();
if ( filterMethods != null ) {
@@ -487,23 +465,19 @@ public class ReferrerFilter implements Preprocessor {
&& !isExcludedRegexUserAgent(userAgent);
}
- public class ConfigurationPrinter {
-
- /**
- * Print out the allowedReferrers
- * @see org.apache.felix.webconsole.ConfigurationPrinter#printConfiguration(java.io.PrintWriter)
- * @param pw the PrintWriter object
- */
- public void printConfiguration(final PrintWriter pw) {
- pw.println("Current Apache Sling Referrer Filter Allowed Referrers:");
- pw.println();
- for (final URL url : allowedUriReferrers) {
- pw.println(url.toString());
- }
- for (final Pattern pattern : allowedRegexReferrers) {
- pw.println(pattern.toString());
- }
+ /**
+ * Print out the allowedReferrers
+ * @see org.apache.felix.webconsole.ConfigurationPrinter#printConfiguration(java.io.PrintWriter)
+ * @param pw the PrintWriter object
+ */
+ public void printConfiguration(final PrintWriter pw) {
+ pw.println("Current Apache Sling Referrer Filter Allowed Referrers:");
+ pw.println();
+ for (final URL url : allowedUriReferrers) {
+ pw.println(url.toString());
+ }
+ for (final Pattern pattern : allowedRegexReferrers) {
+ pw.println(pattern.toString());
}
-
}
}
diff --git a/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java b/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java
index 4c74977..db0c943 100644
--- a/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java
+++ b/src/test/java/org/apache/sling/security/impl/ContentDispositionFilterTest.java
@@ -30,7 +30,6 @@ import org.jmock.Expectations;
import org.jmock.Mockery;
import org.jmock.integration.junit4.JUnit4Mockery;
import org.junit.Assert;
-import org.junit.Before;
import org.junit.Test;
import junitx.util.PrivateAccessor;
@@ -44,11 +43,6 @@ public class ContentDispositionFilterTest {
private static final String JCR_CONTENT_LEAF = "jcr:content";
- @Before
- public void setUp() {
- contentDispositionFilter = new ContentDispositionFilter();
- }
-
/**
* Implementation of the annotation class used for the configuration of the ContentDispositionFilter.
* Unfortunately there is no way to hide the compiler warning: http://stackoverflow.com/a/13261789/5155923
@@ -87,10 +81,6 @@ public class ContentDispositionFilterTest {
}
}
- private void callActivateWithConfiguration(String[] paths) throws Throwable {
- callActivateWithConfiguration(paths, new String[]{});
- }
-
private void callActivateWithConfiguration(String[] paths, String[] excludedPaths) throws Throwable {
callActivateWithConfiguration(paths, excludedPaths, false);
}
@@ -98,7 +88,7 @@ public class ContentDispositionFilterTest {
private void callActivateWithConfiguration(String[] paths, String[] excludedPaths, boolean enableForAllPaths) throws Throwable {
ContentDispositionFilterConfiguration configuration = new Configuration(paths, excludedPaths, enableForAllPaths);
- PrivateAccessor.invoke(contentDispositionFilter,"activate", new Class[]{ContentDispositionFilterConfiguration.class},new Object[]{configuration});
+ contentDispositionFilter = new ContentDispositionFilter(configuration);
}
@SuppressWarnings("unchecked")
@@ -520,7 +510,7 @@ public class ContentDispositionFilterTest {
final SlingHttpServletResponse response = context.mock(SlingHttpServletResponse.class);
final Resource resource = context.mock(Resource.class, "resource" );
final ValueMap properties = context.mock(ValueMap.class);
- contentDispositionFilter = new ContentDispositionFilter();
+
callActivateWithConfiguration(new String[]{"/content/usergenerated:text/html,text/plain"}, new String[]{""});
final AtomicInteger counter = new AtomicInteger();
@@ -793,7 +783,7 @@ public class ContentDispositionFilterTest {
final SlingHttpServletResponse response = context.mock(SlingHttpServletResponse.class);
final Resource resource = context.mock(Resource.class, "resource" );
final ValueMap properties = context.mock(ValueMap.class);
- contentDispositionFilter = new ContentDispositionFilter();
+
callActivateWithConfiguration(new String[]{"/content/usergenerated"}, new String[]{""}, false);
final AtomicInteger counter = new AtomicInteger();
@@ -1173,7 +1163,7 @@ public class ContentDispositionFilterTest {
}
@Test
public void test_isJcrData1() throws Throwable {
- contentDispositionFilter = new ContentDispositionFilter();
+ callActivateWithConfiguration(new String[]{"/content/usergenerated"}, new String[]{"/content/usergenerated"});
final SlingHttpServletRequest request = context.mock(SlingHttpServletRequest.class);
final SlingHttpServletResponse response = context.mock(SlingHttpServletResponse.class);
final Resource resource = null;
@@ -1194,7 +1184,7 @@ public class ContentDispositionFilterTest {
@Test
public void test_isJcrData2() throws Throwable {
- contentDispositionFilter = new ContentDispositionFilter();
+ callActivateWithConfiguration(new String[]{"/content/usergenerated"}, new String[]{"/content/usergenerated"});
final SlingHttpServletRequest request = context.mock(SlingHttpServletRequest.class);
final SlingHttpServletResponse response = context.mock(SlingHttpServletResponse.class);
final Resource resource = context.mock(Resource.class);
@@ -1225,7 +1215,7 @@ public class ContentDispositionFilterTest {
@Test
public void test_isJcrData3() throws Throwable {
- contentDispositionFilter = new ContentDispositionFilter();
+ callActivateWithConfiguration(new String[]{"/content/usergenerated"}, new String[]{"/content/usergenerated"});
final SlingHttpServletRequest request = context.mock(SlingHttpServletRequest.class);
final SlingHttpServletResponse response = context.mock(SlingHttpServletResponse.class);
@@ -1254,7 +1244,7 @@ public class ContentDispositionFilterTest {
@Test
public void test_isJcrData4() throws Throwable {
- contentDispositionFilter = new ContentDispositionFilter();
+ callActivateWithConfiguration(new String[]{"/content/usergenerated"}, new String[]{"/content/usergenerated"});
final SlingHttpServletRequest request = context.mock(SlingHttpServletRequest.class);
final SlingHttpServletResponse response = context.mock(SlingHttpServletResponse.class);
@@ -1289,7 +1279,7 @@ public class ContentDispositionFilterTest {
@Test
public void test_isJcrData5() throws Throwable {
- contentDispositionFilter = new ContentDispositionFilter();
+ callActivateWithConfiguration(new String[]{"/content/usergenerated"}, new String[]{"/content/usergenerated"});
final SlingHttpServletRequest request = context.mock(SlingHttpServletRequest.class);
final SlingHttpServletResponse response = context.mock(SlingHttpServletResponse.class);
@@ -1325,7 +1315,7 @@ public class ContentDispositionFilterTest {
@Test
public void test_isJcrData6() throws Throwable {
- contentDispositionFilter = new ContentDispositionFilter();
+ callActivateWithConfiguration(new String[]{"/content/usergenerated"}, new String[]{"/content/usergenerated"});
final SlingHttpServletRequest request = context.mock(SlingHttpServletRequest.class);
final SlingHttpServletResponse response = context.mock(SlingHttpServletResponse.class);
@@ -1351,7 +1341,7 @@ public class ContentDispositionFilterTest {
@Test
public void test_isJcrData7() throws Throwable {
- contentDispositionFilter = new ContentDispositionFilter();
+ callActivateWithConfiguration(new String[]{"/content/usergenerated"}, new String[]{"/content/usergenerated"});
final SlingHttpServletRequest request = context.mock(SlingHttpServletRequest.class);
final SlingHttpServletResponse response = context.mock(SlingHttpServletResponse.class);
final Resource child = context.mock(Resource.class, "child");
diff --git a/src/test/java/org/apache/sling/security/impl/ReferrerFilterTest.java b/src/test/java/org/apache/sling/security/impl/ReferrerFilterTest.java
index 66e6e45..0eb8fc1 100644
--- a/src/test/java/org/apache/sling/security/impl/ReferrerFilterTest.java
+++ b/src/test/java/org/apache/sling/security/impl/ReferrerFilterTest.java
@@ -16,22 +16,16 @@
*/
package org.apache.sling.security.impl;
-import static org.mockito.Matchers.any;
-import static org.mockito.Mockito.doNothing;
-import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.lang.annotation.Annotation;
-import java.util.Dictionary;
import javax.servlet.http.HttpServletRequest;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.ServiceRegistration;
public class ReferrerFilterTest {
@@ -39,10 +33,6 @@ public class ReferrerFilterTest {
@Before
public void setup() {
- filter = new ReferrerFilter();
- final BundleContext bundleCtx = mock(BundleContext.class);
- final ServiceRegistration reg = mock(ServiceRegistration.class);
-
ReferrerFilter.Config config = new ReferrerFilter.Config() {
@Override
public Class<? extends Annotation> annotationType() {
@@ -77,10 +67,7 @@ public class ReferrerFilterTest {
return new String[]{"[a-zA-Z]*\\/[0-9]*\\.[0-9]*;Some-Agent\\s.*"};
}
};
-
- doReturn(reg).when(bundleCtx).registerService(any(String[].class), any(), any(Dictionary.class));
- doNothing().when(reg).unregister();
- filter.activate(bundleCtx, config);
+ filter = new ReferrerFilter(config);
}
@Test