You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by an...@apache.org on 2007/09/21 05:38:21 UTC
svn commit: r577975 - in /tapestry/tapestry4/trunk/tapestry-framework/src:
java/org/apache/tapestry/asset/AssetService.java
test/org/apache/tapestry/asset/TestAssetService.java
test/org/apache/tapestry/asset/TestResourceDigestSource.java
Author: andyhot
Date: Thu Sep 20 20:38:20 2007
New Revision: 577975
URL: http://svn.apache.org/viewvc?rev=577975&view=rev
Log:
TAPESTRY-1306: no excessive server output on requesting incorrect paths from the asset service - also, return 404 even when resource exists but digest is wrong
Modified:
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/asset/AssetService.java
tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/asset/TestAssetService.java
tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/asset/TestResourceDigestSource.java
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/asset/AssetService.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/asset/AssetService.java?rev=577975&r1=577974&r2=577975&view=diff
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/asset/AssetService.java (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/asset/AssetService.java Thu Sep 20 20:38:20 2007
@@ -210,7 +210,7 @@
public void service(IRequestCycle cycle)
throws IOException
{
- String path = cycle.getParameter(PATH);
+ String path = translatePath(cycle.getParameter(PATH));
String md5Digest = cycle.getParameter(DIGEST);
boolean checkDigest = !_unprotectedMatcher.containsResource(path);
@@ -218,9 +218,19 @@
try
{
+ URL resourceURL = _classResolver.getResource(path);
+
+ if (resourceURL == null)
+ {
+ _response.setStatus(HttpServletResponse.SC_NOT_FOUND);
+ _log.info(AssetMessages.noSuchResource(path));
+ return;
+ }
+
if (checkDigest && !_digestSource.getDigestForResource(path).equals(md5Digest))
{
- _response.sendError(HttpServletResponse.SC_FORBIDDEN, AssetMessages.md5Mismatch(path));
+ _response.setStatus(HttpServletResponse.SC_NOT_FOUND);
+ _log.info(AssetMessages.md5Mismatch(path));
return;
}
@@ -233,15 +243,6 @@
return;
}
- URL resourceURL = _classResolver.getResource(translatePath(path));
-
- if (resourceURL == null)
- {
- _response.setStatus(HttpServletResponse.SC_NOT_FOUND);
- _log.warn(AssetMessages.noSuchResource(path));
- return;
- }
-
resourceConnection = resourceURL.openConnection();
// check caching for unprotected resources
@@ -257,7 +258,9 @@
}
catch (Throwable ex)
{
- _exceptionReporter.reportRequestException(AssetMessages.exceptionReportTitle(path), ex);
+ _response.setStatus(HttpServletResponse.SC_NOT_FOUND);
+ _log.warn(AssetMessages.exceptionReportTitle(path), ex);
+ //_exceptionReporter.reportRequestException(AssetMessages.exceptionReportTitle(path), ex);
}
}
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/asset/TestAssetService.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/asset/TestAssetService.java?rev=577975&r1=577974&r2=577975&view=diff
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/asset/TestAssetService.java (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/asset/TestAssetService.java Thu Sep 20 20:38:20 2007
@@ -15,13 +15,16 @@
import org.apache.commons.logging.LogFactory;
import org.apache.hivemind.ClassResolver;
+import org.apache.hivemind.ApplicationRuntimeException;
import org.apache.hivemind.impl.DefaultClassResolver;
import org.apache.tapestry.IRequestCycle;
import org.apache.tapestry.TestBase;
+import org.apache.tapestry.error.RequestExceptionReporter;
import org.apache.tapestry.util.ContentType;
import org.apache.tapestry.web.WebContext;
import org.apache.tapestry.web.WebRequest;
import org.apache.tapestry.web.WebResponse;
+import org.easymock.EasyMock;
import static org.easymock.EasyMock.checkOrder;
import static org.easymock.EasyMock.expect;
import org.testng.annotations.Test;
@@ -116,6 +119,7 @@
public void test_ETag_Header_Response()
throws Exception
{
+ String requestedResource = "/org/apache/tapestry/asset/tapestry-in-action.png";
WebRequest request = newMock(WebRequest.class);
checkOrder(request, false);
WebResponse response = newMock(WebResponse.class);
@@ -124,7 +128,7 @@
ResourceMatcher matcher = newMock(ResourceMatcher.class);
ClassResolver resolver = new DefaultClassResolver();
- URLConnection url = resolver.getResource("/org/apache/tapestry/asset/tapestry-in-action.png").openConnection();
+ URLConnection url = resolver.getResource(requestedResource).openConnection();
AssetService service = new AssetService();
service.setRequest(request);
@@ -134,13 +138,13 @@
service.setClassResolver(resolver);
service.setContext(context);
- expect(cycle.getParameter("path")).andReturn("/org/apache/tapestry/asset/tapestry-in-action.png");
+ expect(cycle.getParameter("path")).andReturn(requestedResource);
expect(cycle.getParameter("digest")).andReturn(null);
- expect(matcher.containsResource("/org/apache/tapestry/asset/tapestry-in-action.png")).andReturn(true);
+ expect(matcher.containsResource(requestedResource)).andReturn(true);
expect(request.getDateHeader("If-Modified-Since")).andReturn(-1L);
- expect(context.getMimeType("/org/apache/tapestry/asset/tapestry-in-action.png")).andReturn("image/png");
+ expect(context.getMimeType(requestedResource)).andReturn("image/png");
response.setDateHeader("Last-Modified", url.getLastModified());
response.setDateHeader("Expires", service._expireTime);
@@ -158,5 +162,53 @@
service.service(cycle);
verify();
+ }
+
+ public void test_Invalid_Resource()
+ throws Exception
+ {
+ String requestedResource = "/org/apache/tapestry/asset/tapestry-in-action-missing.png";
+ WebRequest request = newMock(WebRequest.class);
+ checkOrder(request, false);
+ WebResponse response = newMock(WebResponse.class);
+ WebContext context = newMock(WebContext.class);
+ IRequestCycle cycle = newMock(IRequestCycle.class);
+ ResourceMatcher matcher = newMock(ResourceMatcher.class);
+ ResourceDigestSource digestSource = newMock(ResourceDigestSource.class);
+ RequestExceptionReporter exceptionReporter = newMock(RequestExceptionReporter.class);
+
+ // digester throws exception for invalid resources
+ expect(digestSource.getDigestForResource(requestedResource))
+ .andThrow(new ApplicationRuntimeException("error"))
+ .anyTimes();
+ // in which case the exception reporter has to show them
+ exceptionReporter.reportRequestException((String)EasyMock.anyObject(), (Throwable)EasyMock.anyObject());
+ EasyMock.expectLastCall().anyTimes();
+
+ ClassResolver resolver = new DefaultClassResolver();
+
+ AssetService service = new AssetService();
+ service.setRequest(request);
+ service.setResponse(response);
+ service.setLog(LogFactory.getLog("test"));
+ service.setUnprotectedMatcher(matcher);
+ service.setClassResolver(resolver);
+ service.setContext(context);
+ service.setDigestSource(digestSource);
+ service.setExceptionReporter(exceptionReporter);
+
+ expect(cycle.getParameter("path")).andReturn(requestedResource);
+ expect(cycle.getParameter("digest")).andReturn(null);
+
+ expect(matcher.containsResource(requestedResource)).andReturn(false);
+
+ // make sure that a 404 is sent - instead of an exception thrown
+ response.setStatus(HttpServletResponse.SC_NOT_FOUND);
+
+ replay();
+
+ service.service(cycle);
+
+ verify();
}
}
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/asset/TestResourceDigestSource.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/asset/TestResourceDigestSource.java?rev=577975&r1=577974&r2=577975&view=diff
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/asset/TestResourceDigestSource.java (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/asset/TestResourceDigestSource.java Thu Sep 20 20:38:20 2007
@@ -58,6 +58,24 @@
}
}
+ public void testFolderInJar()
+ {
+ ResourceDigestSourceImpl s = new ResourceDigestSourceImpl();
+ s.setClassResolver(new DefaultClassResolver());
+ // the next will throw a NPE inside JarURLInputStream.close
+ // there doesn't seem to be a way to prevent this - so users
+ // of this method should be aware
+ try
+ {
+ s.getDigestForResource("/org/apache/hivemind");
+ fail("digests for packages in jars are known to throw NPE");
+ }
+ catch (NullPointerException e)
+ {
+ // ignore
+ }
+ }
+
public void testCache()
{
ClassResolver resolver = newMock(ClassResolver.class);