Merge pull request #968 from apache/fix/WW-5310-fragment

WW-5310 Fixes broken support for Fragments in <s:url/> tag
apache · Jun 21, 2024 · a895450 · a895450
2 parents 444e4d4 + 13916c8
commit a895450
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 7 deletions.
diff --git a/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java b/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java
@@ -103,9 +103,9 @@ public void renderUrl(Writer writer, UrlProvider urlComponent) {
             }
             result = urlHelper.buildUrl(_value, urlComponent.getHttpServletRequest(), urlComponent.getHttpServletResponse(), urlComponent.getParameters(), scheme, urlComponent.isIncludeContext(), urlComponent.isEncode(), urlComponent.isForceAddSchemeHostAndPort(), urlComponent.isEscapeAmp());
         }
-        String anchor = urlComponent.getAnchor();
-        if (StringUtils.isNotEmpty(anchor)) {
-            result += '#' + urlComponent.findString(anchor);
+        if (StringUtils.isNotEmpty(urlComponent.getAnchor())) {
+            String anchor = urlComponent.findString(urlComponent.getAnchor());
+            result += '#' + anchor;
         }
 
         if (urlComponent.isPutInContext()) {
@@ -292,7 +292,7 @@ private void includeExtraParameters(UrlProvider urlComponent) {
     private void includeGetParameters(UrlProvider urlComponent) {
         String query = extractQueryString(urlComponent);
         QueryStringParser.Result result = queryStringParser.parse(query);
-        mergeRequestParameters(urlComponent.getValue(), urlComponent.getParameters(), result.getQueryParams());
+        result = mergeRequestParameters(urlComponent.getValue(), urlComponent.getParameters(), result.getQueryParams());
         if (!result.getQueryFragment().isEmpty()) {
             urlComponent.setAnchor(result.getQueryFragment());
         }
@@ -331,10 +331,11 @@ private String extractQueryString(UrlProvider urlComponent) {
      * @param value             the value attribute (URL to be generated by this component)
      * @param parameters        component parameters
      * @param contextParameters request parameters
+     * @return {@link QueryStringParser.Result} of value's ?query-string or empty()
      */
-    protected void mergeRequestParameters(String value, Map<String, Object> parameters, Map<String, ?> contextParameters) {
-
+    protected QueryStringParser.Result mergeRequestParameters(String value, Map<String, Object> parameters, Map<String, ?> contextParameters) {
         Map<String, Object> mergedParams = new LinkedHashMap<>(contextParameters);
+        QueryStringParser.Result result = queryStringParser.empty();
 
         // Merge contextParameters (from current request) with parameters specified in value attribute
         // eg. value="someAction.action?id=someId&venue=someVenue"
@@ -343,7 +344,8 @@ protected void mergeRequestParameters(String value, Map<String, Object> paramete
         if (StringUtils.contains(value, "?")) {
             String queryString = value.substring(value.indexOf('?') + 1);
 
-            mergedParams = new LinkedHashMap<>(queryStringParser.parse(queryString).getQueryParams());
+            result = queryStringParser.parse(queryString);
+            mergedParams = new LinkedHashMap<>(result.getQueryParams());
             for (Map.Entry<String, ?> entry : contextParameters.entrySet()) {
                 if (!mergedParams.containsKey(entry.getKey())) {
                     mergedParams.put(entry.getKey(), entry.getValue());
@@ -362,6 +364,8 @@ protected void mergeRequestParameters(String value, Map<String, Object> paramete
                 parameters.put(entry.getKey(), entry.getValue());
             }
         }
+
+        return result;
     }
 
 }
diff --git a/core/src/test/java/org/apache/struts2/url/StrutsQueryStringParserTest.java b/core/src/test/java/org/apache/struts2/url/StrutsQueryStringParserTest.java
@@ -112,6 +112,14 @@ public void shouldHandleParamAndFragment() {
         assertEquals("test", queryParameters.getQueryFragment());
     }
 
+    @Test
+    public void shouldHandleOnlyFragment() {
+        QueryStringParser.Result queryParameters = parser.parse("#test");
+
+        assertTrue(queryParameters.getQueryParams().isEmpty());
+        assertEquals("test", queryParameters.getQueryFragment());
+    }
+
     @Before
     public void setUp() throws Exception {
         this.parser = new StrutsQueryStringParser(new StrutsUrlDecoder());

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
@@ -2092,6 +2092,42 @@ public String getString() {
             strutsBodyTagsAreReflectionEqual(tag, freshTag));
     }
 
+    public void testQueryParamsAndFragment() throws Exception {
+        request.setRequestURI("/public/about");
+        tag.setAction("company");
+        tag.setValue("/books?hl=en&lr=Y&redir_esc=y#v=twopage&q&f=false");
+        tag.setEscapeAmp("false");
+
+        tag.doStartTag();
+        tag.doEndTag();
+
+        assertEquals("/books?hl=en&lr=Y&redir_esc=y#v=twopage&q&f=false", writer.toString());
+    }
+
+    public void testDoubleEqualSigns() throws Exception {
+        request.setRequestURI("/public/about");
+        tag.setAction("company");
+        tag.setValue("/PublicationsDetail.aspx?ID=GjTu91suYQI=&t=1");
+        tag.setEscapeAmp("false");
+
+        tag.doStartTag();
+        tag.doEndTag();
+
+        assertEquals("/PublicationsDetail.aspx?ID=GjTu91suYQI%3D&t=1", writer.toString());
+    }
+
+    public void testOnlyFragment() throws Exception {
+        request.setRequestURI("/public/about");
+        tag.setAction("company");
+        tag.setValue("/books#v=twopage&q&f=false");
+        tag.setEscapeAmp("false");
+
+        tag.doStartTag();
+        tag.doEndTag();
+
+        assertEquals("/books#v=twopage&q&f=false", writer.toString());
+    }
+
     @Override
     protected void setUp() throws Exception {
         super.setUp();