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 a95fb9a..639b3b4 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 @@ -26,6 +26,8 @@ import java.net.URLEncoder; import java.util.*; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; + +import org.jasig.cas.client.Protocol; import org.jasig.cas.client.proxy.ProxyGrantingTicketStorage; import org.jasig.cas.client.ssl.HttpURLConnectionFactory; import org.jasig.cas.client.ssl.HttpsURLConnectionFactory; @@ -57,10 +59,21 @@ public final class CommonUtils { private static final HttpURLConnectionFactory DEFAULT_URL_CONNECTION_FACTORY = new HttpsURLConnectionFactory(); + private static final String SERVICE_PARAMETER_NAMES; + private CommonUtils() { // nothing to do } + static { + final Set serviceParameterSet = new HashSet(4); + for (final Protocol protocol : Protocol.values()) { + serviceParameterSet.add(protocol.getServiceParameterName()); + } + SERVICE_PARAMETER_NAMES = serviceParameterSet.toString() + .replaceAll("\\[|\\]", "") + .replaceAll("\\s", ""); + } /** * Check whether the object is null or not. If it is, throw an exception and * display the message. @@ -259,6 +272,30 @@ public final class CommonUtils { 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 + * serviceName. Compiles a list of all service parameters for supported protocols + * and removes them all from the query string. + * + * @param request the HttpServletRequest + * @param response the HttpServletResponse + * @param service the configured service url (this will be used if not null) + * @param serverNames the server name to use to construct the service url if the service param is empty. Note, prior to CAS Client 3.3, this was a single value. + * As of 3.3, it can be a space-separated value. We keep it as a single value, but will convert it to an array internally to get the matching value. This keeps backward compatability with anything using this public + * method. + * @param artifactParameterName the artifact parameter name to remove (i.e. ticket) + * @param encode whether to encode the url or not (i.e. Jsession). + * @return the service url to use. + */ + @Deprecated + public static String constructServiceUrl(final HttpServletRequest request, final HttpServletResponse response, + final String service, final String serverNames, + final String artifactParameterName, final boolean encode) { + return constructServiceUrl(request, response, service, serverNames, SERVICE_PARAMETER_NAMES + , artifactParameterName, encode); + } + /** * Constructs a service url from the HttpServletRequest or from the given * serviceUrl. Prefers the serviceUrl provided if both a serviceUrl and a @@ -267,7 +304,7 @@ public final class CommonUtils { * @param request the HttpServletRequest * @param response the HttpServletResponse * @param service the configured service url (this will be used if not null) - * @param serverNames the server name to use to constuct the service url if the service param is empty. Note, prior to CAS Client 3.3, this was a single value. + * @param serverNames the server name to use to construct the service url if the service param is empty. Note, prior to CAS Client 3.3, this was a single value. * As of 3.3, it can be a space-separated value. We keep it as a single value, but will convert it to an array internally to get the matching value. This keeps backward compatability with anything using this public * method. * @param serviceParameterName the service parameter name to remove (i.e. service) @@ -305,9 +342,12 @@ public final class CommonUtils { builder.setEncodedPath(request.getRequestURI()); - for (final URIBuilder.BasicNameValuePair pair : originalRequestUrl.getQueryParams()) { - if (!pair.getName().equals(artifactParameterName) && !pair.getName().equals(serviceParameterName)) { - builder.addParameter(pair.getName(), pair.getValue()); + final List serviceParameterNames = Arrays.asList(serviceParameterName.split(",")); + if (!serviceParameterNames.isEmpty() && !originalRequestUrl.getQueryParams().isEmpty()) { + for (final URIBuilder.BasicNameValuePair pair : originalRequestUrl.getQueryParams()) { + if (!pair.getName().equals(artifactParameterName) && !serviceParameterNames.contains(pair.getName())) { + builder.addParameter(pair.getName(), pair.getValue()); + } } } 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 8ec0541..b2a9d13 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 @@ -195,6 +195,20 @@ public final class CommonUtilsTests extends TestCase { assertEquals("https://www.myserver.com/hello/hithere/?custom=custom", constructedUrl); } + public void testConstructServiceUrlWithNoServiceParametersPassed() { + final String CONST_MY_URL = "https://www.myserver.com/hello/hithere/"; + final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hello/hithere/"); + request.setScheme("https"); + request.setSecure(true); + request.setQueryString("TARGET=Test1&service=Test2&custom=custom"); + + final MockHttpServletResponse response = new MockHttpServletResponse(); + final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", + Protocol.SAML11.getArtifactParameterName() , true); + + assertEquals("https://www.myserver.com/hello/hithere/?custom=custom", constructedUrl); + } + public void testConstructServiceUrlWithEncodedParams2Saml() { final String CONST_MY_URL = "https://www.myserver.com/hello/hithere/"; final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/hello/hithere/");