This is an automated email from the ASF dual-hosted git repository. rombert pushed a commit to annotated tag org.apache.sling.jcr.davex-1.3.6 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-davex.git
commit c35d525ffe1238f9bcd7c3fd1227fbbbc320202d Author: Felix Meschberger <[email protected]> AuthorDate: Mon Dec 12 14:05:20 2016 +0000 SLING-6378 Unclosed ResourceResolver in davex AuthHttpContext * Refactored Servlet to be a whiteboard registered servlet * Refactored AuthHttpContext to be a whiteboard registered ServletContextHelper used by the SlingDavExServlet git-svn-id: https://svn.apache.org/repos/asf/sling/trunk/bundles/jcr/davex@1773793 13f79535-47bb-0310-9956-ffa450edef68 --- pom.xml | 3 +- .../jcr/davex/impl/servlets/AuthHttpContext.java | 104 ++++++++----------- .../jcr/davex/impl/servlets/SlingDavExServlet.java | 111 ++++++++------------- .../davex/impl/servlets/AuthHttpContextTest.java | 38 +++---- 4 files changed, 104 insertions(+), 152 deletions(-) diff --git a/pom.xml b/pom.xml index ad6775b..1b3d925 100644 --- a/pom.xml +++ b/pom.xml @@ -152,7 +152,8 @@ </dependency> <dependency> <groupId>org.osgi</groupId> - <artifactId>org.osgi.compendium</artifactId> + <artifactId>org.osgi.service.http.whiteboard</artifactId> + <version>1.0.0</version> </dependency> <!-- JUnit --> diff --git a/src/main/java/org/apache/sling/jcr/davex/impl/servlets/AuthHttpContext.java b/src/main/java/org/apache/sling/jcr/davex/impl/servlets/AuthHttpContext.java index b40c98a..e508b83 100644 --- a/src/main/java/org/apache/sling/jcr/davex/impl/servlets/AuthHttpContext.java +++ b/src/main/java/org/apache/sling/jcr/davex/impl/servlets/AuthHttpContext.java @@ -17,53 +17,54 @@ package org.apache.sling.jcr.davex.impl.servlets; import java.io.IOException; -import java.net.URL; +import java.util.Set; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.felix.scr.annotations.Component; +import org.apache.felix.scr.annotations.ConfigurationPolicy; +import org.apache.felix.scr.annotations.Properties; +import org.apache.felix.scr.annotations.Property; +import org.apache.felix.scr.annotations.Reference; +import org.apache.felix.scr.annotations.Service; import org.apache.sling.auth.core.AuthenticationSupport; -import org.osgi.service.http.HttpContext; - -class AuthHttpContext implements HttpContext { +import org.osgi.framework.Constants; +import org.osgi.service.http.context.ServletContextHelper; +import org.osgi.service.http.whiteboard.HttpWhiteboardConstants; + +@Component(metatype = false, policy = ConfigurationPolicy.IGNORE) +@Service(ServletContextHelper.class) +@Properties({ + @Property(name = Constants.SERVICE_DESCRIPTION, value = "Sling JcrRemoting Servlet"), + @Property(name = Constants.SERVICE_VENDOR, value = "The Apache Software Foundation"), + @Property(name = HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_NAME, value = AuthHttpContext.HTTP_CONTEXT_NAME), + @Property(name = HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_PATH, value = "/"), + @Property(name = Constants.SERVICE_RANKING, intValue = 5) +}) +public class AuthHttpContext extends ServletContextHelper { /** - * The root path at which the DavEx servlet is registered. This is used to - * extract the workspace name from the request URL where the workspace name - * is the first segment in the path after the this path. + * The name of this ServletContext for use by they SlingDavExServlet */ - private final String davRoot; + static final String HTTP_CONTEXT_NAME = "DavExAuthHttpContext"; /** * Handles security * * @see #handleSecurity(HttpServletRequest, HttpServletResponse) */ + @Reference private AuthenticationSupport authenticator; - AuthHttpContext(final String davRoot) { - this.davRoot = davRoot; - } - - public void setAuthenticationSupport(final AuthenticationSupport auth) { - this.authenticator = auth; - } - - // ---------- HttpContext - - /** - * Returns the MIME type as resolved by the <code>MimeTypeService</code> or - * <code>null</code> if the service is not available. - */ - public String getMimeType(String name) { - return null; - } + // ---------- ServletContextHelper /** - * Always returns <code>null</code> because resources are all provided - * through the {@link MainServlet}. + * Always returns <code>null</code> as resources are only accessible through + * the {@link SlingDavExServlet}. */ - public URL getResource(String name) { + @Override + public Set<String> getResourcePaths(String path) { return null; } @@ -94,40 +95,21 @@ class AuthHttpContext implements HttpContext { private final String getWorkspace(final String uriPath) { - // Paths to consider - // /davRoot - // /davRoot/ - // /davRoot/wsp - // /davRoot/wsp/ - // /davRoot/wsp/... - - if (uriPath != null && uriPath.startsWith(this.davRoot)) { - - // cut off root - int start = this.davRoot.length(); - - // just the root - if (start >= uriPath.length()) { - return null; - } - - if (uriPath.charAt(start) == '/') { - start++; - } else { - // expected slash, actually (don't care) - return null; - } - - // just the root with trailing slash - if (start >= uriPath.length()) { - return null; - } - - int end = uriPath.indexOf('/', start); - if (end > start) { - return uriPath.substring(start, end); + // Paths to consider | Result + // -------------------+--------- + // null | null + // "" (empty) | null + // / | null + // /wsp | wsp + // /wsp/ | wsp + // /wsp/... | wsp + + if (uriPath != null && uriPath.length() > 1 && uriPath.charAt(0) == '/') { + int end = uriPath.indexOf('/', 1); + if (end > 1) { + return uriPath.substring(1, end); } else if (end < 0) { - return uriPath.substring(start); + return uriPath.substring(1); } } diff --git a/src/main/java/org/apache/sling/jcr/davex/impl/servlets/SlingDavExServlet.java b/src/main/java/org/apache/sling/jcr/davex/impl/servlets/SlingDavExServlet.java index 0aba2d1..8b7cde0 100644 --- a/src/main/java/org/apache/sling/jcr/davex/impl/servlets/SlingDavExServlet.java +++ b/src/main/java/org/apache/sling/jcr/davex/impl/servlets/SlingDavExServlet.java @@ -25,6 +25,7 @@ import javax.jcr.Repository; import javax.jcr.RepositoryException; import javax.jcr.Session; import javax.jcr.SimpleCredentials; +import javax.servlet.Servlet; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -45,7 +46,7 @@ import org.apache.sling.jcr.api.SlingRepository; import org.osgi.framework.BundleContext; import org.osgi.framework.Constants; import org.osgi.framework.ServiceRegistration; -import org.osgi.service.http.HttpService; +import org.osgi.service.http.whiteboard.HttpWhiteboardConstants; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -85,6 +86,17 @@ public class SlingDavExServlet extends JcrRemotingServlet { private static final String PROP_CREATE_ABSOLUTE_URI = "dav.create-absolute-uri"; /** + * Default value for the configuration {@link #PROP_PROTECTED_HANDLERS} + */ + private static final String DEFAULT_PROTECTED_HANDLERS = "org.apache.jackrabbit.server.remoting.davex.AclRemoveHandler"; + + /** + * defines the Protected handlers for the Jcr Remoting Servlet + */ + @Property(value=DEFAULT_PROTECTED_HANDLERS) + private static final String PROP_PROTECTED_HANDLERS = "dav.protectedhandlers"; + + /** * The name of the service property of the registered dummy service to cause * the path to the DavEx servlet to not be subject to forced authentication. */ @@ -98,88 +110,37 @@ public class SlingDavExServlet extends JcrRemotingServlet { @Reference private SlingRepository repository; - @Reference - private HttpService httpService; - - @Reference - private AuthenticationSupport authSupport; - - /** - * The path at which the DavEx servlet has successfully been - * registered in the {@link #activate(Map)} method. If this is - * <code>null</code> the DavEx servlet is not registered with the - * Http Service. - */ - private String servletAlias; - - /** - * The dummy service registration to convey to the Sling Authenticator - * that everything under the alias must not be forcibly authenticated. - * This will be <code>null</code> if the DavEx servlet registration - * fails. - */ - private ServiceRegistration dummyService; - /** - * Default value for the configuration {@link #PROP_PROTECTED_HANDLERS} + * The DavExServlet service registration with the OSGi Whiteboard. */ - public static final String DEFAULT_PROTECTED_HANDLERS = "org.apache.jackrabbit.server.remoting.davex.AclRemoveHandler"; + private ServiceRegistration davServlet; - /** - * defines the Protected handlers for the Jcr Remoting Servlet - */ - @Property(value=DEFAULT_PROTECTED_HANDLERS) - public static final String PROP_PROTECTED_HANDLERS = "dav.protectedhandlers"; - @Activate protected void activate(final BundleContext bundleContext, final Map<String, ?> config) { final String davRoot = OsgiUtil.toString(config.get(PROP_DAV_ROOT), DEFAULT_DAV_ROOT); final boolean createAbsoluteUri = OsgiUtil.toBoolean(config.get(PROP_CREATE_ABSOLUTE_URI), DEFAULT_CREATE_ABSOLUTE_URI); final String protectedHandlers = OsgiUtil.toString(config.get(PROP_PROTECTED_HANDLERS), DEFAULT_PROTECTED_HANDLERS); - final AuthHttpContext context = new AuthHttpContext(davRoot); - context.setAuthenticationSupport(authSupport); - // prepare DavEx servlet config - final Dictionary<String, String> initProps = new Hashtable<String, String>(); - - // prefix to the servlet - initProps.put(INIT_PARAM_RESOURCE_PATH_PREFIX, davRoot); - - // create absolute URIs (or absolute paths) - initProps.put(INIT_PARAM_CREATE_ABSOLUTE_URI, Boolean.toString(createAbsoluteUri)); - - // disable CSRF checks for now (should be handled by Sling) - initProps.put(INIT_PARAM_CSRF_PROTECTION, CSRFUtil.DISABLED); - + final Dictionary<String, Object> initProps = new Hashtable<String, Object>(); + initProps.put(toInitParamProperty(INIT_PARAM_RESOURCE_PATH_PREFIX), davRoot); + initProps.put(toInitParamProperty(INIT_PARAM_CREATE_ABSOLUTE_URI), Boolean.toString(createAbsoluteUri)); + initProps.put(toInitParamProperty(INIT_PARAM_CSRF_PROTECTION), CSRFUtil.DISABLED); initProps.put(INIT_PARAM_PROTECTED_HANDLERS_CONFIG, protectedHandlers); - - // register and handle registration failure - try { - this.httpService.registerServlet(davRoot, this, initProps, context); - this.servletAlias = davRoot; - - java.util.Properties dummyServiceProperties = new java.util.Properties(); - dummyServiceProperties.put(Constants.SERVICE_VENDOR, config.get(Constants.SERVICE_VENDOR)); - dummyServiceProperties.put(Constants.SERVICE_DESCRIPTION, - "Helper for " + config.get(Constants.SERVICE_DESCRIPTION)); - dummyServiceProperties.put(PAR_AUTH_REQ, "-" + davRoot); - this.dummyService = bundleContext.registerService("java.lang.Object", new Object(), dummyServiceProperties); - } catch (Exception e) { - log.error("activate: Failed registering DavEx Servlet at " + davRoot, e); - } + initProps.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN, davRoot.concat("/*")); + initProps.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_SELECT, + "(" + HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_NAME + "=" + AuthHttpContext.HTTP_CONTEXT_NAME + ")"); + initProps.put(Constants.SERVICE_VENDOR, config.get(Constants.SERVICE_VENDOR)); + initProps.put(Constants.SERVICE_DESCRIPTION, config.get(Constants.SERVICE_DESCRIPTION)); + initProps.put(PAR_AUTH_REQ, "-" + davRoot); // make sure this is not forcible authenticated ! + this.davServlet = bundleContext.registerService(Servlet.class.getName(), this, initProps); } @Deactivate protected void deactivate() { - if (this.dummyService != null) { - this.dummyService.unregister(); - this.dummyService = null; - } - - if (this.servletAlias != null) { - this.httpService.unregister(servletAlias); - this.servletAlias = null; + if (this.davServlet!= null) { + this.davServlet.unregister(); + this.davServlet = null; } } @@ -251,4 +212,18 @@ public class SlingDavExServlet extends JcrRemotingServlet { } }; } + + /** + * Returns the name as a String suitable for use as a property registered with + * and OSGi Http Whiteboard Service init parameter. + * + * @param name The parameter to convert. Must not be {@code null} and should not be empty. + * + * @return The converted name properly prefixed. + * + * @throws NullPointerException if {@code name} is {@code null}. + */ + private static String toInitParamProperty(final String name) { + return HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_INIT_PARAM_PREFIX.concat(name); + } } diff --git a/src/test/java/org/apache/sling/jcr/davex/impl/servlets/AuthHttpContextTest.java b/src/test/java/org/apache/sling/jcr/davex/impl/servlets/AuthHttpContextTest.java index 50f4b9d..47477be 100644 --- a/src/test/java/org/apache/sling/jcr/davex/impl/servlets/AuthHttpContextTest.java +++ b/src/test/java/org/apache/sling/jcr/davex/impl/servlets/AuthHttpContextTest.java @@ -37,50 +37,44 @@ public class AuthHttpContextTest { @Test public void test_getWorkspace_null() throws Throwable { - final AuthHttpContext ahc = new AuthHttpContext("/server"); + final AuthHttpContext ahc = new AuthHttpContext(); TestCase.assertNull(getWorkspace.invoke(ahc, (String) null)); } @Test public void test_getWorkspace_root() throws Throwable { - final AuthHttpContext ahc = new AuthHttpContext("/server"); - TestCase.assertNull(getWorkspace.invoke(ahc, "/server")); + final AuthHttpContext ahc = new AuthHttpContext(); + TestCase.assertNull(getWorkspace.invoke(ahc, "")); } @Test public void test_getWorkspace_root_slash() throws Throwable { - final AuthHttpContext ahc = new AuthHttpContext("/server"); - TestCase.assertNull(getWorkspace.invoke(ahc, "/server/")); - } - - @Test - public void test_getWorkspace_root_char() throws Throwable { - final AuthHttpContext ahc = new AuthHttpContext("/server"); - TestCase.assertNull(getWorkspace.invoke(ahc, "/serverxyz")); + final AuthHttpContext ahc = new AuthHttpContext(); + TestCase.assertNull(getWorkspace.invoke(ahc, "/")); } @Test public void test_getWorkspace_wsp() throws Throwable { - final AuthHttpContext ahc = new AuthHttpContext("/server"); - TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/server/w")); - TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/server/wsp")); + final AuthHttpContext ahc = new AuthHttpContext(); + TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/w")); + TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/wsp")); } @Test public void test_getWorkspace_wsp_slash() throws Throwable { - final AuthHttpContext ahc = new AuthHttpContext("/server"); - TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/server/w/")); - TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/server/wsp/")); + final AuthHttpContext ahc = new AuthHttpContext(); + TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/w/")); + TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/wsp/")); } @Test public void test_getWorkspace_wsp_path() throws Throwable { - final AuthHttpContext ahc = new AuthHttpContext("/server"); - TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/server/w/abc")); - TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/server/wsp/abc")); + final AuthHttpContext ahc = new AuthHttpContext(); + TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/w/abc")); + TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/wsp/abc")); - TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/server/w/abc/xyz")); - TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/server/wsp/abc/xyz")); + TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/w/abc/xyz")); + TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/wsp/abc/xyz")); } } -- To stop receiving notification emails like this one, please contact "[email protected]" <[email protected]>.
