incubator-sling-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From fmesc...@apache.org
Subject svn commit: r888399 - in /sling/trunk/bundles/servlets/get/src: main/java/org/apache/sling/servlets/get/impl/ test/java/org/apache/sling/servlets/get/impl/
Date Tue, 08 Dec 2009 13:53:29 GMT
Author: fmeschbe
Date: Tue Dec  8 13:53:28 2009
New Revision: 888399

URL: http://svn.apache.org/viewvc?rev=888399&view=rev
Log:
SLING-1227 Refactor RedirectServlet to always return either a full URL or an absolute path.
If the redirect target is not an absolute URL (including scheme and host part) the path is
ensured absolute (resolving relative to the request's resource.

Added:
    sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockResourceResolver.java
  (with props)
Modified:
    sling/trunk/bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/RedirectServlet.java
    sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockSlingHttpServletRequest.java
    sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/RedirectServletTest.java

Modified: sling/trunk/bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/RedirectServlet.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/RedirectServlet.java?rev=888399&r1=888398&r2=888399&view=diff
==============================================================================
--- sling/trunk/bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/RedirectServlet.java
(original)
+++ sling/trunk/bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/RedirectServlet.java
Tue Dec  8 13:53:28 2009
@@ -28,36 +28,35 @@
 import org.apache.sling.api.SlingHttpServletResponse;
 import org.apache.sling.api.request.RequestPathInfo;
 import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.resource.ResourceUtil;
 import org.apache.sling.api.resource.ValueMap;
 import org.apache.sling.api.servlets.SlingSafeMethodsServlet;
 import org.apache.sling.servlets.get.impl.helpers.JsonRendererServlet;
 
 /**
  * The <code>RedirectServlet</code> implements support for GET requests to
- * resources of type <code>sling:redirect</code>. This servlet tries to
- * get the redirect target by
+ * resources of type <code>sling:redirect</code>. This servlet tries to get the
+ * redirect target by
  * <ul>
- * <li>first adapting the resource to a {@link ValueMap} and trying
- * to get the property <code>sling:target</code>.</li>
+ * <li>first adapting the resource to a {@link ValueMap} and trying to get the
+ * property <code>sling:target</code>.</li>
  * <li>The second attempt is to access the resource <code>sling:target</code>
  * below the requested resource and attapt this to a string.</li>
  * <p>
  * If there is no value found for <code>sling:target</code> a 404 (NOT FOUND)
- * status is
- * sent by this servlet. Otherwise a 302 (FOUND, temporary redirect) status is
- * sent where the target is the relative URL from the current resource to the
- * target resource. Selectors, extension, suffix and query string are also
- * appended to the redirect URL.
+ * status is sent by this servlet. Otherwise a 302 (FOUND, temporary redirect)
+ * status is sent where the target is the relative URL from the current resource
+ * to the target resource. Selectors, extension, suffix and query string are
+ * also appended to the redirect URL.
  *
  * @scr.component immediate="true" metatype="no"
  * @scr.service interface="javax.servlet.Servlet"
- *
  * @scr.property name="service.description" value="Request Redirect Servlet"
  * @scr.property name="service.vendor" value="The Apache Software Foundation"
- *
  * @scr.property name="sling.servlet.resourceTypes" value="sling:redirect"
  * @scr.property name="sling.servlet.methods" value="GET"
- * @scr.property name="sling.servlet.prefix" value="-1" type="Integer" private="true"
+ * @scr.property name="sling.servlet.prefix" value="-1" type="Integer"
+ *               private="true"
  */
 public class RedirectServlet extends SlingSafeMethodsServlet {
 
@@ -82,13 +81,13 @@
         // convert resource to a value map
         final Resource rsrc = request.getResource();
         final ValueMap valueMap = rsrc.adaptTo(ValueMap.class);
-        if ( valueMap != null ) {
+        if (valueMap != null) {
             targetPath = valueMap.get(TARGET_PROP, String.class);
         }
-        if ( targetPath == null ) {
+        if (targetPath == null) {
             // old behaviour
             final Resource targetResource = request.getResourceResolver().getResource(
-                    rsrc, TARGET_PROP);
+                rsrc, TARGET_PROP);
             if (targetResource == null) {
                 response.sendError(HttpServletResponse.SC_NOT_FOUND,
                     "Missing target for redirection");
@@ -102,8 +101,10 @@
 
         // if we got a target path, make it external and redirect to it
         if (targetPath != null) {
-            // make path relative and append selectors, extension etc.
-            targetPath = toRedirectPath(targetPath, request);
+            if (!isUrl(targetPath)) {
+                // make path relative and append selectors, extension etc.
+                targetPath = toRedirectPath(targetPath, request);
+            }
 
             // and redirect there ...
             response.sendRedirect(targetPath);
@@ -123,98 +124,50 @@
      */
     protected static String toRedirectPath(String targetPath,
             SlingHttpServletRequest request) {
-        // first check for an absolute path
-        final int protocolIndex = targetPath.indexOf(":/");
-        final int queryIndex = targetPath.indexOf('?');
-        if (  protocolIndex > -1 && (queryIndex == -1 || queryIndex > protocolIndex)
) {
-            return targetPath;
-        }
 
-        String postFix;
-        RequestPathInfo rpi = request.getRequestPathInfo();
-        if (rpi.getExtension() != null) {
-            StringBuffer postfixBuf = new StringBuffer();
-            if (rpi.getSelectorString() != null) {
-                postfixBuf.append('.').append(rpi.getSelectorString());
-            }
-            postfixBuf.append('.').append(rpi.getExtension());
-            if (rpi.getSuffix() != null) {
-                postfixBuf.append(rpi.getSuffix());
-            }
-            postFix = postfixBuf.toString();
-        } else {
-            postFix = null;
-        }
+        // if the target path is an URL, do nothing and return it unmodified
+        final RequestPathInfo rpi = request.getRequestPathInfo();
 
-        String basePath = rpi.getResourcePath();
 
         // make sure the target path is absolute
-        if (!targetPath.startsWith("/")) {
-            if (!basePath.endsWith("/")) {
-                targetPath = "/".concat(targetPath);
-            }
-            targetPath = basePath.concat(targetPath);
-        }
-
-        // append optional selectors etc.to the base path
-        if (postFix != null) {
-            basePath = basePath.concat(postFix);
+        final String rawAbsPath;
+        if (targetPath.startsWith("/")) {
+            rawAbsPath = targetPath;
+        } else {
+            rawAbsPath = request.getResource().getPath() + "/" + targetPath;
         }
 
-        StringBuffer pathBuf = new StringBuffer();
+        final StringBuilder target = new StringBuilder();
 
-        makeRelative(pathBuf, basePath, targetPath, request);
-
-        if (postFix != null) {
-            pathBuf.append(postFix);
-        }
-
-        if (request.getQueryString() != null) {
-            pathBuf.append('?').append(request.getQueryString());
+        // and ensure the path is normalized, us unnormalized if not possible
+        final String absPath = ResourceUtil.normalize(rawAbsPath);
+        if (absPath == null) {
+            target.append(rawAbsPath);
+        } else {
+            target.append(absPath);
         }
 
-        return pathBuf.toString();
-    }
+        // append current selectors, extension and suffix
+        if (rpi.getExtension() != null) {
 
-    /**
-     * Converts the absolute path target into a path relative to base and stores
-     * this relative path into pathBuffer.
-     */
-    private static void makeRelative(StringBuffer pathBuffer, String base,
-            String target, SlingHttpServletRequest request) {
-        if ( base == null || base.length() == 0 ) {
-            final String ctxPath = request.getContextPath();
-            pathBuffer.append(ctxPath);
-            if ( ctxPath.endsWith("/") ) {
-                pathBuffer.append(target.substring(1));
-            } else {
-                pathBuffer.append(target);
+            if (rpi.getSelectorString() != null) {
+                target.append('.').append(rpi.getSelectorString());
             }
-            return;
-        }
-        // pseudo entry to correctly calculate the relative path
-        if (base.endsWith("/")) {
-            base = base.concat(String.valueOf(Character.MAX_VALUE));
-        }
-
-        String[] bParts = base.substring(1).split("/");
-        String[] tParts = target.substring(1).split("/");
 
-        // find first non-matching part
-        int off;
-        for (off = 0; off < (bParts.length - 1) && off < tParts.length
-            && bParts[off].equals(tParts[off]); off++);
+            target.append('.').append(rpi.getExtension());
 
-        for (int i = bParts.length - off; i > 1; i--) {
-            pathBuffer.append("../");
+            if (rpi.getSuffix() != null) {
+                target.append(rpi.getSuffix());
+            }
         }
 
-        for (int i = off; i < tParts.length; i++) {
-            if (i > off) {
-                pathBuffer.append('/');
-            }
-            pathBuffer.append(tParts[i]);
+        // append current querystring
+        if (request.getQueryString() != null) {
+            target.append('?').append(request.getQueryString());
         }
+
+        // return the mapped full path
+        return request.getResourceResolver().map(request, target.toString());
     }
 
     private Servlet getJsonRendererServlet() {
@@ -229,4 +182,18 @@
         }
         return jsonRendererServlet;
     }
+
+    /**
+     * Returns <code>true</code> if the path is potentially an URL. This
+     * simplistic check looks for a ":/" string in the path assuming that this
+     * is a separator to separate the scheme from the scheme-specific part. If
+     * the separator occurs after a query separator ("?"), though, it is not
+     * assumed to be a scheme-separator.
+     */
+    private static boolean isUrl(final String path) {
+        final int protocolIndex = path.indexOf(":/");
+        final int queryIndex = path.indexOf('?');
+        return protocolIndex > -1
+            && (queryIndex == -1 || queryIndex > protocolIndex);
+    }
 }

Added: sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockResourceResolver.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockResourceResolver.java?rev=888399&view=auto
==============================================================================
--- sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockResourceResolver.java
(added)
+++ sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockResourceResolver.java
Tue Dec  8 13:53:28 2009
@@ -0,0 +1,84 @@
+/*
+ * Copyright 1997-2009 Day Management AG
+ * Barfuesserplatz 6, 4001 Basel, Switzerland
+ * All Rights Reserved.
+ *
+ * This software is the confidential and proprietary information of
+ * Day Management AG, ("Confidential Information"). You shall not
+ * disclose such Confidential Information and shall use it only in
+ * accordance with the terms of the license agreement you entered into
+ * with Day.
+ */
+package org.apache.sling.servlets.get.impl;
+
+import java.util.Iterator;
+import java.util.Map;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.resource.ResourceResolver;
+
+/**
+ * The <code>MockResourceResolver</code> implements the {@link #map(String)}
+ * method simply returning the path unmodified and the
+ * {@link #map(HttpServletRequest, String)} method returning the path prefixed
+ * with the request context path.
+ * <p>
+ * The other methods are not implemented and return <code>null</code>.
+ */
+public class MockResourceResolver implements ResourceResolver {
+
+    public Iterator<Resource> findResources(String arg0, String arg1) {
+        return null;
+    }
+
+    public Resource getResource(String arg0) {
+        return null;
+    }
+
+    public Resource getResource(Resource arg0, String arg1) {
+        return null;
+    }
+
+    public String[] getSearchPath() {
+        return null;
+    }
+
+    public Iterator<Resource> listChildren(Resource arg0) {
+        return null;
+    }
+
+    public String map(String path) {
+        return path;
+    }
+
+    public String map(HttpServletRequest request, String path) {
+        if (request.getContextPath().length() == 0) {
+            return path;
+        }
+
+        return request.getContextPath() + path;
+    }
+
+    public Iterator<Map<String, Object>> queryResources(String arg0, String arg1)
{
+        return null;
+    }
+
+    public Resource resolve(String arg0) {
+        return null;
+    }
+
+    public Resource resolve(HttpServletRequest arg0) {
+        return null;
+    }
+
+    public Resource resolve(HttpServletRequest arg0, String arg1) {
+        return null;
+    }
+
+    public <AdapterType> AdapterType adaptTo(Class<AdapterType> arg0) {
+        return null;
+    }
+
+}

Propchange: sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockResourceResolver.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockResourceResolver.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision Rev Url

Modified: sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockSlingHttpServletRequest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockSlingHttpServletRequest.java?rev=888399&r1=888398&r2=888399&view=diff
==============================================================================
--- sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockSlingHttpServletRequest.java
(original)
+++ sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/MockSlingHttpServletRequest.java
Tue Dec  8 13:53:28 2009
@@ -48,24 +48,28 @@
 
     private final String queryString;
 
+    private final ResourceResolver resourceResolver = new MockResourceResolver();
+
+    private final String contextPath;
+
     MockSlingHttpServletRequest() {
         this(null, null, null, null, null);
     }
 
     public MockSlingHttpServletRequest(String resourcePath, String selectors,
             String extension, String suffix, String queryString) {
-        this.resource = new SyntheticResource(null, resourcePath, null);
-        this.requestPathInfo = new MockRequestPathInfo(selectors, extension,
-            suffix, resourcePath);
-        this.queryString = queryString;
+        this(resourcePath, selectors, extension, suffix, queryString,
+            resourcePath, "");
     }
 
     public MockSlingHttpServletRequest(String resourcePath, String selectors,
-            String extension, String suffix, String queryString, String requestPath) {
+            String extension, String suffix, String queryString,
+            String requestPath, String contextPath) {
         this.resource = new SyntheticResource(null, resourcePath, null);
         this.requestPathInfo = new MockRequestPathInfo(selectors, extension,
             suffix, requestPath);
         this.queryString = queryString;
+        this.contextPath = contextPath;
     }
 
     public Cookie getCookie(String name) {
@@ -119,7 +123,7 @@
     }
 
     public ResourceResolver getResourceResolver() {
-        return null;
+        return resourceResolver;
     }
 
     public String getResponseContentType() {
@@ -135,7 +139,7 @@
     }
 
     public String getContextPath() {
-        return "/webapp";
+        return contextPath;
     }
 
     public Cookie[] getCookies() {

Modified: sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/RedirectServletTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/RedirectServletTest.java?rev=888399&r1=888398&r2=888399&view=diff
==============================================================================
--- sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/RedirectServletTest.java
(original)
+++ sling/trunk/bundles/servlets/get/src/test/java/org/apache/sling/servlets/get/impl/RedirectServletTest.java
Tue Dec  8 13:53:28 2009
@@ -27,127 +27,139 @@
     public void testSameParent() {
         String base = "/a";
         String target = "/b";
-        assertEquals("b", toRedirect(base, target));
+        assertEquals("/b", toRedirect(base, target));
 
         base = "/";
         target = "/a";
-        assertEquals("a", toRedirect(base, target));
+        assertEquals("/a", toRedirect(base, target));
 
         base = "/a/b/c";
         target = "/a/b/d";
-        assertEquals("d", toRedirect(base, target));
+        assertEquals("/a/b/d", toRedirect(base, target));
     }
 
     public void testTrailingSlash() {
         String base = "/a/b/c/";
         String target = "/a/b/c.html";
-        assertEquals("../c.html", toRedirect(base, target));
+        assertEquals("/a/b/c.html", toRedirect(base, target));
     }
 
     public void testCommonAncestor() {
         String base = "/a/b/c/d";
         String target = "/a/b/x/y";
-        assertEquals("../x/y", toRedirect(base, target));
+        assertEquals("/a/b/x/y", toRedirect(base, target));
     }
 
     public void testChild() {
         String base = "/a.html";
         String target = "/a/b.html";
-        assertEquals("a/b.html", toRedirect(base, target));
+        assertEquals("/a/b.html", toRedirect(base, target));
 
         base = "/a";
         target = "/a/b.html";
-        assertEquals("a/b.html", toRedirect(base, target));
+        assertEquals("/a/b.html", toRedirect(base, target));
 
         base = "/a";
         target = "/a/b";
-        assertEquals("a/b", toRedirect(base, target));
+        assertEquals("/a/b", toRedirect(base, target));
 
         base = "/a.html";
         target = "/a/b/c.html";
-        assertEquals("a/b/c.html", toRedirect(base, target));
+        assertEquals("/a/b/c.html", toRedirect(base, target));
 
         base = "/a";
         target = "/a/b/c.html";
-        assertEquals("a/b/c.html", toRedirect(base, target));
+        assertEquals("/a/b/c.html", toRedirect(base, target));
 
         base = "/a";
         target = "/a/b/c";
-        assertEquals("a/b/c", toRedirect(base, target));
+        assertEquals("/a/b/c", toRedirect(base, target));
     }
 
     public void testChildNonRoot() {
         String base = "/x/a.html";
         String target = "/x/a/b.html";
-        assertEquals("a/b.html", toRedirect(base, target));
+        assertEquals("/x/a/b.html", toRedirect(base, target));
 
         base = "/x/a";
         target = "/x/a/b.html";
-        assertEquals("a/b.html", toRedirect(base, target));
+        assertEquals("/x/a/b.html", toRedirect(base, target));
 
         base = "/x/a";
         target = "/x/a/b";
-        assertEquals("a/b", toRedirect(base, target));
+        assertEquals("/x/a/b", toRedirect(base, target));
 
         base = "/x/a.html";
         target = "/x/a/b/c.html";
-        assertEquals("a/b/c.html", toRedirect(base, target));
+        assertEquals("/x/a/b/c.html", toRedirect(base, target));
 
         base = "/x/a";
         target = "/x/a/b/c.html";
-        assertEquals("a/b/c.html", toRedirect(base, target));
+        assertEquals("/x/a/b/c.html", toRedirect(base, target));
 
         base = "/x/a";
         target = "/x/a/b/c";
-        assertEquals("a/b/c", toRedirect(base, target));
+        assertEquals("/x/a/b/c", toRedirect(base, target));
     }
 
     public void testChildRelative() {
         String base = "/a";
         String target = "b.html";
-        assertEquals("a/b.html", toRedirect(base, target));
+        assertEquals("/a/b.html", toRedirect(base, target));
 
         base = "/a";
         target = "b";
-        assertEquals("a/b", toRedirect(base, target));
+        assertEquals("/a/b", toRedirect(base, target));
 
         base = "/a";
         target = "b/c.html";
-        assertEquals("a/b/c.html", toRedirect(base, target));
+        assertEquals("/a/b/c.html", toRedirect(base, target));
 
         base = "/a";
         target = "b/c";
-        assertEquals("a/b/c", toRedirect(base, target));
+        assertEquals("/a/b/c", toRedirect(base, target));
     }
 
     public void testChildNonRootRelative() {
         String base = "/x/a";
         String target = "b.html";
-        assertEquals("a/b.html", toRedirect(base, target));
+        assertEquals("/x/a/b.html", toRedirect(base, target));
 
         base = "/x/a";
         target = "b";
-        assertEquals("a/b", toRedirect(base, target));
+        assertEquals("/x/a/b", toRedirect(base, target));
 
         base = "/x/a";
         target = "b/c.html";
-        assertEquals("a/b/c.html", toRedirect(base, target));
+        assertEquals("/x/a/b/c.html", toRedirect(base, target));
 
         base = "/x/a";
         target = "b/c";
-        assertEquals("a/b/c", toRedirect(base, target));
+        assertEquals("/x/a/b/c", toRedirect(base, target));
     }
 
     public void testUnCommon() {
         String base = "/a/b/c/d";
         String target = "/w/x/y/z";
-        assertEquals("../../../w/x/y/z", toRedirect(base, target));
+        assertEquals("/w/x/y/z", toRedirect(base, target));
+    }
+
+    public void testSibbling() {
+        String base = "/a/b";
+        String target0 = "../y/z";
+        assertEquals("/a/y/z", toRedirect(base, target0));
+
+        String target1 = "../../y/z";
+        assertEquals("/y/z", toRedirect(base, target1));
+
+        String target2 = "../../../y/z";
+        assertEquals(base + "/" + target2, toRedirect(base, target2));
     }
 
     public void testSelectorsEtc() {
         String base = "/a/b/c";
         String target = "/a/b/d";
-        String expected = "d";
+        String expected = "/a/b/d";
 
         String selectors = null;
         String extension = null;
@@ -181,8 +193,8 @@
         extension = "html";
         suffix = "/suffix.pdf";
         queryString = null;
-        assertEquals("../" + expected, base, selectors, extension, suffix,
-            queryString, target);
+        assertEquals(expected, base, selectors, extension, suffix, queryString,
+            target);
 
         selectors = null;
         extension = "html";
@@ -195,15 +207,15 @@
         extension = "html";
         suffix = "/suffix.pdf";
         queryString = "xy=1";
-        assertEquals("../" + expected, base, selectors, extension, suffix,
-            queryString, target);
+        assertEquals(expected, base, selectors, extension, suffix, queryString,
+            target);
 
         selectors = "print.a4";
         extension = "html";
         suffix = "/suffix.pdf";
         queryString = "xy=1";
-        assertEquals("../" + expected, base, selectors, extension, suffix,
-            queryString, target);
+        assertEquals(expected, base, selectors, extension, suffix, queryString,
+            target);
     }
 
     private void assertEquals(String expected, String basePath,
@@ -242,13 +254,13 @@
     }
 
     public void testEmptyPath() {
-        SlingHttpServletRequest request = new MockSlingHttpServletRequest(
-                "/", null, null, null, null, "");
+        SlingHttpServletRequest request = new MockSlingHttpServletRequest("/",
+            null, null, null, null, "", "/webapp");
         String path = RedirectServlet.toRedirectPath("/index.html", request);
         assertEquals("/webapp/index.html", path);
-        request = new MockSlingHttpServletRequest(
-                "/", null, null, null, null, "/");
+        request = new MockSlingHttpServletRequest("/", null, null, null, null,
+            "/", "/webapp");
         path = RedirectServlet.toRedirectPath("/index.html", request);
-        assertEquals("index.html", path);
+        assertEquals("/webapp/index.html", path);
     }
 }



Mime
View raw message