diff --git a/core/src/main/java/org/apache/struts2/components/URL.java b/core/src/main/java/org/apache/struts2/components/URL.java index 76a52392b..2454bbc69 100644 --- a/core/src/main/java/org/apache/struts2/components/URL.java +++ b/core/src/main/java/org/apache/struts2/components/URL.java @@ -33,7 +33,9 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpUtils; import java.io.IOException; import java.io.Writer; +import java.util.Collections; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.Map; /** @@ -157,9 +159,10 @@ public class URL extends Component { } if (NONE.equalsIgnoreCase(includeParams)) { + mergeRequestParameters(value, parameters, Collections.EMPTY_MAP); ActionContext.getContext().put(XWorkContinuationConfig.CONTINUE_KEY, null); } else if (ALL.equalsIgnoreCase(includeParams)) { - mergeRequestParameters(parameters, req.getParameterMap()); + mergeRequestParameters(value, parameters, req.getParameterMap()); // for ALL also include GET parameters includeGetParameters(); @@ -179,10 +182,7 @@ public class URL extends Component { private void includeGetParameters() { if(!(Dispatcher.getInstance().isPortletSupportActive() && PortletActionContext.isPortletRequest())) { String query = extractQueryString(); - if (query != null) { - //mergeRequestParameters(parameters, HttpUtils.parseQueryString(query)); - mergeRequestParameters(parameters, UrlHelper.parseQueryString(query)); - } + mergeRequestParameters(value, parameters, UrlHelper.parseQueryString(query)); } } @@ -221,7 +221,14 @@ public class URL extends Component { result = PortletUrlHelper.buildResourceUrl(value, parameters); } else { - result = UrlHelper.buildUrl(value, req, res, parameters, scheme, includeContext, encode); + String _value = value; + + // We don't include the request parameters cause they would have been + // prioritised before this [in start(Writer) method] + if (_value != null && _value.indexOf("?") > 0) { + _value = _value.substring(0, _value.indexOf("?")); + } + result = UrlHelper.buildUrl(_value, req, res, parameters, scheme, includeContext, encode); } } if ( anchor != null && anchor.length() > 0 ) { @@ -345,13 +352,52 @@ public class URL extends Component { /** * Merge request parameters into current parameters. If a parameter is - * already present, than the request parameter will not override its value. + * already present, than the request parameter in the current request and value atrribute + * will not override its value. * + * The priority is as follows:- + * + * + * @param value the value attribute (url to be generated by this component) * @param parameters component parameters * @param contextParameters request parameters */ - protected void mergeRequestParameters(Map parameters, Map contextParameters){ - for (Iterator iterator = contextParameters.entrySet().iterator(); iterator.hasNext();) { + protected void mergeRequestParameters(String value, Map parameters, Map contextParameters){ + + Map mergedParams = new LinkedHashMap(contextParameters); + + // Merge contextParameters (from current request) with parameters specified in value attribute + // eg. value="someAction.action?id=someId&venue=someVenue" + // where the parameters specified in value attribute takes priority. + + if (value != null && value.trim().length() > 0 && value.indexOf("?") > 0) { + mergedParams = new LinkedHashMap(); + + String queryString = value.substring(value.indexOf("?")+1); + + mergedParams = UrlHelper.parseQueryString(queryString); + for (Iterator iterator = contextParameters.entrySet().iterator(); iterator.hasNext();) { + Map.Entry entry = (Map.Entry) iterator.next(); + Object key = entry.getKey(); + + if (!mergedParams.containsKey(key)) { + mergedParams.put(key, entry.getValue()); + } + } + } + + + // Merge parameters specified in value attribute + // eg. value="someAction.action?id=someId&venue=someVenue" + // with parameters specified though param tag + // eg. + // where parameters specified through param tag takes priority. + + for (Iterator iterator = mergedParams.entrySet().iterator(); iterator.hasNext();) { Map.Entry entry = (Map.Entry) iterator.next(); Object key = entry.getKey(); diff --git a/core/src/main/java/org/apache/struts2/views/util/UrlHelper.java b/core/src/main/java/org/apache/struts2/views/util/UrlHelper.java index 1b710e56d..14a40345c 100644 --- a/core/src/main/java/org/apache/struts2/views/util/UrlHelper.java +++ b/core/src/main/java/org/apache/struts2/views/util/UrlHelper.java @@ -267,18 +267,20 @@ public class UrlHelper { if (queryString != null) { String[] params = queryString.split("&"); for (int a=0; a< params.length; a++) { - String[] tmpParams = params[a].split("="); - String paramName = null; - String paramValue = ""; - if (tmpParams.length > 0) { - paramName = tmpParams[0]; - } - if (tmpParams.length > 1) { - paramValue = tmpParams[1]; - } - if (paramName != null) { - String translatedParamValue = translateAndDecode(paramValue); - queryParams.put(paramName, translatedParamValue); + if (params[a].trim().length() > 0) { + String[] tmpParams = params[a].split("="); + String paramName = null; + String paramValue = ""; + if (tmpParams.length > 0) { + paramName = tmpParams[0]; + } + if (tmpParams.length > 1) { + paramValue = tmpParams[1]; + } + if (paramName != null) { + String translatedParamValue = translateAndDecode(paramValue); + queryParams.put(paramName, translatedParamValue); + } } } } diff --git a/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java index 66296b330..25451f496 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java @@ -18,6 +18,7 @@ package org.apache.struts2.views.jsp; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; import javax.servlet.jsp.JspWriter; @@ -35,6 +36,151 @@ import com.opensymphony.xwork2.util.OgnlValueStack; public class URLTagTest extends AbstractUITagTest { private URLTag tag; + + + /** + * To test priority of parameter passed in to url component though + * various way + * - current request url + * - tag's value attribute + * - tag's nested param tag + * + * id1 + * === + * - found in current request url + * - found in tag's value attribute + * - found in tag's param tag + * CONCLUSION: tag's param tag takes precedence (paramId1) + * + * id2 + * === + * - found in current request url + * - found in tag's value attribute + * CONCLUSION: tag's value attribute take precedence (tagId2) + * + * urlParam1 + * ========= + * - found in current request url + * CONCLUSION: param in current request url will be used (urlValue1) + * + * urlParam2 + * ========= + * - found in current request url + * CONCLUSION: param in current request url will be used. (urlValue2) + * + * tagId + * ===== + * - found in tag's value attribute + * CONCLUSION: param in tag's value attribute wil; be used. (tagValue) + * + * param1 + * ====== + * - found in nested param tag + * CONCLUSION: param in nested param tag will be used. (param1value) + * + * param2 + * ====== + * - found in nested param tag + * CONCLUSION: param in nested param tag will be used. (param2value) + */ + public void testParametersPriority() throws Exception { + request.setQueryString("id1=urlId1&id2=urlId2&urlParam1=urlValue1&urlParam2=urlValue2"); + + tag.setValue("testAction.action?id1=tagId1&id2=tagId2&tagId=tagValue"); + + ParamTag param1 = new ParamTag(); + param1.setPageContext(pageContext); + param1.setName("param1"); + param1.setValue("%{'param1value'}"); + + ParamTag param2 = new ParamTag(); + param2.setPageContext(pageContext); + param2.setName("param2"); + param2.setValue("%{'param2value'}"); + + ParamTag param3 = new ParamTag(); + param3.setPageContext(pageContext); + param3.setName("id1"); + param3.setValue("%{'paramId1'}"); + + + tag.doStartTag(); + param1.doStartTag(); + param1.doEndTag(); + param2.doStartTag(); + param2.doEndTag(); + param3.doStartTag(); + param3.doEndTag(); + + URL url = (URL) tag.getComponent(); + Map parameters = url.getParameters(); + + + assertNotNull(parameters); + assertEquals(parameters.size(), 7); + assertEquals(parameters.get("id1"), "paramId1"); + assertEquals(parameters.get("id2"), "tagId2"); + assertEquals(parameters.get("urlParam1"), "urlValue1"); + assertEquals(parameters.get("urlParam2"), "urlValue2"); + assertEquals(parameters.get("tagId"), "tagValue"); + assertEquals(parameters.get("param1"), "param1value"); + assertEquals(parameters.get("param2"), "param2value"); + } + + + /** + * To test priority of parameter passed in to url component though + * various way, with includeParams="NONE" + * - current request url + * - tag's value attribute + * - tag's nested param tag + * + * In this case only parameters from the tag itself is taken into account. + * Those from request will not count, only those in tag's value attribute + * and nested param tag. + * + * @throws Exception + */ + public void testParametersPriorityWithIncludeParamsAsNONE() throws Exception { + request.setQueryString("id1=urlId1&id2=urlId2&urlParam1=urlValue1&urlParam2=urlValue2"); + + tag.setValue("testAction.action?id1=tagId1&id2=tagId2&tagId=tagValue"); + tag.setIncludeParams("NONE"); + + ParamTag param1 = new ParamTag(); + param1.setPageContext(pageContext); + param1.setName("param1"); + param1.setValue("%{'param1value'}"); + + ParamTag param2 = new ParamTag(); + param2.setPageContext(pageContext); + param2.setName("param2"); + param2.setValue("%{'param2value'}"); + + ParamTag param3 = new ParamTag(); + param3.setPageContext(pageContext); + param3.setName("id1"); + param3.setValue("%{'paramId1'}"); + + + tag.doStartTag(); + param1.doStartTag(); + param1.doEndTag(); + param2.doStartTag(); + param2.doEndTag(); + param3.doStartTag(); + param3.doEndTag(); + + URL url = (URL) tag.getComponent(); + Map parameters = url.getParameters(); + + assertEquals(parameters.size(), 5); + assertEquals(parameters.get("id1"), "paramId1"); + assertEquals(parameters.get("id2"), "tagId2"); + assertEquals(parameters.get("tagId"), "tagValue"); + assertEquals(parameters.get("param1"), "param1value"); + assertEquals(parameters.get("param2"), "param2value"); + } public void testIncludeParamsDefaultToGET() throws Exception { request.setQueryString("one=oneVal&two=twoVal&three=threeVal"); diff --git a/core/src/test/java/org/apache/struts2/views/util/UrlHelperTest.java b/core/src/test/java/org/apache/struts2/views/util/UrlHelperTest.java index 261cb0ebf..798ca6fbf 100644 --- a/core/src/test/java/org/apache/struts2/views/util/UrlHelperTest.java +++ b/core/src/test/java/org/apache/struts2/views/util/UrlHelperTest.java @@ -25,6 +25,7 @@ import org.apache.struts2.config.Settings; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; import java.util.TreeMap; @@ -281,6 +282,21 @@ public class UrlHelperTest extends StrutsTestCase { assertEquals(result.get("ccc"), ""); } + public void testParseEmptyQuery() throws Exception { + Map result = UrlHelper.parseQueryString(""); + + assertNotNull(result); + assertEquals(result.size(), 0); + } + + public void testParseNullQuery() throws Exception { + Map result = UrlHelper.parseQueryString(null); + + assertNotNull(result); + assertEquals(result.size(), 0); + } + + public void testTranslateAndEncode() throws Exception { String defaultI18nEncoding = Settings.get(StrutsConstants.STRUTS_I18N_ENCODING); try {