WICKET-5054 reverted Packages#absolutePath() to treat non-absolute

paths (i.e. without leading '/') as relative, see PackagesTest#absolutePath5(); resource path is absolute already when it is passed to IPackageResourceGuard, so it must not be made absolute again
apache · Feb 24, 2013 · 5a3e0b7 · 5a3e0b7
1 parent a86f842
commit 5a3e0b7
Show file tree

Hide file tree

Showing 9 changed files with 103 additions and 58 deletions.
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/IPackageResourceGuard.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/IPackageResourceGuard.java
@@ -32,11 +32,11 @@ public interface IPackageResourceGuard
 	 * 
 	 * @param scope
 	 *            This argument will be used to get the class loader for loading the package
-	 *            resource, and to determine what package it is in
-	 * @param path
-	 *            The path to the resource
+	 *            resource
+	 * @param absolutePath
+	 *            The absolute path to the resource
 	 * 
 	 * @return True if access is permitted, false otherwise
 	 */
-	boolean accept(final Class<?> scope, final String path);
+	boolean accept(final Class<?> scope, final String absolutePath);
 }
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/PackageResourceGuard.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/PackageResourceGuard.java
@@ -20,7 +20,6 @@
 import java.util.Set;
 
 import org.apache.wicket.Application;
-import org.apache.wicket.util.lang.Packages;
 import org.apache.wicket.util.string.Strings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -65,10 +64,9 @@ public PackageResourceGuard()
 	 * @see org.apache.wicket.markup.html.IPackageResourceGuard#accept(java.lang.Class,
 	 *      java.lang.String)
 	 */
-	@Override
-	public boolean accept(Class<?> scope, String path)
+	public boolean accept(Class<?> scope, String absolutePath)
 	{
-		String absolutePath = Packages.absolutePath(scope, path);
+		// path is already absolute
 		return acceptAbsolutePath(absolutePath);
 	}
 

diff --git a/wicket-core/src/test/java/org/apache/wicket/ParentResourceEscapePathTest.java b/wicket-core/src/test/java/org/apache/wicket/ParentResourceEscapePathTest.java
@@ -18,6 +18,7 @@
 
 import java.io.InputStream;
 
+import org.apache.wicket.markup.html.PackageResourceGuard;
 import org.apache.wicket.request.Url;
 import org.apache.wicket.request.resource.PackageResourceReference;
 import org.apache.wicket.request.resource.ResourceReference;
@@ -93,6 +94,10 @@ private void resourceUrlGeneratedByResourceReference()
 	@Test
 	public void requestHandlingOfResourceUrlWithEscapeStringInsideTest()
 	{
+		((PackageResourceGuard)tester.getApplication()
+			.getResourceSettings()
+			.getPackageResourceGuard()).setAllowAccessToRootResources(true);
+
 		tester.getApplication().getResourceSettings().setParentFolderPlaceholder("-updir-");
 		requestHandlingOfResourceUrlWithEscapeStringInside();
 

diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/PackageResourceTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/PackageResourceTest.java
@@ -23,10 +23,10 @@
 import org.apache.wicket.WicketTestCase;
 import org.apache.wicket.protocol.http.WebApplication;
 import org.apache.wicket.request.resource.JavaScriptPackageResource;
-import org.apache.wicket.request.resource.JavaScriptResourceReference;
 import org.apache.wicket.request.resource.PackageResource;
 import org.apache.wicket.request.resource.PackageResourceReference;
 import org.apache.wicket.request.resource.ResourceReference;
+import org.apache.wicket.util.lang.Packages;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -75,13 +75,20 @@ public void packageResourceGuard() throws Exception
 		assertFalse(guard.acceptExtension("java"));
 		assertTrue(guard.acceptAbsolutePath("foo/Bar.txt"));
 		assertFalse(guard.acceptAbsolutePath("foo/Bar.java"));
-		assertTrue(guard.accept(PackageResourceTest.class, "Bar.txt"));
-		assertTrue(guard.accept(PackageResourceTest.class, "Bar.txt."));
-		assertTrue(guard.accept(PackageResourceTest.class, ".Bar.txt"));
-		assertTrue(guard.accept(PackageResourceTest.class, ".Bar.txt."));
-		assertTrue(guard.accept(PackageResourceTest.class, ".Bar"));
-		assertTrue(guard.accept(PackageResourceTest.class, ".java"));
-		assertFalse(guard.accept(PackageResourceTest.class, "Bar.java"));
+		assertTrue(guard.accept(PackageResourceTest.class,
+			Packages.absolutePath(PackageResourceTest.class, "Bar.txt")));
+		assertTrue(guard.accept(PackageResourceTest.class,
+			Packages.absolutePath(PackageResourceTest.class, "Bar.txt.")));
+		assertTrue(guard.accept(PackageResourceTest.class,
+			Packages.absolutePath(PackageResourceTest.class, ".Bar.txt")));
+		assertTrue(guard.accept(PackageResourceTest.class,
+			Packages.absolutePath(PackageResourceTest.class, ".Bar.txt.")));
+		assertTrue(guard.accept(PackageResourceTest.class,
+			Packages.absolutePath(PackageResourceTest.class, ".Bar")));
+		assertTrue(guard.accept(PackageResourceTest.class,
+			Packages.absolutePath(PackageResourceTest.class, ".java")));
+		assertFalse(guard.accept(PackageResourceTest.class,
+			Packages.absolutePath(PackageResourceTest.class, "Bar.java")));
 	}
 
 	/**
@@ -163,11 +170,11 @@ public void contentType()
 	public void textFileWithEncoding()
 	{
 		final String encoding = "Klingon-8859-42";
-		final PackageResource resource =
-			new PackageResource(PackageResourceTest.class, "packaged1.txt", null, null, null)
-			{
-				private static final long serialVersionUID = 1L;
-			};
+		final PackageResource resource = new PackageResource(PackageResourceTest.class,
+			"packaged1.txt", null, null, null)
+		{
+			private static final long serialVersionUID = 1L;
+		};
 		resource.setTextEncoding(encoding);
 		tester.startResource(resource);
 		final String contentType = tester.getLastResponse().getContentType();
@@ -178,11 +185,11 @@ public void textFileWithEncoding()
 	public void javascriptFileWithEncoding()
 	{
 		final String encoding = "Klingon-8859-42";
-		final JavaScriptPackageResource resource =
-			new JavaScriptPackageResource(PackageResourceTest.class, "packaged3.js", null, null, null)
-			{
-				private static final long serialVersionUID = 1L;
-			};
+		final JavaScriptPackageResource resource = new JavaScriptPackageResource(
+			PackageResourceTest.class, "packaged3.js", null, null, null)
+		{
+			private static final long serialVersionUID = 1L;
+		};
 		resource.setTextEncoding(encoding);
 		tester.startResource(resource);
 		final String contentType = tester.getLastResponse().getContentType();

diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/SecurePackageResourceGuardTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/SecurePackageResourceGuardTest.java
@@ -18,6 +18,7 @@
 
 import org.apache.wicket.Application;
 import org.apache.wicket.WicketTestCase;
+import org.apache.wicket.util.lang.Packages;
 import org.junit.Test;
 
 /**
@@ -34,22 +35,29 @@ public void accept()
 		SecurePackageResourceGuard guard = new SecurePackageResourceGuard();
 		guard.setAllowAccessToRootResources(false);
 		guard.addPattern("+*.gif");
-		assertTrue(guard.accept(Application.class, "test.gif"));
-		assertTrue(guard.accept(Application.class, "mydir/test.gif"));
+		assertTrue(guard.accept(Application.class,
+			Packages.absolutePath(Application.class, "test.gif")));
+		assertTrue(guard.accept(Application.class,
+			Packages.absolutePath(Application.class, "mydir/test.gif")));
 		assertTrue(guard.accept(Application.class, "/root/mydir/test.gif"));
-		assertTrue(guard.accept(Application.class, "../test.gif"));
-		assertTrue(guard.accept(Application.class, "../../test.gif"));
+		assertTrue(guard.accept(Application.class,
+			Packages.absolutePath(Application.class, "../test.gif")));
+		assertTrue(guard.accept(Application.class,
+			Packages.absolutePath(Application.class, "../../test.gif")));
 
-		// root package
-		assertFalse(guard.accept(Application.class, "../../../test.gif"));
+		// web-inf (root package)
+		assertFalse(guard.accept(Application.class,
+			Packages.absolutePath(Application.class, "../../../test.gif")));
 		guard.setAllowAccessToRootResources(true);
-		assertTrue(guard.accept(Application.class, "../../../test.gif"));
+		assertTrue(guard.accept(Application.class,
+			Packages.absolutePath(Application.class, "../../../test.gif")));
 
 		boolean hit = false;
 		try
 		{
 			// you can not go below root
-			assertTrue(guard.accept(Application.class, "../../../../test.gif"));
+			assertTrue(guard.accept(Application.class,
+				Packages.absolutePath(Application.class, "../../../../test.gif")));
 		}
 		catch (IllegalArgumentException ex)
 		{

diff --git a/...t-core/src/test/java/org/apache/wicket/markup/html/link/AutolinkPageExpectedResult_2.html b/...t-core/src/test/java/org/apache/wicket/markup/html/link/AutolinkPageExpectedResult_2.html
@@ -27,6 +27,7 @@
 <link href="../resource/org.apache.wicket.markup.html.link.AutolinkPage_2/test.css"/>
 <a href="/root/test.html">Home</a>
 <a href="./org.apache.wicket.markup.html.link.Page1">Home</a>
+<a href="org/apache/wicket/markup/html/link/Page1.html">Home</a>
   <a href="http://www.google.com">Google</a>
 </body>
 </html>
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/AutolinkPage_2.html b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/AutolinkPage_2.html
@@ -26,6 +26,7 @@
 <a href="subdir/Page1.html">Home</a>
 <link href="test.css"/>
 <a href="/root/test.html">Home</a>
+<a href="/org/apache/wicket/markup/html/link/Page1.html">Home</a>
 <a href="org/apache/wicket/markup/html/link/Page1.html">Home</a>
   <a href="http://www.google.com">Google</a>
 </body>

diff --git a/wicket-util/src/main/java/org/apache/wicket/util/lang/Packages.java b/wicket-util/src/main/java/org/apache/wicket/util/lang/Packages.java
@@ -27,31 +27,31 @@
 public final class Packages
 {
 	/**
-	 * Takes a package and a relative path to a resource and returns an absolute path to the
-	 * resource. For example, if the given package was java.lang and the relative path was
-	 * "../util/List", then "java/util/List" would be returned.
+	 * Takes a package and a path to a resource and returns an absolute path to the resource.
+	 * <p>
+	 * See {@link #absolutePath(String, String)} for details.
 	 * 
 	 * @param p
 	 *            The package to start at
-	 * @param relativePath
-	 *            The relative path to the class
+	 * @param path
+	 *            The path to the resource
 	 * @return The absolute path
 	 */
-	public static String absolutePath(final Class<?> p, final String relativePath)
+	public static String absolutePath(final Class<?> p, final String path)
 	{
 		String packName = (p != null ? extractPackageName(p) : "");
-		return absolutePath(packName, relativePath);
+		return absolutePath(packName, path);
 	}
 
 	/**
-	 * Takes a package and a relative path to a resource and returns an absolute path to the
-	 * resource. For example, if the given package was java.lang and the relative path was
-	 * "../util/List", then "java/util/List" would be returned.
+	 * Takes a package and a path to a resource and returns an absolute path to the resource.
+	 * <p>
+	 * See {@link #absolutePath(String, String)} for details.
 	 * 
 	 * @param p
 	 *            The package to start at
 	 * @param relativePath
-	 *            The relative path to the class
+	 *            The path to the resource
 	 * @return The absolute path
 	 */
 	public static String absolutePath(final Package p, final String relativePath)
@@ -60,30 +60,32 @@ public static String absolutePath(final Package p, final String relativePath)
 	}
 
 	/**
-	 * Takes a package and a relative path to a resource and returns an absolute path to the
-	 * resource. For example, if the given package was java.lang and the relative path was
-	 * "../util/List", then "java/util/List" would be returned.
+	 * Takes a package and a path to a resource and returns an absolute path to the resource. For
+	 * example, if the given package was java.lang and the relative path was "../util/List", then
+	 * "java/util/List" would be returned. An already absolute path stays absolute.
+	 * <p>
+	 * Note: The returned absolute path does not start with a slash ("/").
 	 * 
 	 * @param packageName
 	 *            The package to start at
-	 * @param relativePath
-	 *            The relative path to the class
+	 * @param path
+	 *            The path to the resource
 	 * @return The absolute path
 	 */
-	public static String absolutePath(final String packageName, final String relativePath)
+	public static String absolutePath(final String packageName, final String path)
 	{
 		// Is path already absolute?
-		if (relativePath.startsWith("/"))
+		if (path.startsWith("/"))
 		{
-			return relativePath;
+			return path.substring(1);
 		}
 		else
 		{
 			// Break package into list of package names
 			final StringList absolutePath = StringList.tokenize(packageName, ".");
 
 			// Break path into folders
-			final StringList folders = StringList.tokenize(relativePath, "/\\");
+			final StringList folders = StringList.tokenize(path, "/\\");
 
 			// Iterate through folders
 			for (int i = 0, size = folders.size(); i < size; i++)
@@ -101,10 +103,10 @@ public static String absolutePath(final String packageName, final String relativ
 					}
 					else
 					{
-						throw new IllegalArgumentException("Invalid path " + relativePath);
+						throw new IllegalArgumentException("Invalid path " + path);
 					}
 				}
-				else if (absolutePath.size() <= i || absolutePath.get(i).equals(folder) == false)
+				else
 				{
 					// Add to stack
 					absolutePath.add(folder);

diff --git a/wicket-util/src/test/java/org/apache/wicket/util/lang/PackagesTest.java b/wicket-util/src/test/java/org/apache/wicket/util/lang/PackagesTest.java
@@ -24,14 +24,24 @@
  */
 public class PackagesTest extends Assert
 {
+	@Test
+	public void absolutePath0() throws Exception
+	{
+		String packageName = "org.apache.wicket.util.tester";
+		String relativePath = "/org/apache/wicket/util/tester/BlockedResourceLinkPage.html";
+
+		String absolutePath = Packages.absolutePath(packageName, relativePath);
+		assertEquals("org/apache/wicket/util/tester/BlockedResourceLinkPage.html", absolutePath);
+	}
+
 	@Test
 	public void absolutePath1() throws Exception
 	{
 		String packageName = "org.apache.wicket.util.tester";
-		String relativePath = "org/apache/wicket/util/tester/BlockedResourceLinkPage.html";
+		String relativePath = "BlockedResourceLinkPage.html";
 
 		String absolutePath = Packages.absolutePath(packageName, relativePath);
-		assertEquals(relativePath, absolutePath);
+		assertEquals("org/apache/wicket/util/tester/BlockedResourceLinkPage.html", absolutePath);
 	}
 
 	@Test
@@ -63,4 +73,17 @@ public void absolutePath4() throws Exception
 		String absolutePath = Packages.absolutePath(packageName, relativePath);
 		assertEquals("org/apache/BlockedResourceLinkPage.html", absolutePath);
 	}
+
+	/**
+	 * WICKET-5054
+	 */
+	@Test
+	public void absolutePath5() throws Exception
+	{
+		String packageName = "com.foo.bar";
+		String relativePath = "baz/foo/qux";
+
+		String absolutePath = Packages.absolutePath(packageName, relativePath);
+		assertEquals("com/foo/bar/baz/foo/qux", absolutePath);
+	}
 }