You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2017/11/07 09:48:39 UTC

[sling-org-apache-sling-jcr-davex] 02/07: Revert "SLING-6378 Unclosed ResourceResolver in davex AuthHttpContext"

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.8
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-davex.git

commit 0078a7bba87a7d5b9c69721169c4f95309f80412
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Mon Jan 30 10:28:53 2017 +0000

    Revert "SLING-6378 Unclosed ResourceResolver in davex AuthHttpContext"
    
    This reverts commit 69061dfc40dbc8675888ee566935a897c09fb844.
    
    git-svn-id: https://svn.apache.org/repos/asf/sling/trunk/bundles/jcr/davex@1780888 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, 152 insertions(+), 104 deletions(-)

diff --git a/pom.xml b/pom.xml
index ab9cea1..a229260 100644
--- a/pom.xml
+++ b/pom.xml
@@ -152,8 +152,7 @@
         </dependency>
         <dependency>
             <groupId>org.osgi</groupId>
-            <artifactId>org.osgi.service.http.whiteboard</artifactId>
-            <version>1.0.0</version>
+            <artifactId>org.osgi.compendium</artifactId>
         </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 e508b83..b40c98a 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,54 +17,53 @@
 package org.apache.sling.jcr.davex.impl.servlets;
 
 import java.io.IOException;
-import java.util.Set;
+import java.net.URL;
 
 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.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 {
+import org.osgi.service.http.HttpContext;
+
+class AuthHttpContext implements HttpContext {
 
     /**
-     * The name of this ServletContext for use by they SlingDavExServlet
+     * 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.
      */
-    static final String HTTP_CONTEXT_NAME = "DavExAuthHttpContext";
+    private final String davRoot;
 
     /**
      * Handles security
      *
      * @see #handleSecurity(HttpServletRequest, HttpServletResponse)
      */
-    @Reference
     private AuthenticationSupport authenticator;
 
-    // ---------- ServletContextHelper
+    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;
+    }
 
     /**
-     * Always returns <code>null</code> as resources are only accessible through
-     * the {@link SlingDavExServlet}.
+     * Always returns <code>null</code> because resources are all provided
+     * through the {@link MainServlet}.
      */
-    @Override
-    public Set<String> getResourcePaths(String path) {
+    public URL getResource(String name) {
         return null;
     }
 
@@ -95,21 +94,40 @@ public class AuthHttpContext extends ServletContextHelper {
 
     private final String getWorkspace(final String uriPath) {
 
-        // 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);
+        // 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);
             } else if (end < 0) {
-                return uriPath.substring(1);
+                return uriPath.substring(start);
             }
         }
 
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 67f7f30..9896760 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,7 +25,6 @@ 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;
 
@@ -46,7 +45,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.whiteboard.HttpWhiteboardConstants;
+import org.osgi.service.http.HttpService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -86,17 +85,6 @@ 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.
      */
@@ -110,37 +98,88 @@ 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;
+
     /**
-     * The DavExServlet service registration with the OSGi Whiteboard.
+     * Default value for the configuration {@link #PROP_PROTECTED_HANDLERS}
      */
-    private ServiceRegistration davServlet;
+    public 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)
+    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, 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);
+        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);
+        
         initProps.put(INIT_PARAM_PROTECTED_HANDLERS_CONFIG, protectedHandlers);
-        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);
+
+        // 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);
+        }
     }
 
     @Deactivate
     protected void deactivate() {
-        if (this.davServlet!= null) {
-            this.davServlet.unregister();
-            this.davServlet = null;
+        if (this.dummyService != null) {
+            this.dummyService.unregister();
+            this.dummyService = null;
+        }
+
+        if (this.servletAlias != null) {
+            this.httpService.unregister(servletAlias);
+            this.servletAlias = null;
         }
     }
 
@@ -207,18 +246,4 @@ 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 47477be..50f4b9d 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,44 +37,50 @@ public class AuthHttpContextTest {
 
     @Test
     public void test_getWorkspace_null() throws Throwable {
-        final AuthHttpContext ahc = new AuthHttpContext();
+        final AuthHttpContext ahc = new AuthHttpContext("/server");
         TestCase.assertNull(getWorkspace.invoke(ahc, (String) null));
     }
 
     @Test
     public void test_getWorkspace_root() throws Throwable {
-        final AuthHttpContext ahc = new AuthHttpContext();
-        TestCase.assertNull(getWorkspace.invoke(ahc, ""));
+        final AuthHttpContext ahc = new AuthHttpContext("/server");
+        TestCase.assertNull(getWorkspace.invoke(ahc, "/server"));
     }
 
     @Test
     public void test_getWorkspace_root_slash() throws Throwable {
-        final AuthHttpContext ahc = new AuthHttpContext();
-        TestCase.assertNull(getWorkspace.invoke(ahc, "/"));
+        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"));
     }
 
     @Test
     public void test_getWorkspace_wsp() throws Throwable {
-        final AuthHttpContext ahc = new AuthHttpContext();
-        TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/w"));
-        TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/wsp"));
+        final AuthHttpContext ahc = new AuthHttpContext("/server");
+        TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/server/w"));
+        TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/server/wsp"));
     }
 
     @Test
     public void test_getWorkspace_wsp_slash() throws Throwable {
-        final AuthHttpContext ahc = new AuthHttpContext();
-        TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/w/"));
-        TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/wsp/"));
+        final AuthHttpContext ahc = new AuthHttpContext("/server");
+        TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/server/w/"));
+        TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/server/wsp/"));
     }
 
     @Test
     public void test_getWorkspace_wsp_path() throws Throwable {
-        final AuthHttpContext ahc = new AuthHttpContext();
-        TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/w/abc"));
-        TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/wsp/abc"));
+        final AuthHttpContext ahc = new AuthHttpContext("/server");
+        TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/server/w/abc"));
+        TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/server/wsp/abc"));
 
-        TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/w/abc/xyz"));
-        TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/wsp/abc/xyz"));
+        TestCase.assertEquals("w", getWorkspace.invoke(ahc, "/server/w/abc/xyz"));
+        TestCase.assertEquals("wsp", getWorkspace.invoke(ahc, "/server/wsp/abc/xyz"));
     }
 
 }

-- 
To stop receiving notification emails like this one, please contact
"commits@sling.apache.org" <co...@sling.apache.org>.