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

Reply via email to