From 5a3e0b7d45c489d58ac8a630341e65f3e567ea24 Mon Sep 17 00:00:00 2001 From: svenmeier Date: Sun, 24 Feb 2013 10:50:08 +0100 Subject: [PATCH] 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 --- .../markup/html/IPackageResourceGuard.java | 8 ++-- .../markup/html/PackageResourceGuard.java | 6 +-- .../wicket/ParentResourceEscapePathTest.java | 5 ++ .../markup/html/PackageResourceTest.java | 43 +++++++++-------- .../html/SecurePackageResourceGuardTest.java | 24 ++++++---- .../link/AutolinkPageExpectedResult_2.html | 1 + .../markup/html/link/AutolinkPage_2.html | 1 + .../org/apache/wicket/util/lang/Packages.java | 46 ++++++++++--------- .../apache/wicket/util/lang/PackagesTest.java | 27 ++++++++++- 9 files changed, 103 insertions(+), 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 index 8607d3778a0..9c90af60e20 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); } 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 393c6a63033..94156f3a079 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.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 index 1f29840c775..eac7f42fd5c 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 @@ 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 index 056fad93141..a995e4eff5c 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.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 index 158b08b8968..3c859ce71c7 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 @@ 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/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 8be45639ca7..a1e1633fdee 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 @@ Home Home +Home Google 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 46e2e11f4f7..834d6d4fa65 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 @@ Home Home +Home Home Google 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 e089bab2c40..f3a66fa4f89 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 @@ 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. + *

+ * 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. + *

+ * 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 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. + *

+ * 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 static String absolutePath(final String packageName, final String relativ 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 index 20c9f508d4a..e676e7f4c23 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 @@ -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); + } }