Author: chris.laprun(a)jboss.com Date: 2008-04-04 19:47:59 -0400 (Fri, 04 Apr 2008) New Revision: 10506 Modified: branches/JBoss_Portal_Branch_2_6/wsrp/build.xml branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/RenderHandlerTestCase.java branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/other/WSRPPortletURLTestCase.java branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/WSRPPortletURL.java branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/WSRPResourceURL.java branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RenderHandler.java Log: - Improved robustness of WSRPPortletURLs (more error checking) in particular with respect to resource URLs. - Added test cases and activated RenderHandlerTestCase. - Minor improvements. Modified: branches/JBoss_Portal_Branch_2_6/wsrp/build.xml =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/build.xml 2008-04-04 21:38:18 UTC (rev 10505) +++ branches/JBoss_Portal_Branch_2_6/wsrp/build.xml 2008-04-04 23:47:59 UTC (rev 10506) @@ -841,6 +841,7 @@ <test todir="${test.reports}" name="org.jboss.portal.test.wsrp.consumer.ProducerInfoTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.wsrp.consumer.RegistrationInfoTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.wsrp.consumer.RegistrationPropertyTestCase"/> + <test todir="${test.reports}" name="org.jboss.portal.test.wsrp.consumer.RenderHandlerTestCase"/> <zest todir="${test.reports}" name="org.jboss.portal.test.wsrp.consumer.ConsumerRegistryTestCase" outfile="TEST-ConsumerRegistryTestCase"> <parameter name="Config" value="persistent-jboss-beans.xml"/> Modified: branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/RenderHandlerTestCase.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/RenderHandlerTestCase.java 2008-04-04 21:38:18 UTC (rev 10505) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/consumer/RenderHandlerTestCase.java 2008-04-04 23:47:59 UTC (rev 10506) @@ -34,8 +34,7 @@ */ public class RenderHandlerTestCase extends TestCase { - - public void testResourceURLRewriter() + public void testResourceURLRewriterProcessNonResource() { String markup = "<a href=\"/portal/portal/default/Test/EXAMPLE/EXAMPLE?action=1d&windowstate=&mode=" + "&ns=_next%3D%2Fdk%2Fskat%2Fportal%2Ffront%2Fportlets%2Fexample%2Findex.jsp" + Modified: branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/other/WSRPPortletURLTestCase.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/other/WSRPPortletURLTestCase.java 2008-04-04 21:38:18 UTC (rev 10505) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/test/wsrp/other/WSRPPortletURLTestCase.java 2008-04-04 23:47:59 UTC (rev 10506) @@ -27,6 +27,7 @@ import org.jboss.portal.wsrp.WSRPActionURL; import org.jboss.portal.wsrp.WSRPPortletURL; import org.jboss.portal.wsrp.WSRPRenderURL; +import org.jboss.portal.wsrp.WSRPRewritingConstants; /** * @author <a href="mailto:chris.laprun@jboss.com">Chris Laprun</a> @@ -35,18 +36,13 @@ */ public class WSRPPortletURLTestCase extends TestCase { - /** Load a resource http://test.com/images/test.gif */ public void testResource() { - // julien : cannot be tested properly for now + String expected = "wsrp_rewrite?wsrp-urlType=resource&amp;wsrp-url=http%3A%2F%2Ftest.com%2Fimages%2Ftest.gif" + + "&amp;wsrp-requiresRewrite=true/wsrp_rewrite"; + WSRPPortletURL url = WSRPPortletURL.create(expected); -// String expected = "wsrp_rewrite?wsrp-urlType=resource&amp;wsrp-url=http%3A%2F%2Ftest.com%2Fimages%2Ftest.gif" + -// "&amp;wsrp-requiresRewrite=true/wsrp_rewrite"; -// WSRPPortletURL url = WSRPPortletURL.create(expected); -// -// assertEquals(WSRPRewritingConstants.URL_TYPE_RESOURCE, url.getURLType()); -// assertEquals("http%3A%2F%2Ftest.com%2Fimages%2Ftest.gif", url.getRawParameterValueFor(WSRPRewritingConstants.RESOURCE_URL)); -// assertEquals("true", url.getRawParameterValueFor(WSRPRewritingConstants.RESOURCE_REQUIRES_REWRITE)); + assertEquals("http://test.com/images/test.gif";, url.toString()); } /** Declare a secure interaction back to the Portlet */ @@ -82,16 +78,49 @@ assertTrue(url instanceof WSRPRenderURL); } - public void testRobustness() + public void testInvalidParameterName() { - String wrongURLType = "wsrp_rewrite?wsrp-urlType=pipo&amp;wsrp-mode=help/wsrp_rewrite"; + String message = "Should have detected invalid parameter: "; + String invalid = "wsrp_rewrite?wsrp-urlType=render&amp;amp;wsrp-url=http%3A%2F%2Flocalhost%2F/wsrp_rewrite"; + checkInvalidURL(invalid, message, "amp;wsrp-url"); + + invalid = "wsrp_rewrite?wsrp-urlType=render&amp;foo=bar/wsrp_rewrite"; + checkInvalidURL(invalid, message, "foo"); + } + + public void testInvalidResourceURLV1() + { + String message = "Should have detected missing parameter: "; + + String invalid = "wsrp_rewrite?wsrp-urlType=resource&amp;wsrp-url=http%3A%2F%2Flocalhost%2F/wsrp_rewrite"; + checkInvalidURL(invalid, message, WSRPRewritingConstants.RESOURCE_REQUIRES_REWRITE); + + invalid = "wsrp_rewrite?wsrp-urlType=resource&amp;wsrp-requiresRewrite=true/wsrp_rewrite"; + checkInvalidURL(invalid, message, WSRPRewritingConstants.RESOURCE_URL); + + invalid = "wsrp_rewrite?wsrp-urlType=resource&amp;wsrp-url=invalidURL&amp;wsrp-requiresRewrite=true/wsrp_rewrite"; + checkInvalidURL(invalid, "Should have detected invalid URL: ", "invalidURL"); + } + + private void checkInvalidURL(String invalid, String message, String mustBeInException) + { try { - WSRPPortletURL url = WSRPPortletURL.create(wrongURLType); - fail("Should have detected wrong URL type"); + WSRPPortletURL.create(invalid); + fail(message + mustBeInException); + } + catch (IllegalArgumentException e) + { + assertTrue(e.getLocalizedMessage().contains(mustBeInException)); + } + } - url = WSRPPortletURL.create(null); + public void testNullURL() + { + try + { + WSRPPortletURL.create(null); fail("Should have detected null URL"); } catch (IllegalArgumentException e) @@ -99,4 +128,19 @@ // expected } } + + public void testInvalidURLType() + { + String wrongURLType = "wsrp_rewrite?wsrp-urlType=pipo&amp;wsrp-mode=help/wsrp_rewrite"; + + try + { + WSRPPortletURL.create(wrongURLType); + fail("Should have detected wrong URL type"); + } + catch (IllegalArgumentException e) + { + // expected + } + } } Modified: branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/WSRPPortletURL.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/WSRPPortletURL.java 2008-04-04 21:38:18 UTC (rev 10505) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/WSRPPortletURL.java 2008-04-04 23:47:59 UTC (rev 10506) @@ -125,7 +125,7 @@ } // standardize parameter separators - encodedURL = Tools.replace(encodedURL, "&amp;", PARAM_SEPARATOR); + encodedURL = Tools.replace(encodedURL, AMPERSAND, PARAM_SEPARATOR); encodedURL = Tools.replace(encodedURL, "&", PARAM_SEPARATOR); // this second shouldn't be used but in case it is... // remove url type param name and extract value @@ -153,10 +153,10 @@ throw new IllegalArgumentException("Unrecognized URL type: " + encodedURL.substring(0, encodedURL.indexOf(PARAM_SEPARATOR))); } - // starting from now, everything else is optional so need to check if we still have some chars to parse + // other parameters Map params = null; int urlTypeLength = urlType.length(); - if (encodedURL.length() != urlTypeLength) + if (encodedURL.length() > urlTypeLength) { // truncate again once the value is extracted encodedURL = encodedURL.substring(urlTypeLength + 1); // +1 for the param separator @@ -164,6 +164,11 @@ // extract the other parameters params = extractParams(encodedURL, originalURL); } + else if (WSRPRewritingConstants.URL_TYPE_RESOURCE.equals(urlType)) + { + throw new IllegalArgumentException("Both the 'wsrp-url' and 'wsrp-requiresRewrite' parameters MUST also be specified for resource URL '" + + originalURL + "'"); + } url.setParams(params); return url; @@ -205,7 +210,7 @@ paramValue = getRawParameterValueFor(params, WSRPRewritingConstants.SECURE_URL); if (paramValue != null) { - secure = Boolean.valueOf(paramValue).booleanValue(); + secure = Boolean.valueOf(paramValue); } } @@ -286,7 +291,7 @@ private static Map extractParams(String encodedURL, String originalURL) { - Map params = new HashMap(); + Map<String, String> params = new HashMap<String, String>(); boolean finished = false; while (encodedURL.length() > 0 && !finished) { @@ -310,6 +315,10 @@ } String name = param.substring(0, equalsIndex); + if (!name.startsWith("wsrp-")) + { + throw new IllegalArgumentException("Invalid parameter name: '" + name + "'"); + } String value = param.substring(equalsIndex + 1, param.length()); // +1 to ignore "=" params.put(name, value); encodedURL = encodedURL.substring(endParamIndex + 1); // +1 for the param separator Modified: branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/WSRPResourceURL.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/WSRPResourceURL.java 2008-04-04 21:38:18 UTC (rev 10505) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/WSRPResourceURL.java 2008-04-04 23:47:59 UTC (rev 10506) @@ -47,7 +47,7 @@ private URL resourceURL; private boolean requiresRewrite = false; - private static final List SUPPORTED_RESOURCE_TYPES = new ArrayList(4); + private static final List<MediaType> SUPPORTED_RESOURCE_TYPES = new ArrayList<MediaType>(4); static { @@ -108,8 +108,13 @@ String paramValue = getRawParameterValueFor(params, WSRPRewritingConstants.RESOURCE_REQUIRES_REWRITE); if (paramValue != null) { - requiresRewrite = Boolean.valueOf(paramValue).booleanValue(); + requiresRewrite = Boolean.valueOf(paramValue); } + else + { + throw new IllegalArgumentException("The parsed parameters don't contain a value for the required " + + WSRPRewritingConstants.RESOURCE_REQUIRES_REWRITE + " parameter."); + } paramValue = getRawParameterValueFor(params, WSRPRewritingConstants.RESOURCE_URL); if (paramValue != null) @@ -120,7 +125,7 @@ if (requiresRewrite) { - // fixme: do something... + // todo: do something... log.debug("Required re-writing but this is not yet implemented..."); } @@ -142,20 +147,22 @@ } catch (MalformedURLException e) { - IllegalArgumentException iae = new IllegalArgumentException("Malformed URL: " + paramValue); - iae.initCause(e); - throw iae; + throw new IllegalArgumentException("Malformed URL: " + paramValue, e); } } + else + { + throw new IllegalArgumentException("The parsed parameters don't contain a value for the required " + + WSRPRewritingConstants.RESOURCE_URL + " parameter."); + } } private void isSupported(MediaType mediaType) { if (mediaType != null) { - for (int i = 0; i < SUPPORTED_RESOURCE_TYPES.size(); i++) + for (MediaType type : SUPPORTED_RESOURCE_TYPES) { - MediaType type = (MediaType)SUPPORTED_RESOURCE_TYPES.get(i); if (!type.isAllowedSubType(mediaType)) { log.debug("MIME type '" + mediaType Modified: branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RenderHandler.java =================================================================== --- branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RenderHandler.java 2008-04-04 21:38:18 UTC (rev 10505) +++ branches/JBoss_Portal_Branch_2_6/wsrp/src/main/org/jboss/portal/wsrp/consumer/RenderHandler.java 2008-04-04 23:47:59 UTC (rev 10506) @@ -260,6 +260,7 @@ } } + // todo: public for tests public static class ResourceURLRewriter extends URLTools.URLReplacementGenerator { public String getReplacementFor(int currentIndex, URLTools.URLMatch currentMatch)