WW-4843 Fixes DefaultUrlHelper().buildUrl() to always output port

nickavv · Sep 6, 2017 · 223e735 · 223e735
2 parents 64fa814 + 0b9e68e
commit 223e735
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 15 deletions.
diff --git a/core/src/main/java/org/apache/struts2/views/util/DefaultUrlHelper.java b/core/src/main/java/org/apache/struts2/views/util/DefaultUrlHelper.java
@@ -110,31 +110,21 @@ public String buildUrl(String action, HttpServletRequest request, HttpServletRes
             if (scheme != null) {
                 // If switching schemes, use the configured port for the particular scheme.
                 if (!scheme.equals(reqScheme)) {
-                    if ((HTTP_PROTOCOL.equals(scheme) && (httpPort != DEFAULT_HTTP_PORT)) || (HTTPS_PROTOCOL.equals(scheme) && httpsPort != DEFAULT_HTTPS_PORT)) {
-                        link.append(":");
-                        link.append(HTTP_PROTOCOL.equals(scheme) ? httpPort : httpsPort);
-                    }
+                    appendPort(link, scheme, HTTP_PROTOCOL.equals(scheme) ? httpPort : httpsPort);
                 // Else use the port from the current request.
                 } else {
-                    int reqPort = request.getServerPort();
-
-                    if ((scheme.equals(HTTP_PROTOCOL) && (reqPort != DEFAULT_HTTP_PORT)) || (scheme.equals(HTTPS_PROTOCOL) && reqPort != DEFAULT_HTTPS_PORT)) {
-                        link.append(":");
-                        link.append(reqPort);
-                    }
+                    appendPort(link, scheme, request.getServerPort());
                 }
+            } else {
+                appendPort(link, reqScheme, request.getServerPort());
             }
         } else if ((scheme != null) && !scheme.equals(request.getScheme())) {
             changedScheme = true;
             link.append(scheme);
             link.append("://");
             link.append(request.getServerName());
 
-            if ((scheme.equals(HTTP_PROTOCOL) && (httpPort != DEFAULT_HTTP_PORT)) || (HTTPS_PROTOCOL.equals(scheme) && httpsPort != DEFAULT_HTTPS_PORT))
-            {
-                link.append(":");
-                link.append(HTTP_PROTOCOL.equals(scheme) ? httpPort : httpsPort);
-            }
+            appendPort(link, scheme, HTTP_PROTOCOL.equals(scheme) ? httpPort : httpsPort);
         }
 
         if (action != null) {
@@ -201,6 +191,13 @@ public String buildUrl(String action, HttpServletRequest request, HttpServletRes
         return result;
     }
 
+    private void appendPort(StringBuilder link, String scheme, int port) {
+        if ((HTTP_PROTOCOL.equals(scheme) && port != DEFAULT_HTTP_PORT) || (HTTPS_PROTOCOL.equals(scheme) && port != DEFAULT_HTTPS_PORT)) {
+            link.append(":");
+            link.append(port);
+        }
+    }
+
     public void buildParametersString(Map<String, Object> params, StringBuilder link, String paramSeparator) {
         buildParametersString(params, link, paramSeparator, true);
     }

diff --git a/core/src/test/java/org/apache/struts2/views/util/DefaultUrlHelperTest.java b/core/src/test/java/org/apache/struts2/views/util/DefaultUrlHelperTest.java
@@ -138,6 +138,29 @@ public void testForceAddNullSchemeHostAndPort() throws Exception {
         mockHttpServletRequest.expectAndReturn("getServerName", "localhost");
         mockHttpServletRequest.expectAndReturn("getContextPath",
             "/contextPath");
+        mockHttpServletRequest.expectAndReturn("getServerPort", 80);
+
+        Mock mockHttpServletResponse = new Mock(HttpServletResponse.class);
+        mockHttpServletResponse.expectAndReturn("encodeURL", expectedUrl,
+            expectedUrl);
+
+        String result = urlHelper.buildUrl("/path1/path2/myAction.action",
+                (HttpServletRequest) mockHttpServletRequest.proxy(),
+                (HttpServletResponse) mockHttpServletResponse.proxy(), null,
+                null, true, true, true);
+        assertEquals(expectedUrl, result);
+        mockHttpServletRequest.verify();
+    }
+
+    public void testForceAddNullSchemeHostAndPort2() throws Exception {
+        String expectedUrl = "http://localhost:8080/contextPath/path1/path2/myAction.action";
+
+        Mock mockHttpServletRequest = new Mock(HttpServletRequest.class);
+        mockHttpServletRequest.expectAndReturn("getScheme", "http");
+        mockHttpServletRequest.expectAndReturn("getServerName", "localhost");
+        mockHttpServletRequest.expectAndReturn("getContextPath",
+            "/contextPath");
+        mockHttpServletRequest.expectAndReturn("getServerPort", 8080);
 
         Mock mockHttpServletResponse = new Mock(HttpServletResponse.class);
         mockHttpServletResponse.expectAndReturn("encodeURL", expectedUrl,