This is an automated email from the ASF dual-hosted git repository. pedro pushed a commit to branch wicket-8.x in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 00acced5e78f19fb3c536fa3361e45048c778fc2 Author: Pedro Santos <pe...@apache.org> AuthorDate: Mon Nov 4 12:59:01 2024 -0300 WICKET-7024 decode PackageResourceReference URL attributes only inside requests for it - URL parameters sanitization - URL sanitization for not mounted resources - skip cache update if resource is not found --- .../mapper/BasicResourceReferenceMapper.java | 15 ++- .../caching/CachingResourceStreamLocator.java | 14 ++- .../wicket/request/resource/PackageResource.java | 49 +++++++- .../request/resource/PackageResourceReference.java | 15 ++- .../mapper/BasicResourceReferenceMapperTest.java | 17 +++ .../resource/PackageResourceReferenceTest.java | 136 +++++++++++++++++++++ .../org/apache/wicket/request/resource/a_blue.css | 3 + .../apache/wicket/request/resource/a_orange.css | 3 + 8 files changed, 236 insertions(+), 16 deletions(-) diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapper.java b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapper.java index 2c533bc59b..73edb9c610 100755 --- a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapper.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapper.java @@ -27,10 +27,7 @@ import org.apache.wicket.request.Url; import org.apache.wicket.request.handler.resource.ResourceReferenceRequestHandler; import org.apache.wicket.request.mapper.parameter.IPageParametersEncoder; import org.apache.wicket.request.mapper.parameter.PageParameters; -import org.apache.wicket.request.resource.IResource; -import org.apache.wicket.request.resource.MetaInfStaticResourceReference; -import org.apache.wicket.request.resource.ResourceReference; -import org.apache.wicket.request.resource.ResourceReferenceRegistry; +import org.apache.wicket.request.resource.*; import org.apache.wicket.request.resource.caching.IResourceCachingStrategy; import org.apache.wicket.request.resource.caching.IStaticCacheableResource; import org.apache.wicket.request.resource.caching.ResourceUrl; @@ -135,9 +132,17 @@ public class BasicResourceReferenceMapper extends AbstractResourceReferenceMappe if (scope != null && scope.getPackage() != null) { + ResourceReference.UrlAttributes sanitized = PackageResource.sanitize(attributes, scope, name.toString()); + boolean createIfNotFound = false; + if (sanitized != null) + { + attributes = sanitized; + createIfNotFound = true; + } + ResourceReference res = getContext().getResourceReferenceRegistry() .getResourceReference(scope, name.toString(), attributes.getLocale(), - attributes.getStyle(), attributes.getVariation(), true, true); + attributes.getStyle(), attributes.getVariation(), true, createIfNotFound); if (res != null) { diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/caching/CachingResourceStreamLocator.java b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/caching/CachingResourceStreamLocator.java index f9b3624320..7cc3a45684 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/caching/CachingResourceStreamLocator.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/caching/CachingResourceStreamLocator.java @@ -109,9 +109,18 @@ public class CachingResourceStreamLocator implements IResourceStreamLocator } } + /** + * @deprecated + */ @Override public IResourceStream locate(Class<?> scope, String path, String style, String variation, Locale locale, String extension, boolean strict) + { + return locate(scope, path, style, variation, locale, extension, strict, true); + } + + public IResourceStream locate(Class<?> scope, String path, String style, String variation, + Locale locale, String extension, boolean strict, boolean updateCache) { CacheKey key = new CacheKey(scope.getName(), path, extension, locale, style, variation, strict); IResourceStreamReference resourceStreamReference = cache.get(key); @@ -121,7 +130,10 @@ public class CachingResourceStreamLocator implements IResourceStreamLocator { result = delegate.locate(scope, path, style, variation, locale, extension, strict); - updateCache(key, result); + if (updateCache) + { + updateCache(key, result); + } } else { diff --git a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java index 614d6f228d..b766dd0097 100644 --- a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java +++ b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java @@ -33,6 +33,7 @@ import org.apache.wicket.Session; import org.apache.wicket.WicketRuntimeException; import org.apache.wicket.core.util.lang.WicketObjects; import org.apache.wicket.core.util.resource.locator.IResourceStreamLocator; +import org.apache.wicket.core.util.resource.locator.caching.CachingResourceStreamLocator; import org.apache.wicket.javascript.IJavaScriptCompressor; import org.apache.wicket.markup.html.IPackageResourceGuard; import org.apache.wicket.mock.MockWebRequest; @@ -703,12 +704,26 @@ public class PackageResource extends AbstractResource implements IStaticCacheabl */ public static boolean exists(final Class<?> scope, final String path, final Locale locale, final String style, final String variation) + { + return getResourceStream(scope, path, locale, style, variation, true) != null; + } + + private static IResourceStream getResourceStream(final Class<?> scope, final String path, final Locale locale, + final String style, final String variation, final boolean updateCache) { String absolutePath = Packages.absolutePath(scope, path); - return Application.get() - .getResourceSettings() - .getResourceStreamLocator() - .locate(scope, absolutePath, style, variation, locale, null, false) != null; + IResourceStreamLocator resourceStreamLocator = Application.get().getResourceSettings() + .getResourceStreamLocator(); + if (resourceStreamLocator instanceof CachingResourceStreamLocator) + { + CachingResourceStreamLocator cache = (CachingResourceStreamLocator)resourceStreamLocator; + return cache.locate(scope, absolutePath, style, variation, locale, null, false, + updateCache); + } + else + { + return resourceStreamLocator.locate(scope, absolutePath, style, variation, locale, null, false); + } } @Override @@ -857,4 +872,30 @@ public class PackageResource extends AbstractResource implements IStaticCacheabl this.readBuffered = readBuffered; return this; } + + /** + * @return UrlAttributes with an existent locale/style/variation if a resource is bound to the + * scope+name, otherwise returns null + */ + public static ResourceReference.UrlAttributes sanitize( + ResourceReference.UrlAttributes urlAttributes, Class<?> scope, String name) + { + IResourceStream filesystemMatch = getResourceStream(scope, name, urlAttributes.getLocale(), + urlAttributes.getStyle(), urlAttributes.getVariation(), false); + if (filesystemMatch == null) + { + return null; + } + try + { + filesystemMatch.close(); + } + catch (IOException e) + { + log.error("failed to close", e); + } + return new ResourceReference.UrlAttributes(filesystemMatch.getLocale(), + filesystemMatch.getStyle(), filesystemMatch.getVariation()); + } + } diff --git a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java index f10a01cb83..3a5958cfd9 100644 --- a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java +++ b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java @@ -26,6 +26,7 @@ import org.apache.wicket.Session; import org.apache.wicket.core.util.resource.locator.IResourceStreamLocator; import org.apache.wicket.request.Url; import org.apache.wicket.request.cycle.RequestCycle; +import org.apache.wicket.request.handler.resource.ResourceReferenceRequestHandler; import org.apache.wicket.resource.ResourceUtil; import org.apache.wicket.util.lang.Generics; import org.apache.wicket.util.lang.Packages; @@ -109,23 +110,25 @@ public class PackageResourceReference extends ResourceReference public PackageResource getResource() { final String extension = getExtension(); + final Class<?> scope = getScope(); + final String name = getName(); final PackageResource resource; RequestCycle requestCycle = RequestCycle.get(); UrlAttributes urlAttributes = null; - if (requestCycle != null) + if (requestCycle != null + && requestCycle.getActiveRequestHandler() instanceof ResourceReferenceRequestHandler) { //resource attributes (locale, style, variation) might be encoded in the URL final Url url = requestCycle.getRequest().getUrl(); urlAttributes = ResourceUtil.decodeResourceReferenceAttributes(url); + urlAttributes = PackageResource.sanitize(urlAttributes, scope, name); } - final String currentVariation = getCurrentVariation(urlAttributes); - final String currentStyle = getCurrentStyle(urlAttributes); - final Locale currentLocale = getCurrentLocale(urlAttributes); - final Class<?> scope = getScope(); - final String name = getName(); + String currentVariation = getCurrentVariation(urlAttributes); + String currentStyle = getCurrentStyle(urlAttributes); + Locale currentLocale = getCurrentLocale(urlAttributes); if (CSS_EXTENSION.equals(extension)) { diff --git a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapperTest.java b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapperTest.java index 00e373403e..32d4aca6f9 100644 --- a/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapperTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapperTest.java @@ -41,6 +41,9 @@ import org.apache.wicket.util.ValueProvider; import org.apache.wicket.util.lang.Args; import org.apache.wicket.util.resource.IResourceStream; import org.apache.wicket.util.resource.StringResourceStream; +import org.apache.wicket.util.tester.WicketTester; +import org.junit.After; +import org.junit.Before; import org.junit.Test; /** @@ -61,6 +64,20 @@ public class BasicResourceReferenceMapperTest extends AbstractResourceReferenceM } }; + WicketTester tester; + @Before + public void setup() + { + // set the application ResourceSettings, used by the BasicResourceReferenceMapper + tester = new WicketTester(); + } + + @After + public void destroy() + { + tester.destroy(); + } + /** * */ diff --git a/wicket-core/src/test/java/org/apache/wicket/request/resource/PackageResourceReferenceTest.java b/wicket-core/src/test/java/org/apache/wicket/request/resource/PackageResourceReferenceTest.java index 0af08daeb5..77542eb623 100644 --- a/wicket-core/src/test/java/org/apache/wicket/request/resource/PackageResourceReferenceTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/request/resource/PackageResourceReferenceTest.java @@ -21,7 +21,13 @@ import java.io.InputStream; import java.util.Locale; import org.apache.wicket.Application; +import org.apache.wicket.MarkupContainer; import org.apache.wicket.ThreadContext; +import org.apache.wicket.core.util.resource.UrlResourceStream; +import org.apache.wicket.core.util.resource.locator.IResourceStreamLocator; +import org.apache.wicket.core.util.resource.locator.caching.CachingResourceStreamLocator; +import org.apache.wicket.markup.IMarkupResourceStreamProvider; +import org.apache.wicket.markup.html.WebPage; import org.apache.wicket.protocol.http.mock.MockHttpServletRequest; import org.apache.wicket.protocol.http.mock.MockHttpServletResponse; import org.apache.wicket.request.Request; @@ -31,10 +37,17 @@ import org.apache.wicket.request.resource.IResource.Attributes; import org.apache.wicket.request.resource.ResourceReference.UrlAttributes; import org.apache.wicket.response.ByteArrayResponse; import org.apache.wicket.util.io.IOUtils; +import org.apache.wicket.util.resource.IResourceStream; +import org.apache.wicket.util.resource.StringResourceStream; import org.apache.wicket.util.tester.WicketTestCase; +import org.hamcrest.CoreMatchers; import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; + +import static org.hamcrest.CoreMatchers.*; +import static org.mockito.Mockito.*; /** @@ -375,4 +388,127 @@ public class PackageResourceReferenceTest extends WicketTestCase assertEquals(variations[1], resource.getResourceStream().getVariation()); } + @Test + public void getResourceWithNoStyle() + { + tester.executeUrl( + "wicket/resource/org.apache.wicket.request.resource.PackageResourceReferenceTest/a.css"); + + assertThat(tester.getLastResponseAsString(), not(containsString("color"))); + } + + @Test + public void getStyleFromSession() + { + tester.getSession().setStyle("blue"); + tester.executeUrl( + "wicket/resource/org.apache.wicket.request.resource.PackageResourceReferenceTest/a.css"); + + assertThat(tester.getLastResponseAsString(), containsString("blue")); + } + + @Test + public void decodeStyleFromUrl() + { + tester.getSession().setStyle("blue"); + tester.executeUrl( + "wicket/resource/org.apache.wicket.request.resource.PackageResourceReferenceTest/a.css?-orange"); + + assertThat(tester.getLastResponseAsString(), containsString("orange")); + assertThat(tester.getLastResponseAsString(), not(containsString("blue"))); + } + + @Test + public void doNotFindNullResourceInTheCache() + { + IResourceStreamLocator resourceStreamLocator = mock(IResourceStreamLocator.class); + when(resourceStreamLocator.locate(scope, "org/apache/wicket/request/resource/z.css", + "orange", null, null, null, false)).thenReturn(null); + + tester.getApplication().getResourceSettings() + .setResourceStreamLocator(new CachingResourceStreamLocator(resourceStreamLocator)); + + tester.executeUrl( + "wicket/resource/org.apache.wicket.request.resource.PackageResourceReferenceTest/z.css?-orange"); + tester.executeUrl( + "wicket/resource/org.apache.wicket.request.resource.PackageResourceReferenceTest/z.css?-orange"); + + verify(resourceStreamLocator, times(2)).locate(PackageResourceReferenceTest.class, + "org/apache/wicket/request/resource/z.css", "orange", null, null, null, false); + } + + @Test + public void doNotFindResourceInTheCache() + { + IResourceStreamLocator resourceStreamLocator = mock(IResourceStreamLocator.class); + when(resourceStreamLocator.locate(scope, "org/apache/wicket/request/resource/a.css", + "yellow", null, null, null, false)).thenReturn( + new UrlResourceStream(scope.getResource("a.css"))); + + tester.getApplication().getResourceSettings() + .setResourceStreamLocator(new CachingResourceStreamLocator(resourceStreamLocator)); + + tester.executeUrl( + "wicket/resource/org.apache.wicket.request.resource.PackageResourceReferenceTest/a.css?-yellow"); + tester.executeUrl( + "wicket/resource/org.apache.wicket.request.resource.PackageResourceReferenceTest/a.css?-yellow"); + + verify(resourceStreamLocator, times(2)).locate(PackageResourceReferenceTest.class, + "org/apache/wicket/request/resource/a.css", "yellow", null, null, null, false); + } + + @Test + public void doNotFindMountedResourceInTheCache() + { + IResourceStreamLocator resourceStreamLocator = mock(IResourceStreamLocator.class); + when(resourceStreamLocator.locate(scope, "org/apache/wicket/request/resource/a.css", + "yellow", null, null, null, false)).thenReturn( + new UrlResourceStream(scope.getResource("a.css"))); + + tester.getApplication().getResourceSettings() + .setResourceStreamLocator(new CachingResourceStreamLocator(resourceStreamLocator)); + tester.getApplication() + .mountResource("/a.css", new PackageResourceReference(scope, "a.css")); + + tester.executeUrl("a.css?-yellow"); + tester.executeUrl("a.css?-yellow"); + + verify(resourceStreamLocator, times(2)).locate(scope, + "org/apache/wicket/request/resource/a.css", "yellow", null, null, null, false); + } + + /** + * https://issues.apache.org/jira/browse/WICKET-7024 + */ + @Test + public void notDecodeStyleFromUrl() + { + tester.executeUrl( + "wicket/bookmarkable/org.apache.wicket.request.resource.PackageResourceReferenceTest$TestPage?0-1.0-resumeButton&_=1730041277224"); + + TestPage page = (TestPage)tester.getLastRenderedPage(); + + assertThat(page.resource.getStyle(), not(is("1.0"))); + } + + public static class TestPage extends WebPage implements IMarkupResourceStreamProvider + { + CssPackageResource resource; + + @Override + protected void onConfigure() + { + super.onConfigure(); + resource = (CssPackageResource)new PackageResourceReference(scope, "a.css") + .getResource(); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + return new StringResourceStream("<html><head></head><body></body></html>"); + } + } + } diff --git a/wicket-core/src/test/java/org/apache/wicket/request/resource/a_blue.css b/wicket-core/src/test/java/org/apache/wicket/request/resource/a_blue.css new file mode 100644 index 0000000000..6c4aa1a395 --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/request/resource/a_blue.css @@ -0,0 +1,3 @@ +.a{ + color: blue; +} \ No newline at end of file diff --git a/wicket-core/src/test/java/org/apache/wicket/request/resource/a_orange.css b/wicket-core/src/test/java/org/apache/wicket/request/resource/a_orange.css new file mode 100644 index 0000000000..3282726ba6 --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/request/resource/a_orange.css @@ -0,0 +1,3 @@ +.a{ + color: orange; +} \ No newline at end of file