You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by sv...@apache.org on 2013/02/24 10:50:40 UTC

git commit: 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

Updated Branches:
  refs/heads/master a86f842de -> 5a3e0b7d4


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

Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/5a3e0b7d
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/5a3e0b7d
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/5a3e0b7d

Branch: refs/heads/master
Commit: 5a3e0b7d45c489d58ac8a630341e65f3e567ea24
Parents: a86f842
Author: svenmeier <sv...@apache.org>
Authored: Sun Feb 24 10:50:08 2013 +0100
Committer: svenmeier <sv...@apache.org>
Committed: Sun Feb 24 10:50:08 2013 +0100

----------------------------------------------------------------------
 .../wicket/markup/html/IPackageResourceGuard.java  |    8 +-
 .../wicket/markup/html/PackageResourceGuard.java   |    6 +-
 .../wicket/ParentResourceEscapePathTest.java       |    5 ++
 .../wicket/markup/html/PackageResourceTest.java    |   43 ++++++++------
 .../html/SecurePackageResourceGuardTest.java       |   24 +++++---
 .../html/link/AutolinkPageExpectedResult_2.html    |    1 +
 .../wicket/markup/html/link/AutolinkPage_2.html    |    1 +
 .../java/org/apache/wicket/util/lang/Packages.java |   46 ++++++++-------
 .../org/apache/wicket/util/lang/PackagesTest.java  |   27 ++++++++-
 9 files changed, 103 insertions(+), 58 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/wicket/blob/5a3e0b7d/wicket-core/src/main/java/org/apache/wicket/markup/html/IPackageResourceGuard.java
----------------------------------------------------------------------
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
index 8607d37..9c90af6 100644
--- 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);
 }

http://git-wip-us.apache.org/repos/asf/wicket/blob/5a3e0b7d/wicket-core/src/main/java/org/apache/wicket/markup/html/PackageResourceGuard.java
----------------------------------------------------------------------
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
index 393c6a6..94156f3 100644
--- 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.HashSet;
 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 class PackageResourceGuard implements IPackageResourceGuard
 	 * @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);
 	}
 

http://git-wip-us.apache.org/repos/asf/wicket/blob/5a3e0b7d/wicket-core/src/test/java/org/apache/wicket/ParentResourceEscapePathTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/ParentResourceEscapePathTest.java b/wicket-core/src/test/java/org/apache/wicket/ParentResourceEscapePathTest.java
index 1f29840..eac7f42 100644
--- 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 @@ package org.apache.wicket;
 
 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 @@ public class ParentResourceEscapePathTest extends WicketTestCase
 	@Test
 	public void requestHandlingOfResourceUrlWithEscapeStringInsideTest()
 	{
+		((PackageResourceGuard)tester.getApplication()
+			.getResourceSettings()
+			.getPackageResourceGuard()).setAllowAccessToRootResources(true);
+
 		tester.getApplication().getResourceSettings().setParentFolderPlaceholder("-updir-");
 		requestHandlingOfResourceUrlWithEscapeStringInside();
 

http://git-wip-us.apache.org/repos/asf/wicket/blob/5a3e0b7d/wicket-core/src/test/java/org/apache/wicket/markup/html/PackageResourceTest.java
----------------------------------------------------------------------
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
index 056fad9..a995e4e 100644
--- 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.SharedResources;
 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 class PackageResourceTest extends WicketTestCase
 		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 class PackageResourceTest extends WicketTestCase
 	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 class PackageResourceTest extends WicketTestCase
 	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();

http://git-wip-us.apache.org/repos/asf/wicket/blob/5a3e0b7d/wicket-core/src/test/java/org/apache/wicket/markup/html/SecurePackageResourceGuardTest.java
----------------------------------------------------------------------
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
index 158b08b..3c859ce 100644
--- 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 @@ package org.apache.wicket.markup.html;
 
 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 class SecurePackageResourceGuardTest extends WicketTestCase
 		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)
 		{

http://git-wip-us.apache.org/repos/asf/wicket/blob/5a3e0b7d/wicket-core/src/test/java/org/apache/wicket/markup/html/link/AutolinkPageExpectedResult_2.html
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/AutolinkPageExpectedResult_2.html b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/AutolinkPageExpectedResult_2.html
index 8be4563..a1e1633 100644
--- a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/AutolinkPageExpectedResult_2.html
+++ b/wicket-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>

http://git-wip-us.apache.org/repos/asf/wicket/blob/5a3e0b7d/wicket-core/src/test/java/org/apache/wicket/markup/html/link/AutolinkPage_2.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
index 46e2e11..834d6d4 100644
--- 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>

http://git-wip-us.apache.org/repos/asf/wicket/blob/5a3e0b7d/wicket-util/src/main/java/org/apache/wicket/util/lang/Packages.java
----------------------------------------------------------------------
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
index e089bab..f3a66fa 100755
--- 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 @@ import org.apache.wicket.util.string.StringList;
 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,22 +60,24 @@ 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. 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
 		{
@@ -83,7 +85,7 @@ public final class Packages
 			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 final class Packages
 					}
 					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);

http://git-wip-us.apache.org/repos/asf/wicket/blob/5a3e0b7d/wicket-util/src/test/java/org/apache/wicket/util/lang/PackagesTest.java
----------------------------------------------------------------------
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
index 20c9f50..e676e7f 100644
--- 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
@@ -25,13 +25,23 @@ import org.junit.Test;
 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 class PackagesTest extends Assert
 		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);
+	}
 }