Add parser for URL parameters to avoid Servlets using ISO-8859-1 instead of UTF-8

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java

@@ -160,6 +160,7 @@ public class Constants {
 	public static final String URL_TOKEN_HISTORY = "_history";
 	
 	public static final String URL_TOKEN_METADATA = "metadata";
+	public static final String CT_X_FORM_URLENCODED = "application/x-www-form-urlencoded";
 
 	static {
 		Map<String, EncodingEnum> valToEncoding = new HashMap<String, EncodingEnum>();

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java

@@ -20,6 +20,7 @@ package ca.uhn.fhir.rest.server;
  * #L%
  */
 import static org.apache.commons.lang3.StringUtils.isBlank;
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -46,6 +47,7 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.commons.io.Charsets;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.Validate;
@@ -100,6 +102,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 	private EncodingEnum myDefaultResponseEncoding = EncodingEnum.XML;
 	private ETagSupportEnum myETagSupport = DEFAULT_ETAG_SUPPORT;
 	private FhirContext myFhirContext;
+	private boolean myIgnoreServerParsedRequestParameters = true;
 	private String myImplementationDescription;
 	private final List<IServerInterceptor> myInterceptors = new ArrayList<IServerInterceptor>();
 	private IPagingProvider myPagingProvider;
@@ -118,6 +121,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 	private Map<String, IResourceProvider> myTypeToProvider = new HashMap<String, IResourceProvider>();
 	private boolean myUncompressIncomingContents = true;
 	private boolean myUseBrowserFriendlyContentTypes;
+	
 	/**
 	 * Constructor. Note that if no {@link FhirContext} is passed in to the server (either through the constructor, or through {@link #setFhirContext(FhirContext)}) the server will determine which
 	 * version of FHIR to support through classpath scanning. This is brittle, and it is highly recommended to explicitly specify a FHIR version.
@@ -562,9 +566,27 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 
 			fhirServerBase = getServerBaseForRequest(theRequest);
 
-			String completeUrl = StringUtils.isNotBlank(theRequest.getQueryString()) ? requestUrl + "?" + theRequest.getQueryString() : requestUrl.toString();
+			String completeUrl;
+			Map<String, String[]> params = null;
+			if (StringUtils.isNotBlank(theRequest.getQueryString())) {
+				completeUrl = requestUrl + "?" + theRequest.getQueryString();
+				if (isIgnoreServerParsedRequestParameters()) {
+					String contentType = theRequest.getHeader(Constants.HEADER_CONTENT_TYPE);
+					if (theRequestType == RequestTypeEnum.POST && isNotBlank(contentType) && contentType.startsWith(Constants.CT_X_FORM_URLENCODED)) {
+						String requestBody = new String(requestDetails.loadRequestContents(), Charsets.UTF_8);
+						params = UrlUtil.parseQueryStrings(theRequest.getQueryString(), requestBody);
+					} else if (theRequestType == RequestTypeEnum.GET) {
+						params = UrlUtil.parseQueryString(theRequest.getQueryString());
+					}
+				}
+			} else {
+				completeUrl = requestUrl.toString();
+			}
+			
+			if (params == null) {
+				params = new HashMap<String, String[]>(theRequest.getParameterMap());
+			}
 
-			Map<String, String[]> params = new HashMap<String, String[]>(theRequest.getParameterMap());
 			requestDetails.setParameters(params);
 
 			IIdType id;
@@ -852,6 +874,20 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 		return myDefaultPrettyPrint;
 	}
 
+	/**
+	 * If set to <code>true</code> (the default is <code>true</code>) this server will not 
+	 * use the parsed request parameters (URL parameters and HTTP POST form contents) but
+	 * will instead parse these values manually from the request URL and request body.
+	 * <p>
+	 * This is useful because many servlet containers (e.g. Tomcat, Glassfish) will use 
+	 * ISO-8859-1 encoding to parse escaped URL characters instead of using UTF-8
+	 * as is specified by FHIR. 
+	 * </p>
+	 */
+	public boolean isIgnoreServerParsedRequestParameters() {
+		return myIgnoreServerParsedRequestParameters;
+	}
+
 	/**
 	 * Should the server attempt to decompress incoming request contents (default is <code>true</code>). Typically this should be set to <code>true</code> unless the server has other configuration to
 	 * deal with decompressing request bodies (e.g. a filter applied to the whole server).
@@ -1184,6 +1220,20 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 		myFhirContext = theFhirContext;
 	}
 
+	/**
+	 * If set to <code>true</code> (the default is <code>true</code>) this server will not 
+	 * use the parsed request parameters (URL parameters and HTTP POST form contents) but
+	 * will instead parse these values manually from the request URL and request body.
+	 * <p>
+	 * This is useful because many servlet containers (e.g. Tomcat, Glassfish) will use 
+	 * ISO-8859-1 encoding to parse escaped URL characters instead of using UTF-8
+	 * as is specified by FHIR. 
+	 * </p>
+	 */
+	public void setIgnoreServerParsedRequestParameters(boolean theIgnoreServerParsedRequestParameters) {
+		myIgnoreServerParsedRequestParameters = theIgnoreServerParsedRequestParameters;
+	}
+
 	public void setImplementationDescription(String theImplementationDescription) {
 		myImplementationDescription = theImplementationDescription;
 	}

hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java

@@ -1,12 +1,19 @@
 package ca.uhn.fhir.util;
 
 import static org.apache.commons.lang3.StringUtils.defaultIfBlank;
+import static org.apache.commons.lang3.StringUtils.isBlank;
 
 import java.io.UnsupportedEncodingException;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLDecoder;
 import java.net.URLEncoder;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.StringTokenizer;
 
 import ca.uhn.fhir.model.primitive.IdDt;
 import ca.uhn.fhir.rest.server.Constants;
@@ -35,10 +42,6 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 public class UrlUtil {
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(UrlUtil.class);
 
-	public static void main(String[] args) {
-		System.out.println(escape("http://snomed.info/sct?fhir_vs=isa/126851005"));
-	}
-
 	/**
 	 * Resolve a relative URL - THIS METHOD WILL NOT FAIL but will log a warning and return theEndpoint if the input is invalid.
 	 */
@@ -61,11 +64,6 @@ public class UrlUtil {
 		}
 	}
 
-	public static boolean isAbsolute(String theValue) {
-		String value = theValue.toLowerCase();
-		return value.startsWith("http://") || value.startsWith("https://");
-	}
-
 	public static String constructRelativeUrl(String theParentExtensionUrl, String theExtensionUrl) {
 		if (theParentExtensionUrl == null) {
 			return theExtensionUrl;
@@ -96,6 +94,25 @@ public class UrlUtil {
 		return theExtensionUrl;
 	}
 
+	/**
+	 * URL encode a value
+	 */
+	public static String escape(String theValue) {
+		if (theValue == null) {
+			return null;
+		}
+		try {
+			return URLEncoder.encode(theValue, "UTF-8");
+		} catch (UnsupportedEncodingException e) {
+			throw new Error("UTF-8 not supported on this platform");
+		}
+	}
+
+	public static boolean isAbsolute(String theValue) {
+		String value = theValue.toLowerCase();
+		return value.startsWith("http://") || value.startsWith("https://");
+	}
+
 	public static boolean isValid(String theUrl) {
 		if (theUrl == null || theUrl.length() < 8) {
 			return false;
@@ -136,31 +153,66 @@ public class UrlUtil {
 		return true;
 	}
 
-	public static String unescape(String theString) {
+	public static void main(String[] args) {
-		if (theString == null) {
+		System.out.println(escape("http://snomed.info/sct?fhir_vs=isa/126851005"));
-			return null;
-		}
-		if (theString.indexOf('%') == -1) {
-			return theString;
-		}
-		try {
-			return URLDecoder.decode(theString, "UTF-8");
-		} catch (UnsupportedEncodingException e) {
-			throw new Error("UTF-8 not supported, this shouldn't happen", e);
-		}
 	}
 
-	/**
+	public static Map<String, String[]> parseQueryString(String theQueryString) {
-	 * URL encode a value
+		HashMap<String, List<String>> map = new HashMap<String, List<String>>();
-	 */
+		parseQueryString(theQueryString, map);
-	public static String escape(String theValue) {
+		return toQueryStringMap(map);
-		if (theValue == null) {
-			return null;
 	}
-		try {
+
-			return URLEncoder.encode(theValue, "UTF-8");
+	public static Map<String, String[]> parseQueryStrings(String... theQueryString) {
-		} catch (UnsupportedEncodingException e) {
+		HashMap<String, List<String>> map = new HashMap<String, List<String>>();
-			throw new Error("UTF-8 not supported on this platform");
+		for (String next : theQueryString) {
+			parseQueryString(next, map);
+		}
+		return toQueryStringMap(map);
+	}
+
+	private static Map<String, String[]> toQueryStringMap(HashMap<String, List<String>> map) {
+		HashMap<String, String[]> retVal = new HashMap<String, String[]>();
+		for (Entry<String, List<String>> nextEntry : map.entrySet()) {
+			retVal.put(nextEntry.getKey(), nextEntry.getValue().toArray(new String[nextEntry.getValue().size()]));
+		}
+		return retVal;
+	}
+
+	private static void parseQueryString(String theQueryString, HashMap<String, List<String>> map) {
+		String query = theQueryString;
+		if (query.startsWith("?")) {
+			query = query.substring(1);
+		}
+		
+		
+		StringTokenizer tok = new StringTokenizer(query, "&");
+		while (tok.hasMoreTokens()) {
+			String nextToken = tok.nextToken();
+			if (isBlank(nextToken)) {
+				continue;
+			}
+			
+			int equalsIndex = nextToken.indexOf('=');
+			String nextValue;
+			String nextKey;
+			if (equalsIndex == -1) {
+				nextKey = nextToken;
+				nextValue = "";
+			} else {
+				nextKey = nextToken.substring(0, equalsIndex);
+				nextValue = nextToken.substring(equalsIndex + 1);
+			}
+	
+			nextKey = unescape(nextKey);
+			nextValue = unescape(nextValue);
+			
+			List<String> list = map.get(nextKey);
+			if (list == null) {
+				list = new ArrayList<String>();
+				map.put(nextKey, list);
+			}
+			list.add(nextValue);
 		}
 	}
 
@@ -233,6 +285,20 @@ public class UrlUtil {
 		}
 	}
 
+	public static String unescape(String theString) {
+		if (theString == null) {
+			return null;
+		}
+		if (theString.indexOf('%') == -1) {
+			return theString;
+		}
+		try {
+			return URLDecoder.decode(theString, "UTF-8");
+		} catch (UnsupportedEncodingException e) {
+			throw new Error("UTF-8 not supported, this shouldn't happen", e);
+		}
+	}
+
 	public static class UrlParts {
 		private String myParams;
 		private String myResourceId;

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java

@@ -2044,6 +2044,7 @@ public class SearchBuilder {
 				} else {
 
 					List<String> paths;
+					RuntimeSearchParam param = null;
 					if (theContext.getVersion().getVersion() == FhirVersionEnum.DSTU1) {
 						paths = Collections.singletonList(nextInclude.getValue());
 					} else {
@@ -2058,7 +2059,7 @@ public class SearchBuilder {
 						}
 
 						String paramName = nextInclude.getParamName();
-						RuntimeSearchParam param = isNotBlank(paramName) ? def.getSearchParam(paramName) : null;
+						param = isNotBlank(paramName) ? def.getSearchParam(paramName) : null;
 						if (param == null) {
 							ourLog.warn("Unknown param name in include/revinclude=" + nextInclude.getValue());
 							continue;
@@ -2083,10 +2084,24 @@ public class SearchBuilder {
 						}
 						List<ResourceLink> results = q.getResultList();
 						for (ResourceLink resourceLink : results) {
+							if (param != null && param.getTargets() != null && param.getTargets().isEmpty() == false) {
+								String type;
 								if (theReverseMode) {
-								pidsToInclude.add(resourceLink.getSourceResourcePid());
+									type = resourceLink.getSourceResource().getResourceType();
 								} else {
-								pidsToInclude.add(resourceLink.getTargetResourcePid());
+									type = resourceLink.getTargetResource().getResourceType();
+								}
+								if (!param.getTargets().contains(type)) {
+									continue;
+								}
+							}
+							
+							if (theReverseMode) {
+								Long pid = resourceLink.getSourceResourcePid();
+								pidsToInclude.add(pid);
+							} else {
+								Long pid = resourceLink.getTargetResourcePid();
+								pidsToInclude.add(pid);
 							}
 						}
 					}

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java

@@ -142,7 +142,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 			String output = IOUtils.toString(resp.getEntity().getContent());
 			ourLog.info(output);
 			
-			assertThat(output, containsString("<url value=\"" + ourServerBase + "/Patient?name=Jernel%C3%B6v&amp;_pretty=true\"/>"));
+			assertThat(output, containsString("<url value=\"http://localhost:" + ourPort + "/fhir/context/Patient?_pretty=true&amp;name=Jernel%C3%B6v\"/>"));
 			assertThat(output, containsString("<family value=\"Jernelöv\"/>"));
 		} finally {
 			IOUtils.closeQuietly(resp.getEntity().getContent());

hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/SearchDstu2Test.java

@@ -15,6 +15,8 @@ import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.http.HttpResponse;
+import org.apache.http.NameValuePair;
+import org.apache.http.client.entity.UrlEncodedFormEntity;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
@@ -24,6 +26,7 @@ import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.http.message.BasicNameValuePair;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.servlet.ServletHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
@@ -40,8 +43,11 @@ import ca.uhn.fhir.model.dstu2.resource.Bundle;
 import ca.uhn.fhir.model.dstu2.resource.Bundle.Link;
 import ca.uhn.fhir.model.dstu2.resource.Patient;
 import ca.uhn.fhir.model.primitive.InstantDt;
+import ca.uhn.fhir.rest.annotation.Create;
 import ca.uhn.fhir.rest.annotation.RequiredParam;
+import ca.uhn.fhir.rest.annotation.ResourceParam;
 import ca.uhn.fhir.rest.annotation.Search;
+import ca.uhn.fhir.rest.api.MethodOutcome;
 import ca.uhn.fhir.rest.param.DateAndListParam;
 import ca.uhn.fhir.rest.param.ParamPrefixEnum;
 import ca.uhn.fhir.rest.param.QuantityParam;
@@ -63,6 +69,7 @@ public class SearchDstu2Test {
 	private static int ourPort;
 	private static InstantDt ourReturnPublished;
 	private static Server ourServer;
+	private static RestfulServer ourServlet;
 
 	@Before
 	public void before() {
@@ -70,8 +77,29 @@ public class SearchDstu2Test {
 		ourLastDateAndList = null;
 		ourLastRef = null;
 		ourLastQuantity = null;
+		ourServlet.setIgnoreServerParsedRequestParameters(true);
 	}
 
+	@Test
+	public void testSearchWithInvalidPostUrl() throws Exception {
+		// should end with _search
+		HttpPost filePost = new HttpPost("http://localhost:" + ourPort + "/Patient?name=Central"); 
+
+		// add parameters to the post method
+		List<NameValuePair> parameters = new ArrayList<NameValuePair>();
+		parameters.add(new BasicNameValuePair("_id", "aaa"));
+
+		UrlEncodedFormEntity sendentity = new UrlEncodedFormEntity(parameters, "UTF-8");
+		filePost.setEntity(sendentity);
+
+		HttpResponse status = ourClient.execute(filePost);
+		String responseContent = IOUtils.toString(status.getEntity().getContent());
+		IOUtils.closeQuietly(status.getEntity().getContent());
+		ourLog.info(responseContent);
+		assertEquals(400, status.getStatusLine().getStatusCode());
+		assertThat(responseContent, containsString("<diagnostics value=\"Incorrect Content-Type header value of &quot;application/x-www-form-urlencoded; charset=UTF-8&quot; was provided in the request. A FHIR Content-Type is required for &quot;CREATE&quot; operation\"/>"));
+
+	}
 
 	@Test
 	public void testEncodeConvertsReferencesToRelative() throws Exception {
@@ -174,6 +202,27 @@ public class SearchDstu2Test {
 
 	@Test
 	public void testSearchByPostWithBodyAndUrlParams() throws Exception {
+		HttpPost httpGet = new HttpPost("http://localhost:" + ourPort + "/Patient/_search?_format=json");
+		StringEntity entity = new StringEntity("searchDateAndList=2001,2002&searchDateAndList=2003,2004", ContentType.APPLICATION_FORM_URLENCODED);
+		httpGet.setEntity(entity);
+
+		CloseableHttpResponse status = ourClient.execute(httpGet);
+		String responseContent = IOUtils.toString(status.getEntity().getContent());
+		IOUtils.closeQuietly(status.getEntity().getContent());
+		ourLog.info(responseContent);
+		assertEquals("searchDateAndList", ourLastMethod);
+		assertEquals(2, ourLastDateAndList.getValuesAsQueryTokens().size());
+		assertEquals(2, ourLastDateAndList.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().size());
+		assertEquals(2, ourLastDateAndList.getValuesAsQueryTokens().get(1).getValuesAsQueryTokens().size());
+		assertEquals("2001", ourLastDateAndList.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValueAsString());
+		assertEquals("2002", ourLastDateAndList.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(1).getValueAsString());
+		assertThat(responseContent, containsString(":\"SYSTEM\""));
+
+	}
+
+	@Test
+	public void testSearchByPostWithBodyAndUrlParamsNoManual() throws Exception {
+		ourServlet.setIgnoreServerParsedRequestParameters(false);
 
 		HttpPost httpGet = new HttpPost("http://localhost:" + ourPort + "/Patient/_search?_format=json");
 		StringEntity entity = new StringEntity("searchDateAndList=2001,2002&searchDateAndList=2003,2004", ContentType.APPLICATION_FORM_URLENCODED);
@@ -371,7 +420,6 @@ public class SearchDstu2Test {
 		assertEquals("searchWhitelist01", ourLastMethod);
 	}
 
-
 	@AfterClass
 	public static void afterClassClearContext() throws Exception {
 		ourServer.stop();
@@ -386,11 +434,11 @@ public class SearchDstu2Test {
 		DummyPatientResourceProvider patientProvider = new DummyPatientResourceProvider();
 
 		ServletHandler proxyHandler = new ServletHandler();
-		RestfulServer servlet = new RestfulServer(ourCtx);
+		ourServlet = new RestfulServer(ourCtx);
-		servlet.setPagingProvider(new FifoMemoryPagingProvider(10));
+		ourServlet.setPagingProvider(new FifoMemoryPagingProvider(10));
 
-		servlet.setResourceProviders(patientProvider);
+		ourServlet.setResourceProviders(patientProvider);
-		ServletHolder servletHolder = new ServletHolder(servlet);
+		ServletHolder servletHolder = new ServletHolder(ourServlet);
 		proxyHandler.addServletWithMapping(servletHolder, "/*");
 		ourServer.setHandler(proxyHandler);
 		ourServer.start();
@@ -402,9 +450,6 @@ public class SearchDstu2Test {
 
 	}
 
-	/**
-	 * Created by dsotnikov on 2/25/2014.
-	 */
 	public static class DummyPatientResourceProvider implements IResourceProvider {
 
 		@Override
@@ -421,6 +466,14 @@ public class SearchDstu2Test {
 		}
 		//@formatter:on
 
+		/**
+		 * For testSearchWithInvalidPostUrl, ok to add a real body later
+		 */
+		@Create
+		public MethodOutcome create(@ResourceParam Patient thePatient) {
+			throw new UnsupportedOperationException();
+		}
+		
 		//@formatter:off
 		@Search()
 		public List<Patient> searchDateAndList(

src/changes/changes.xml

@@ -527,6 +527,12 @@
 				a path of "Appointment.participant.actor" and a target type of "Patient". The search path
 				was being correctly handled, but the target type was being ignored.
 			</action>
+			<action type="add">
+				RestfulServer now manually parses URL parameters instead of relying on the container's
+				parsed parameters. This is useful because many Java servlet containers (e.g. Tomcat, Glassfish)
+				default to ISO-8859-1 encoding for URLs insetad of the UTF-8 encoding specified by
+				FHIR.
+			</action>
 		</release>
 		<release version="1.4" date="2016-02-04">
 			<action type="add">