From 3875c39a219f9ccad70a3372dee8edf3d4b2faf8 Mon Sep 17 00:00:00 2001 From: Misagh Moayyed Date: Wed, 31 Jan 2018 14:11:00 +0330 Subject: [PATCH] handle un-encoded query strings in url parameters --- .../org/jasig/cas/client/util/URIBuilder.java | 40 +++++++++----- .../cas/client/util/CommonUtilsTests.java | 54 ++++++++++++------- 2 files changed, 60 insertions(+), 34 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 7d61175..f4c0b1a 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 @@ -28,13 +28,13 @@ 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 */ @@ -99,14 +99,15 @@ public final class URIBuilder { /** * Construct an instance from the provided URI. - * @param uri the uri to digest + * + * @param uri the uri to digest */ public URIBuilder(final URI uri) { super(); digestURI(uri); } - private List parseQuery(final String query) { + private List parseQuery(final String query) { try { final Charset utf8 = Charset.forName("UTF-8"); @@ -115,11 +116,23 @@ public final class URIBuilder { final String[] parametersArray = query.split("&"); for (final String parameter : parametersArray) { - final String[] parameterCombo = parameter.split("="); - if (parameterCombo.length >= 1) { - final String key = URLDecoder.decode(parameterCombo[0], utf8.name()); - final String val = parameterCombo.length == 2 ? URLDecoder.decode(parameterCombo[1], utf8.name()) : ""; - list.add(new BasicNameValuePair(key, val)); + final int firstIndex = parameter.indexOf("="); + if (firstIndex != -1) { + final String paramName = parameter.substring(0, firstIndex); + final String decodedParamName = URLDecoder.decode(paramName, utf8.name()); + + final String paramVal = parameter.substring(firstIndex + 1); + final String decodedParamVal = URLDecoder.decode(paramVal, utf8.name()); + + list.add(new BasicNameValuePair(decodedParamName, decodedParamVal)); + } else { + // Either we do not have a query parameter, or it might be encoded; take it verbaitm + final String[] parameterCombo = parameter.split("="); + if (parameterCombo.length >= 1) { + final String key = URLDecoder.decode(parameterCombo[0], utf8.name()); + final String val = parameterCombo.length == 2 ? URLDecoder.decode(parameterCombo[1], utf8.name()) : ""; + list.add(new BasicNameValuePair(key, val)); + } } } return list; @@ -327,7 +340,7 @@ public final class URIBuilder { * will remove custom query if present. *

*/ - public URIBuilder setParameters(final List nvps) { + public URIBuilder setParameters(final List nvps) { this.queryParams = new ArrayList(); this.queryParams.addAll(nvps); this.encodedQuery = null; @@ -346,7 +359,6 @@ public final class URIBuilder { } - /** * Adds URI query parameters. The parameter name / values are expected to be unescaped * and may contain non ASCII characters. @@ -355,7 +367,7 @@ public final class URIBuilder { * will remove custom query if present. *

*/ - public URIBuilder addParameters(final List nvps) { + public URIBuilder addParameters(final List nvps) { if (this.queryParams == null || this.queryParams.isEmpty()) { this.queryParams = new ArrayList(); } @@ -380,7 +392,7 @@ public final class URIBuilder { } else { this.queryParams.clear(); } - for (final BasicNameValuePair nvp: nvps) { + for (final BasicNameValuePair nvp : nvps) { this.queryParams.add(nvp); } this.encodedQuery = null; @@ -602,7 +614,7 @@ public final class URIBuilder { /** * Default Constructor taking a name and a value. The value may be null. * - * @param name The name. + * @param name The name. * @param value The value. */ public BasicNameValuePair(final String name, final String value) { @@ -647,7 +659,7 @@ public final class URIBuilder { if (object instanceof BasicNameValuePair) { final BasicNameValuePair that = (BasicNameValuePair) object; return this.name.equals(that.name) - && this.value.equals(that.value); + && this.value.equals(that.value); } return false; } 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 b2a9d13..c924933 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 @@ -18,17 +18,18 @@ */ package org.jasig.cas.client.util; -import java.net.URL; -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.junit.Ignore; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import java.net.URL; +import java.util.ArrayList; +import java.util.Collection; + /** * Tests for the CommonUtils. * @@ -133,7 +134,7 @@ 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", - "service", "ticket", false); + "service", "ticket", false); assertEquals(CONST_MY_URL, constructedUrl); } @@ -147,7 +148,7 @@ public final class CommonUtilsTests extends TestCase { final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", - Protocol.CAS3.getServiceParameterName(), Protocol.CAS3.getArtifactParameterName() , false); + Protocol.CAS3.getServiceParameterName(), Protocol.CAS3.getArtifactParameterName(), false); assertEquals("https://www.myserver.com/hello/hithere/?custom=custom", constructedUrl); } @@ -161,7 +162,7 @@ public final class CommonUtilsTests extends TestCase { final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "https://www.myserver.com", - Protocol.CAS3.getServiceParameterName(), Protocol.CAS3.getArtifactParameterName() , false); + Protocol.CAS3.getServiceParameterName(), Protocol.CAS3.getArtifactParameterName(), false); assertEquals("https://www.myserver.com/hello/hithere/?custom=custom", constructedUrl); } @@ -176,7 +177,7 @@ public final class CommonUtilsTests extends TestCase { final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", - Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName() , false); + Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName(), false); assertEquals("https://www.myserver.com/hello/hithere/?custom=custom", constructedUrl); } @@ -190,7 +191,7 @@ public final class CommonUtilsTests extends TestCase { final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", - Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName() , false); + Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName(), false); assertEquals("https://www.myserver.com/hello/hithere/?custom=custom", constructedUrl); } @@ -204,13 +205,12 @@ public final class CommonUtilsTests extends TestCase { final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", - Protocol.SAML11.getArtifactParameterName() , true); + 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/"); request.setScheme("https"); request.setSecure(true); @@ -218,13 +218,12 @@ public final class CommonUtilsTests extends TestCase { final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", - Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName() , true); + 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); @@ -232,7 +231,7 @@ public final class CommonUtilsTests extends TestCase { final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", - Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName() , false); + Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName(), false); assertEquals("https://www.myserver.com/hello/hithere/?custom=custom value here&another=good", constructedUrl); } @@ -246,7 +245,7 @@ public final class CommonUtilsTests extends TestCase { final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, "www.myserver.com", - Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName() , true); + Protocol.SAML11.getServiceParameterName(), Protocol.SAML11.getArtifactParameterName(), true); assertEquals("https://www.myserver.com/hello/hithere/?custom=custom+value+here&another=good", constructedUrl); } @@ -260,7 +259,7 @@ public final class CommonUtilsTests extends TestCase { request.setServerPort(555); final MockHttpServletResponse response = new MockHttpServletResponse(); final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, - serverNameList, "service", "ticket", false); + serverNameList, "service", "ticket", false); assertEquals(CONST_MY_URL, constructedUrl); } @@ -280,7 +279,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", "service", "ticket", false); + "www.amazon.com www.bestbuy.com www.myserver.com", "service", "ticket", false); assertEquals(CONST_MY_URL, constructedUrl); } @@ -292,7 +291,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", "service", "ticket", false); + "http://www.amazon.com https://www.bestbuy.com https://www.myserver.com", "service", "ticket", false); assertEquals(CONST_MY_URL, constructedUrl); } @@ -303,8 +302,23 @@ public final class CommonUtilsTests extends TestCase { final String responsedContent = CommonUtils.getResponseFromServer(new URL("http://localhost:8090"), new HttpsURLConnectionFactory(), null); assertEquals(RESPONSE, responsedContent); } - + public void testUrlEncode() { - assertEquals("this+is+a+very+special+parameter+with+%3D%25%2F", CommonUtils.urlEncode("this is a very special parameter with =%/")); + assertEquals("this+is+a+very+special+parameter+with+%3D%25%2F", + CommonUtils.urlEncode("this is a very special parameter with =%/")); + } + + public void testUrlEncodeWithQueryParameters() { + final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/idp/authN/ExtCas"); + request.setQueryString("conversation=e1s1&ticket=ST-1234-123456789-a&entityId=https://test.edu/sp?alias=1234-1234-1234-1234&something=else"); + request.addHeader("Host", "www.myserver.com"); + request.setScheme("https"); + request.setSecure(true); + final MockHttpServletResponse response = new MockHttpServletResponse(); + final String constructedUrl = CommonUtils.constructServiceUrl(request, response, null, + "https://my.server.com", + "service", "ticket", false); + assertEquals("https://my.server.com/idp/authN/ExtCas?conversation=e1s1&entityId=https://test.edu/sp?alias=1234-1234-1234-1234&something=else", + constructedUrl); } }