Fix #192 - Correctly unescape search parameters in the server when they

have a trailing comma or an escaped backslash

.travis.yml

@@ -14,12 +14,11 @@ cache:
 
 install: /bin/true
 
+# This seems to be required to get travis to set Xmx4g, per https://github.com/travis-ci/travis-ci/issues/3893
 before_script:
   - export MAVEN_SKIP_RC=true
 
 script:
-  - export MAVEN_OPTS="-XX:MaxPermSize=512m -Xmx4g"
   - mvn -B clean install && cd hapi-fhir-cobertura && mvn -B -DTRAVIS_JOB_ID=$TRAVIS_JOB_ID -P COBERTURA clean cobertura:cobertura coveralls:report 
-#  - mvn -B clean install -Dcobertura.skip=true && mvn -B -DTRAVIS_JOB_ID=$TRAVIS_JOB_ID -P COBERTURA clean cobertura:cobertura coveralls:report 
 
 

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/QualifiedParamList.java

@@ -1,5 +1,7 @@
 package ca.uhn.fhir.rest.method;
 
+import static org.apache.commons.lang3.StringUtils.isBlank;
+
 /*
  * #%L
  * HAPI FHIR - Core Library
@@ -29,21 +31,21 @@ import ca.uhn.fhir.model.api.IQueryParameterType;
 public class QualifiedParamList extends ArrayList<String> {
 
 	private static final long serialVersionUID = 1L;
-	
+
 	private String myQualifier;
 
 	public QualifiedParamList() {
 		super();
 	}
-	
+
 	public QualifiedParamList(int theCapacity) {
 		super(theCapacity);
 	}
 
 	public QualifiedParamList(IQueryParameterOr<?> theNextOr) {
 		for (IQueryParameterType next : theNextOr.getValuesAsQueryTokens()) {
-			if (myQualifier==null) {
+			if (myQualifier == null) {
-				myQualifier=next.getQueryParameterQualifier();
+				myQualifier = next.getQueryParameterQualifier();
 			}
 			add(next.getValueAsQueryToken());
 		}
@@ -60,36 +62,68 @@ public class QualifiedParamList extends ArrayList<String> {
 	public static QualifiedParamList singleton(String theParamValue) {
 		return singleton(null, theParamValue);
 	}
-	
+
 	public static QualifiedParamList singleton(String theQualifier, String theParamValue) {
 		QualifiedParamList retVal = new QualifiedParamList(1);
 		retVal.setQualifier(theQualifier);
 		retVal.add(theParamValue);
 		return retVal;
 	}
-	
+
-	
+	public static QualifiedParamList splitQueryStringByCommasIgnoreEscape(String theQualifier, String theParams) {
-	public static QualifiedParamList splitQueryStringByCommasIgnoreEscape(String theQualifier, String theParams){
+		QualifiedParamList retVal = new QualifiedParamList();
-		 QualifiedParamList retVal = new QualifiedParamList();
+		retVal.setQualifier(theQualifier);
-		 retVal.setQualifier(theQualifier);
+
-		
+		StringTokenizer tok = new StringTokenizer(theParams, ",", true);
-		StringTokenizer tok = new StringTokenizer(theParams,",");
+		String prev = null;
-		String prev=null;
 		while (tok.hasMoreElements()) {
 			String str = tok.nextToken();
-			if (prev!=null&&prev.endsWith("\\")) {
+			if (isBlank(str)) {
-				int idx = retVal.size()-1;
+				prev = null;
-				String existing = retVal.get(idx);
+				continue;
-				retVal.set(idx, existing.substring(0, existing.length()-1) + "," + str);
-			}else {
-				retVal.add(str);
 			}
 			
-			prev=str;
+			if (str.equals(",")) {
+					if (countTrailingSlashes(prev) % 2 == 1) {
+						int idx = retVal.size() - 1;
+						String existing = retVal.get(idx);
+						prev = existing.substring(0, existing.length() - 1) + ',';
+						retVal.set(idx, prev);
+					} else {
+						prev = null;
+					}
+				continue;
+			}
+
+			if (prev != null && prev.length() > 0 && prev.charAt(prev.length() - 1) == ',') {
+				int idx = retVal.size() - 1;
+				String existing = retVal.get(idx);
+				prev = existing + str;
+				retVal.set(idx, prev);
+			} else {
+				retVal.add(str);
+				prev = str;
+			}
+
+		}
+
+		return retVal;
+	}
+
+	private static int countTrailingSlashes(String theString) {
+		if(theString==null) {
+			return 0;
+		}
+		int retVal = 0;
+		for (int i = theString.length() - 1; i >= 0; i--) {
+			char nextChar = theString.charAt(i);
+			if (nextChar != '\\') {
+				break;
+			} else {
+				retVal++;
+			}
 		}
-		
 		return retVal;
 	}
 
-	
 }

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ParameterUtil.java

@@ -255,6 +255,7 @@ public class ParameterUtil {
 					case '$':
 					case ',':
 					case '|':
+					case '\\':
 						continue;
 					default:
 						b.append(next);

hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/TokenParameterTest.java

@@ -49,16 +49,90 @@ public class TokenParameterTest {
 	 * Test #192
 	 */
 	@Test
-	public void testOrListWithEscapedValue() throws Exception {
+	public void testOrListWithEscapedValue1() throws Exception {
 		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=" + UrlUtil.escape("system|code-include-but-not-end-with-comma\\,suffix"));
 		HttpResponse status = ourClient.execute(httpGet);
 		IOUtils.closeQuietly(status.getEntity().getContent());
 
 		assertEquals(200, status.getStatusLine().getStatusCode());
 
+		assertEquals("system", ourLastOrList.getListAsCodings().get(0).getSystemElement().getValue());
+		assertEquals("code-include-but-not-end-with-comma,suffix", ourLastOrList.getListAsCodings().get(0).getCodeElement().getValue());
+		assertEquals(1, ourLastOrList.getListAsCodings().size());
+	}
+
+	/**
+	 * Test #192
+	 */
+	@Test
+	public void testOrListWithEscapedValue2() throws Exception {
+		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=" + UrlUtil.escape("system|code-include-end-with-comma\\,"));
+		HttpResponse status = ourClient.execute(httpGet);
+		IOUtils.closeQuietly(status.getEntity().getContent());
+
+		assertEquals(200, status.getStatusLine().getStatusCode());
+
 		assertEquals(1, ourLastOrList.getListAsCodings().size());
 		assertEquals("system", ourLastOrList.getListAsCodings().get(0).getSystemElement().getValue());
-		assertEquals("code-include-but-not-end-with-comma,suffix", ourLastOrList.getListAsCodings().get(0).getCodeElement().getValue());
+		assertEquals("code-include-end-with-comma,", ourLastOrList.getListAsCodings().get(0).getCodeElement().getValue());
+	}
+
+	/**
+	 * Test #192
+	 */
+	@Test
+	public void testOrListWithEscapedValue3() throws Exception {
+		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=" + UrlUtil.escape("system|code-include-end-with-comma1,system|code-include-end-with-comma2,,,,,"));
+		HttpResponse status = ourClient.execute(httpGet);
+		IOUtils.closeQuietly(status.getEntity().getContent());
+
+		assertEquals(200, status.getStatusLine().getStatusCode());
+
+		assertEquals(2, ourLastOrList.getListAsCodings().size());
+		assertEquals("system", ourLastOrList.getListAsCodings().get(0).getSystemElement().getValue());
+		assertEquals("code-include-end-with-comma1", ourLastOrList.getListAsCodings().get(0).getCodeElement().getValue());
+		assertEquals("system", ourLastOrList.getListAsCodings().get(1).getSystemElement().getValue());
+		assertEquals("code-include-end-with-comma2", ourLastOrList.getListAsCodings().get(1).getCodeElement().getValue());
+	}
+
+	/**
+	 * Test #192
+	 */
+	@Test
+	public void testOrListWithEscapedValue4() throws Exception {
+		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=" + UrlUtil.escape("\\,\\,\\,value1\\,\\,\\,with\\,\\,\\,commas\\,\\,\\,,,,\\,\\,\\,value2\\,\\,\\,with\\,\\,\\,commas,,,\\,"));
+		HttpResponse status = ourClient.execute(httpGet);
+		IOUtils.closeQuietly(status.getEntity().getContent());
+
+		assertEquals(200, status.getStatusLine().getStatusCode());
+
+		assertEquals(null, ourLastOrList.getListAsCodings().get(0).getSystemElement().getValue());
+		assertEquals(",,,value1,,,with,,,commas,,,", ourLastOrList.getListAsCodings().get(0).getCodeElement().getValue());
+		assertEquals(null, ourLastOrList.getListAsCodings().get(1).getSystemElement().getValue());
+		assertEquals(",,,value2,,,with,,,commas", ourLastOrList.getListAsCodings().get(1).getCodeElement().getValue());
+		assertEquals(null, ourLastOrList.getListAsCodings().get(2).getSystemElement().getValue());
+		assertEquals(",", ourLastOrList.getListAsCodings().get(2).getCodeElement().getValue());
+		assertEquals(3, ourLastOrList.getListAsCodings().size());
+	}
+	
+	/**
+	 * Test #192
+	 */
+	@Test
+	public void testOrListWithEscapedValue5() throws Exception {
+		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=" + UrlUtil.escape("A\\\\,B,\\$"));
+		HttpResponse status = ourClient.execute(httpGet);
+		IOUtils.closeQuietly(status.getEntity().getContent());
+
+		assertEquals(200, status.getStatusLine().getStatusCode());
+
+		assertEquals(null, ourLastOrList.getListAsCodings().get(0).getSystemElement().getValue());
+		assertEquals("A\\", ourLastOrList.getListAsCodings().get(0).getCodeElement().getValue());
+		assertEquals(null, ourLastOrList.getListAsCodings().get(1).getSystemElement().getValue());
+		assertEquals("B", ourLastOrList.getListAsCodings().get(1).getCodeElement().getValue());
+		assertEquals(null, ourLastOrList.getListAsCodings().get(2).getSystemElement().getValue());
+		assertEquals("$", ourLastOrList.getListAsCodings().get(2).getCodeElement().getValue());
+		assertEquals(3, ourLastOrList.getListAsCodings().size());
 	}
 
 	@AfterClass

src/changes/changes.xml

@@ -67,6 +67,11 @@
 			<action type="add">
 				JPA server and generic client now both support the _tag search parameter
 			</action>
+			<action type="fix" issue="192">
+				Server was not correctly unescaping URL parameter values with
+				a trailing comma or an escaped backslash. Thanks to GitHub user
+				@SherryH for all of her help in diagnosing this issue!
+			</action>
 		</release>
 		<release version="1.1" date="2015-07-13">
 			<action type="add">