From 75584a2c33b3c4ed2f923d2bc68256856e4a084f Mon Sep 17 00:00:00 2001 From: Scott Battaglia Date: Sun, 9 Mar 2014 23:13:04 -0400 Subject: [PATCH 1/2] CASC-214 Improve Service Url Construction to Add Non-Standard Ports if Missing from Configuration Problem: sometimes the port is missing from the configuration. This generates the wrong service url. Solution: Add the server port if the server configuration does not have one. QA Notes: Added unit tests to confirm behavior (and old unit tests still pass) --- .../jasig/cas/client/util/CommonUtils.java | 28 +++++++++++++++++++ .../cas/client/util/CommonUtilsTests.java | 21 ++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/util/CommonUtils.java b/cas-client-core/src/main/java/org/jasig/cas/client/util/CommonUtils.java index c767c56..2b0d380 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/util/CommonUtils.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/util/CommonUtils.java @@ -225,6 +225,21 @@ public final class CommonUtils { return serverNames[0]; } + private static boolean serverNameContainsPort(final boolean containsScheme, final String serverName) { + if (!containsScheme && serverName.contains(":")) { + return true; + } + + final int schemeIndex = serverName.indexOf(":"); + final int portIndex = serverName.lastIndexOf(":"); + return schemeIndex != portIndex; + } + + private static boolean requestIsOnStandardPort(final HttpServletRequest request) { + final int serverPort = request.getServerPort(); + return serverPort == 80 || serverPort == 443; + } + /** * Constructs a service url from the HttpServletRequest or from the given * serviceUrl. Prefers the serviceUrl provided if both a serviceUrl and a @@ -250,11 +265,24 @@ public final class CommonUtils { final String serverName = findMatchingServerName(request, serverNames); + boolean containsScheme = true; if (!serverName.startsWith("https://") && !serverName.startsWith("http://")) { buffer.append(request.isSecure() ? "https://" : "http://"); + containsScheme = false; } buffer.append(serverName); + + final boolean serverNameContainsPort = serverNameContainsPort(containsScheme, serverName); + System.out.println("serverNameContainsPort " + serverNameContainsPort); + final boolean requestIsOnStandardPort = requestIsOnStandardPort(request); + System.out.println("requestIsOnStandardPort " + requestIsOnStandardPort); + + if (!serverNameContainsPort(containsScheme, serverName) && !requestIsOnStandardPort(request)) { + buffer.append(":"); + buffer.append(request.getServerPort()); + } + buffer.append(request.getRequestURI()); if (CommonUtils.isNotBlank(request.getQueryString())) { diff --git a/cas-client-core/src/test/java/org/jasig/cas/client/util/CommonUtilsTests.java b/cas-client-core/src/test/java/org/jasig/cas/client/util/CommonUtilsTests.java index f6e686c..43967ab 100644 --- a/cas-client-core/src/test/java/org/jasig/cas/client/util/CommonUtilsTests.java +++ b/cas-client-core/src/test/java/org/jasig/cas/client/util/CommonUtilsTests.java @@ -136,6 +136,27 @@ public final class CommonUtilsTests extends TestCase { assertEquals(CONST_MY_URL, constructedUrl); } + private void constructUrlNonStandardPortAndNoPortInConfigTest(final String serverNameList) { + final String CONST_MY_URL = "https://www.myserver.com:555/hello/hithere/"; + final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hello/hithere/"); + request.addHeader("Host", "www.myserver.com"); + request.setScheme("https"); + request.setSecure(true); + request.setServerPort(555); + final MockHttpServletResponse response = new MockHttpServletResponse(); + final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, + serverNameList, "ticket", false); + assertEquals(CONST_MY_URL, constructedUrl); + } + + public void testConstructUrlNonStandardPortAndNoScheme() { + constructUrlNonStandardPortAndNoPortInConfigTest("www.myserver.com"); + } + + public void testConstructUrlNonStandardPortAndScheme() { + constructUrlNonStandardPortAndNoPortInConfigTest("https://www.myserver.com"); + } + public void testConstructUrlWithMultipleHostsNoPortsOrProtocol() { final String CONST_MY_URL = "https://www.myserver.com/hello/hithere/"; final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hello/hithere/"); From a4df6582eea1b1873a2f0b9c344e1e961abfe6ad Mon Sep 17 00:00:00 2001 From: Scott Battaglia Date: Sun, 9 Mar 2014 23:18:14 -0400 Subject: [PATCH 2/2] Removed debug statements used to confirm private methods worked. --- .../src/main/java/org/jasig/cas/client/util/CommonUtils.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/util/CommonUtils.java b/cas-client-core/src/main/java/org/jasig/cas/client/util/CommonUtils.java index 2b0d380..8fb90c7 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/util/CommonUtils.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/util/CommonUtils.java @@ -273,11 +273,6 @@ public final class CommonUtils { buffer.append(serverName); - final boolean serverNameContainsPort = serverNameContainsPort(containsScheme, serverName); - System.out.println("serverNameContainsPort " + serverNameContainsPort); - final boolean requestIsOnStandardPort = requestIsOnStandardPort(request); - System.out.println("requestIsOnStandardPort " + requestIsOnStandardPort); - if (!serverNameContainsPort(containsScheme, serverName) && !requestIsOnStandardPort(request)) { buffer.append(":"); buffer.append(request.getServerPort());