From be2a64147320aacc6262b5fd181bea46e02618cd Mon Sep 17 00:00:00 2001 From: Misagh Moayyed Date: Wed, 13 May 2015 03:44:13 -0700 Subject: [PATCH 1/2] added uri builder to construct query params --- .../cas/client/util/AbstractCasFilter.java | 1 + .../jasig/cas/client/util/CommonUtils.java | 78 +-- .../org/jasig/cas/client/util/URIBuilder.java | 604 ++++++++++++++++++ .../cas/client/util/CommonUtilsTests.java | 94 ++- .../client/tomcat/AuthenticatorDelegate.java | 2 +- 5 files changed, 730 insertions(+), 49 deletions(-) create mode 100644 cas-client-core/src/main/java/org/jasig/cas/client/util/URIBuilder.java diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractCasFilter.java b/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractCasFilter.java index 6e0df5f..9b48296 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractCasFilter.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractCasFilter.java @@ -102,6 +102,7 @@ public abstract class AbstractCasFilter extends AbstractConfigurationFilter { protected final String constructServiceUrl(final HttpServletRequest request, final HttpServletResponse response) { return CommonUtils.constructServiceUrl(request, response, this.service, this.serverName, + this.protocol.getServiceParameterName(), this.protocol.getArtifactParameterName(), this.encodeServiceUrl); } 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 95eff51..92bec84 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 @@ -20,6 +20,7 @@ package org.jasig.cas.client.util; import java.io.*; import java.net.HttpURLConnection; +import java.net.URI; import java.net.URL; import java.net.URLEncoder; import java.util.*; @@ -259,70 +260,59 @@ public final class CommonUtils { } /** - * Constructs a service url from the HttpServletRequest or from the given - * serviceUrl. Prefers the serviceUrl provided if both a serviceUrl and a - * serviceName. - * - * @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. - * 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. - */ + * Constructs a service url from the HttpServletRequest or from the given + * serviceUrl. Prefers the serviceUrl provided if both a serviceUrl and a + * serviceName. + * + * @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. + * 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) + * @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. + */ public static String constructServiceUrl(final HttpServletRequest request, final HttpServletResponse response, - final String service, final String serverNames, final String artifactParameterName, final boolean encode) { + final String service, final String serverNames, final String serviceParameterName, + final String artifactParameterName, final boolean encode) { if (CommonUtils.isNotBlank(service)) { return encode ? response.encodeURL(service) : service; } - final StringBuilder buffer = new StringBuilder(); - final String serverName = findMatchingServerName(request, serverNames); + final URIBuilder originalRequestUrl = new URIBuilder(request.getRequestURL().toString(), encode); + originalRequestUrl.setParameters(request.getQueryString()); + + URIBuilder builder = null; boolean containsScheme = true; if (!serverName.startsWith("https://") && !serverName.startsWith("http://")) { - buffer.append(request.isSecure() ? "https://" : "http://"); + builder = new URIBuilder(encode); + builder.setScheme(request.isSecure() ? "https" : "http"); + builder.setHost(serverName); containsScheme = false; + } else { + builder = new URIBuilder(serverName, encode); } - buffer.append(serverName); if (!serverNameContainsPort(containsScheme, serverName) && !requestIsOnStandardPort(request)) { - buffer.append(":"); - buffer.append(request.getServerPort()); + builder.setPort(request.getServerPort()); } - buffer.append(request.getRequestURI()); + builder.setEncodedPath(request.getRequestURI()); - if (CommonUtils.isNotBlank(request.getQueryString())) { - final int location = request.getQueryString().indexOf(artifactParameterName + "="); - - if (location == 0) { - final String returnValue = encode ? response.encodeURL(buffer.toString()) : buffer.toString(); - LOGGER.debug("serviceUrl generated: {}", returnValue); - return returnValue; - } - - buffer.append("?"); - - if (location == -1) { - buffer.append(request.getQueryString()); - } else if (location > 0) { - final int actualLocation = request.getQueryString().indexOf("&" + artifactParameterName + "="); - - if (actualLocation == -1) { - buffer.append(request.getQueryString()); - } else if (actualLocation > 0) { - buffer.append(request.getQueryString().substring(0, actualLocation)); - } + for (final URIBuilder.BasicNameValuePair pair : originalRequestUrl.getQueryParams()) { + if (!pair.getName().equals(artifactParameterName) && !pair.getName().equals(serviceParameterName)) { + builder.addParameter(pair.getName(), pair.getValue()); } } - final String returnValue = encode ? response.encodeURL(buffer.toString()) : buffer.toString(); + final String result = builder.toString(); + final String returnValue = encode ? response.encodeURL(result) : result; LOGGER.debug("serviceUrl generated: {}", returnValue); return returnValue; } diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/util/URIBuilder.java b/cas-client-core/src/main/java/org/jasig/cas/client/util/URIBuilder.java new file mode 100644 index 0000000..5cad257 --- /dev/null +++ b/cas-client-core/src/main/java/org/jasig/cas/client/util/URIBuilder.java @@ -0,0 +1,604 @@ +/* + * Licensed to Jasig under one or more contributor license + * agreements. See the NOTICE file distributed with this work + * for additional information regarding copyright ownership. + * Jasig licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a + * copy of the License at the following location: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.jasig.cas.client.util; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.Serializable; +import java.io.UnsupportedEncodingException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URLDecoder; +import java.nio.charset.Charset; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.regex.Pattern; + +/** + * A utility class borrowed from apache http-client to build uris. + * @author Misagh Moayyed + * @since 3.4 + */ +public final class URIBuilder { + private static final Logger LOGGER = LoggerFactory.getLogger(URIBuilder.class); + private static final Pattern IPV6_STD_PATTERN = Pattern.compile("^[0-9a-fA-F]{1,4}(:[0-9a-fA-F]{1,4}){7}$"); + + private String scheme; + private String encodedSchemeSpecificPart; + private String encodedAuthority; + private String userInfo; + private String encodedUserInfo; + private String host; + private int port; + private String path; + private String encodedPath; + private String encodedQuery; + private List queryParams; + private String query; + private boolean encode; + private String fragment; + private String encodedFragment; + + /** + * Constructs an empty instance. + */ + public URIBuilder() { + super(); + this.port = -1; + } + + public URIBuilder(final boolean encode) { + this(); + setEncode(encode); + } + + /** + * Construct an instance from the string which must be a valid URI. + * + * @param string a valid URI in string form + * @throws URISyntaxException if the input is not a valid URI + */ + public URIBuilder(final String string) { + super(); + try { + digestURI(new URI(string)); + } catch (final URISyntaxException e) { + throw new RuntimeException(e.getMessage(), e); + } + } + + public URIBuilder(final String string, boolean encode) { + super(); + try { + setEncode(encode); + digestURI(new URI(string)); + } catch (final URISyntaxException e) { + throw new RuntimeException(e.getMessage(), e); + } + } + + + /** + * Construct an instance from the provided URI. + * @param uri + */ + public URIBuilder(final URI uri) { + super(); + digestURI(uri); + } + + private List parseQuery(final String query) { + + try { + final Charset utf8 = Charset.forName("UTF-8"); + if (query != null && !query.isEmpty()) { + final List list = new ArrayList(); + final String queryValue = URLDecoder.decode(query, utf8.name()); + final String[] parametersArray = queryValue.split("&"); + + for (final String parameter : parametersArray) { + final String[] parameterCombo = parameter.split("="); + if (parameterCombo.length == 2) { + list.add(new BasicNameValuePair(parameterCombo[0], parameterCombo[1])); + } + } + return list; + } + } catch (final UnsupportedEncodingException e) { + LOGGER.error(e.getMessage(), e); + } + return Collections.emptyList(); + } + + /** + * Builds a {@link URI} instance. + */ + public URI build() { + try { + return new URI(buildString()); + } catch (final URISyntaxException e) { + throw new RuntimeException(e); + } + } + + private static boolean isIPv6Address(final String input) { + return IPV6_STD_PATTERN.matcher(input).matches(); + } + + private String buildString() { + final StringBuilder sb = new StringBuilder(); + if (this.scheme != null) { + sb.append(this.scheme).append(':'); + } + if (this.encodedSchemeSpecificPart != null) { + sb.append(this.encodedSchemeSpecificPart); + } else { + if (this.encodedAuthority != null) { + sb.append("//").append(this.encodedAuthority); + } else if (this.host != null) { + sb.append("//"); + if (this.encodedUserInfo != null) { + sb.append(this.encodedUserInfo).append("@"); + } else if (this.userInfo != null) { + sb.append(encodeUserInfo(this.userInfo)).append("@"); + } + if (isIPv6Address(this.host)) { + sb.append("[").append(this.host).append("]"); + } else { + sb.append(this.host); + } + if (this.port >= 0) { + sb.append(":").append(this.port); + } + } + if (this.encodedPath != null) { + sb.append(normalizePath(this.encodedPath)); + } else if (this.path != null) { + sb.append(encodePath(normalizePath(this.path))); + } + if (this.encodedQuery != null) { + sb.append("?").append(this.encodedQuery); + } else if (this.queryParams != null && !this.queryParams.isEmpty()) { + sb.append("?").append(encodeUrlForm(this.queryParams)); + } else if (this.query != null) { + sb.append("?").append(encodeUric(this.query)); + } + } + if (this.encodedFragment != null) { + sb.append("#").append(this.encodedFragment); + } else if (this.fragment != null) { + sb.append("#").append(encodeUric(this.fragment)); + } + return sb.toString(); + } + + public URIBuilder digestURI(final URI uri) { + this.scheme = uri.getScheme(); + this.encodedSchemeSpecificPart = uri.getRawSchemeSpecificPart(); + this.encodedAuthority = uri.getRawAuthority(); + this.host = uri.getHost(); + this.port = uri.getPort(); + this.encodedUserInfo = uri.getRawUserInfo(); + this.userInfo = uri.getUserInfo(); + this.encodedPath = uri.getRawPath(); + this.path = uri.getPath(); + this.encodedQuery = uri.getRawQuery(); + this.queryParams = parseQuery(uri.getRawQuery()); + this.encodedFragment = uri.getRawFragment(); + this.fragment = uri.getFragment(); + return this; + } + + private String encodeUserInfo(final String userInfo) { + return this.encode ? CommonUtils.urlEncode(userInfo) : userInfo; + } + + private String encodePath(final String path) { + return this.encode ? CommonUtils.urlEncode(path) : path; + } + + private String encodeUrlForm(final List params) { + final StringBuilder result = new StringBuilder(); + for (final BasicNameValuePair parameter : params) { + final String encodedName = this.encode ? CommonUtils.urlEncode(parameter.getName()) : parameter.getName(); + final String encodedValue = this.encode ? CommonUtils.urlEncode(parameter.getValue()) : parameter.getValue(); + + if (result.length() > 0) { + result.append("&"); + } + result.append(encodedName); + if (encodedValue != null) { + result.append("="); + result.append(encodedValue); + } + } + return result.toString(); + } + + private String encodeUric(final String fragment) { + return this.encode ? CommonUtils.urlEncode(fragment) : fragment; + } + + public void setEncode(boolean encode) { + this.encode = encode; + } + + /** + * Sets URI scheme. + */ + public URIBuilder setScheme(final String scheme) { + this.scheme = scheme; + return this; + } + + /** + * Sets URI user info. The value is expected to be unescaped and may contain non ASCII + * characters. + */ + public URIBuilder setUserInfo(final String userInfo) { + this.userInfo = userInfo; + this.encodedSchemeSpecificPart = null; + this.encodedAuthority = null; + this.encodedUserInfo = null; + return this; + } + + /** + * Sets URI user info as a combination of username and password. These values are expected to + * be unescaped and may contain non ASCII characters. + */ + public URIBuilder setUserInfo(final String username, final String password) { + return setUserInfo(username + ':' + password); + } + + /** + * Sets URI host. + */ + public URIBuilder setHost(final String host) { + this.host = host; + this.encodedSchemeSpecificPart = null; + this.encodedAuthority = null; + return this; + } + + /** + * Sets URI port. + */ + public URIBuilder setPort(final int port) { + this.port = port < 0 ? -1 : port; + this.encodedSchemeSpecificPart = null; + this.encodedAuthority = null; + return this; + } + + /** + * Sets URI path. The value is expected to be unescaped and may contain non ASCII characters. + */ + public URIBuilder setPath(final String path) { + this.path = path; + this.encodedSchemeSpecificPart = null; + this.encodedPath = null; + return this; + } + + public URIBuilder setEncodedPath(final String path) { + this.encodedPath = path; + this.encodedSchemeSpecificPart = null; + return this; + } + + /** + * Removes URI query. + */ + public URIBuilder removeQuery() { + this.queryParams = null; + this.query = null; + this.encodedQuery = null; + this.encodedSchemeSpecificPart = null; + return this; + } + + /** + * Sets URI query parameters. The parameter name / values are expected to be unescaped + * and may contain non ASCII characters. + *

+ * Please note query parameters and custom query component are mutually exclusive. This method + * will remove custom query if present. + *

+ */ + public URIBuilder setParameters(final List nvps) { + this.queryParams = new ArrayList(); + this.queryParams.addAll(nvps); + this.encodedQuery = null; + this.encodedSchemeSpecificPart = null; + this.query = null; + return this; + } + + public URIBuilder setParameters(final String queryParameters) { + this.queryParams = new ArrayList(); + this.queryParams.addAll(parseQuery(queryParameters)); + this.encodedQuery = null; + this.encodedSchemeSpecificPart = null; + this.query = null; + return this; + } + + + + /** + * Adds URI query parameters. The parameter name / values are expected to be unescaped + * and may contain non ASCII characters. + *

+ * Please note query parameters and custom query component are mutually exclusive. This method + * will remove custom query if present. + *

+ */ + public URIBuilder addParameters(final List nvps) { + if (this.queryParams == null) { + this.queryParams = new ArrayList(); + } + this.queryParams.addAll(nvps); + this.encodedQuery = null; + this.encodedSchemeSpecificPart = null; + this.query = null; + return this; + } + + /** + * Sets URI query parameters. The parameter name / values are expected to be unescaped + * and may contain non ASCII characters. + *

+ * Please note query parameters and custom query component are mutually exclusive. This method + * will remove custom query if present. + *

+ */ + public URIBuilder setParameters(final BasicNameValuePair... nvps) { + if (this.queryParams == null) { + this.queryParams = new ArrayList(); + } else { + this.queryParams.clear(); + } + for (final BasicNameValuePair nvp: nvps) { + this.queryParams.add(nvp); + } + this.encodedQuery = null; + this.encodedSchemeSpecificPart = null; + this.query = null; + return this; + } + + /** + * Adds parameter to URI query. The parameter name and value are expected to be unescaped + * and may contain non ASCII characters. + *

+ * Please note query parameters and custom query component are mutually exclusive. This method + * will remove custom query if present. + *

+ */ + public URIBuilder addParameter(final String param, final String value) { + if (this.queryParams == null) { + this.queryParams = new ArrayList(); + } + this.queryParams.add(new BasicNameValuePair(param, value)); + this.encodedQuery = null; + this.encodedSchemeSpecificPart = null; + this.query = null; + return this; + } + + /** + * Sets parameter of URI query overriding existing value if set. The parameter name and value + * are expected to be unescaped and may contain non ASCII characters. + *

+ * Please note query parameters and custom query component are mutually exclusive. This method + * will remove custom query if present. + *

+ */ + public URIBuilder setParameter(final String param, final String value) { + if (this.queryParams == null) { + this.queryParams = new ArrayList(); + } + if (!this.queryParams.isEmpty()) { + for (final Iterator it = this.queryParams.iterator(); it.hasNext(); ) { + final BasicNameValuePair nvp = it.next(); + if (nvp.getName().equals(param)) { + it.remove(); + } + } + } + this.queryParams.add(new BasicNameValuePair(param, value)); + this.encodedQuery = null; + this.encodedSchemeSpecificPart = null; + this.query = null; + return this; + } + + /** + * Clears URI query parameters. + */ + public URIBuilder clearParameters() { + this.queryParams = null; + this.encodedQuery = null; + this.encodedSchemeSpecificPart = null; + return this; + } + + /** + * Sets custom URI query. The value is expected to be unescaped and may contain non ASCII + * characters. + *

+ * Please note query parameters and custom query component are mutually exclusive. This method + * will remove query parameters if present. + *

+ */ + public URIBuilder setCustomQuery(final String query) { + this.query = query; + this.encodedQuery = null; + this.encodedSchemeSpecificPart = null; + this.queryParams = null; + return this; + } + + /** + * Sets URI fragment. The value is expected to be unescaped and may contain non ASCII + * characters. + */ + public URIBuilder setFragment(final String fragment) { + this.fragment = fragment; + this.encodedFragment = null; + return this; + } + + public boolean isAbsolute() { + return this.scheme != null; + } + + public boolean isOpaque() { + return this.path == null; + } + + public String getScheme() { + return this.scheme; + } + + public String getUserInfo() { + return this.userInfo; + } + + public String getHost() { + return this.host; + } + + public int getPort() { + return this.port; + } + + public String getPath() { + return this.path; + } + + public List getQueryParams() { + if (this.queryParams != null) { + return new ArrayList(this.queryParams); + } + return new ArrayList(); + + } + + public String getFragment() { + return this.fragment; + } + + @Override + public String toString() { + return buildString(); + } + + private static String normalizePath(final String path) { + String s = path; + if (s == null) { + return null; + } + int n = 0; + for (; n < s.length(); n++) { + if (s.charAt(n) != '/') { + break; + } + } + if (n > 1) { + s = s.substring(n - 1); + } + return s; + } + + public static class BasicNameValuePair implements Cloneable, Serializable { + private static final long serialVersionUID = -6437800749411518984L; + + private final String name; + private final String value; + + /** + * Default Constructor taking a name and a value. The value may be null. + * + * @param name The name. + * @param value The value. + */ + public BasicNameValuePair(final String name, final String value) { + super(); + this.name = name; + this.value = value; + } + + public String getName() { + return this.name; + } + + public String getValue() { + return this.value; + } + + @Override + public String toString() { + // don't call complex default formatting for a simple toString + + if (this.value == null) { + return name; + } + final int len = this.name.length() + 1 + this.value.length(); + final StringBuilder buffer = new StringBuilder(len); + buffer.append(this.name); + buffer.append("="); + buffer.append(this.value); + return buffer.toString(); + } + + @Override + public boolean equals(final Object object) { + if (this == object) { + return true; + } + + if (object == null) { + return false; + } + + if (object instanceof BasicNameValuePair) { + final BasicNameValuePair that = (BasicNameValuePair) object; + return this.name.equals(that.name) + && this.value.equals(that.value); + } + return false; + } + + @Override + public int hashCode() { + return 133 * this.name.hashCode() * this.value.hashCode(); + } + + @Override + public Object clone() throws CloneNotSupportedException { + return super.clone(); + } + + } +} 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 1b28ad5..aaac4e6 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 @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Collection; import junit.framework.TestCase; +import org.jasig.cas.client.Protocol; import org.jasig.cas.client.PublicTestHttpServer; import org.jasig.cas.client.ssl.HttpsURLConnectionFactory; import org.springframework.mock.web.MockHttpServletRequest; @@ -132,11 +133,96 @@ public final class CommonUtilsTests extends TestCase { request.setSecure(true); final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", - "ticket", false); + "service", "ticket", false); assertEquals(CONST_MY_URL, constructedUrl); } + public void testConstructServiceUrlWithParamsCas() { + 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("service=this&ticket=that&custom=custom"); + + final MockHttpServletResponse response = new MockHttpServletResponse(); + final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", + Protocol.CAS3.getServiceParameterName(), Protocol.CAS3.getArtifactParameterName() , false); + + assertEquals("https://www.myserver.com/hello/hithere/?custom=custom", constructedUrl); + } + + + public void testConstructServiceUrlWithParamsSaml() { + 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=this&SAMLart=that&custom=custom"); + + final MockHttpServletResponse response = new MockHttpServletResponse(); + final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", + Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName() , false); + + assertEquals("https://www.myserver.com/hello/hithere/?custom=custom", constructedUrl); + } + + public void testConstructServiceUrlWithEncodedParamsSaml() { + 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%3Dthis%26SAMLart%3Dthat%26custom%3Dcustom"); + + final MockHttpServletResponse response = new MockHttpServletResponse(); + final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", + Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName() , false); + + 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/"); + request.setScheme("https"); + request.setSecure(true); + request.setQueryString("TARGET%3Dthis%26SAMLart%3Dthat%26custom%3Dcustom%20value%20here%26another%3Dgood"); + + final MockHttpServletResponse response = new MockHttpServletResponse(); + final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", + Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName() , true); + + assertEquals("https://www.myserver.com/hello/hithere/?custom=custom+value+here&another=good", constructedUrl); + } + + public void testConstructServiceUrlWithoutEncodedParamsSamlAndNoEncoding() { + 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=this&SAMLart=that&custom=custom value here&another=good"); + + final MockHttpServletResponse response = new MockHttpServletResponse(); + final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", + Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName() , false); + + assertEquals("https://www.myserver.com/hello/hithere/?custom=custom value here&another=good", constructedUrl); + } + + public void testConstructServiceUrlWithEncodedParamsSamlAndNoEncoding() { + 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=this&SAMLart=that&custom=custom+value+here&another=good"); + + final MockHttpServletResponse response = new MockHttpServletResponse(); + final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", + Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName() , true); + + assertEquals("https://www.myserver.com/hello/hithere/?custom=custom+value+here&another=good", 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/"); @@ -146,7 +232,7 @@ public final class CommonUtilsTests extends TestCase { request.setServerPort(555); final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, - serverNameList, "ticket", false); + serverNameList, "service", "ticket", false); assertEquals(CONST_MY_URL, constructedUrl); } @@ -166,7 +252,7 @@ public final class CommonUtilsTests extends TestCase { request.setSecure(true); final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, - "www.amazon.com www.bestbuy.com www.myserver.com", "ticket", false); + "www.amazon.com www.bestbuy.com www.myserver.com", "service", "ticket", false); assertEquals(CONST_MY_URL, constructedUrl); } @@ -178,7 +264,7 @@ public final class CommonUtilsTests extends TestCase { request.setSecure(true); final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, - "http://www.amazon.com https://www.bestbuy.com https://www.myserver.com", "ticket", false); + "http://www.amazon.com https://www.bestbuy.com https://www.myserver.com", "service", "ticket", false); assertEquals(CONST_MY_URL, constructedUrl); } diff --git a/cas-client-integration-tomcat-common/src/main/java/org/jasig/cas/client/tomcat/AuthenticatorDelegate.java b/cas-client-integration-tomcat-common/src/main/java/org/jasig/cas/client/tomcat/AuthenticatorDelegate.java index 3f92d32..9fae1a6 100644 --- a/cas-client-integration-tomcat-common/src/main/java/org/jasig/cas/client/tomcat/AuthenticatorDelegate.java +++ b/cas-client-integration-tomcat-common/src/main/java/org/jasig/cas/client/tomcat/AuthenticatorDelegate.java @@ -86,7 +86,7 @@ public final class AuthenticatorDelegate { logger.debug("CAS assertion not found in session -- authentication required."); final String token = request.getParameter(this.artifactParameterName); final String service = CommonUtils.constructServiceUrl(request, response, this.serviceUrl, this.serverName, - this.artifactParameterName, true); + this.serviceParameterName, this.artifactParameterName, true); if (CommonUtils.isBlank(token)) { final String redirectUrl = CommonUtils.constructRedirectUrl(this.casServerLoginUrl, this.serviceParameterName, service, false, false); From 3080484a885987e4043ab10805cbd28ac0771d44 Mon Sep 17 00:00:00 2001 From: Misagh Moayyed Date: Fri, 5 Jun 2015 15:17:04 -0700 Subject: [PATCH 2/2] updated param building --- .../src/main/java/org/jasig/cas/client/util/URIBuilder.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/util/URIBuilder.java b/cas-client-core/src/main/java/org/jasig/cas/client/util/URIBuilder.java index 5cad257..ef0db52 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/util/URIBuilder.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/util/URIBuilder.java @@ -75,7 +75,7 @@ public final class URIBuilder { * Construct an instance from the string which must be a valid URI. * * @param string a valid URI in string form - * @throws URISyntaxException if the input is not a valid URI + * @throws RuntimeException if the input is not a valid URI */ public URIBuilder(final String string) { super(); @@ -99,7 +99,7 @@ public final class URIBuilder { /** * Construct an instance from the provided URI. - * @param uri + * @param uri the uri to digest */ public URIBuilder(final URI uri) { super(); @@ -354,7 +354,7 @@ public final class URIBuilder { *

*/ public URIBuilder addParameters(final List nvps) { - if (this.queryParams == null) { + if (this.queryParams == null || this.queryParams.isEmpty()) { this.queryParams = new ArrayList(); } this.queryParams.addAll(nvps);