Allow plus symbol in email address match url (#2224)

* Allow plus symbol in email address match url

* Add changelog

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

@@ -10,6 +10,7 @@ import com.google.common.escape.Escaper;
 import com.google.common.net.PercentEscaper;
 import org.apache.http.NameValuePair;
 import org.apache.http.client.utils.URLEncodedUtils;
+import org.apache.http.message.BasicNameValuePair;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
 
 import javax.annotation.Nonnull;
@@ -511,6 +512,18 @@ public class UrlUtil {
 		}
 
 		parameters = URLEncodedUtils.parse((matchUrl), Constants.CHARSET_UTF8, '&');
+
+		// One issue that has happened before is people putting a "+" sign into an email address in a match URL
+		// and having that turn into a " ". Since spaces are never appropriate for email addresses, let's just
+		// assume they really meant "+".
+		for (int i = 0; i < parameters.size(); i++) {
+			NameValuePair next = parameters.get(i);
+			if (next.getName().equals("email") && next.getValue().contains(" ")) {
+				BasicNameValuePair newPair = new BasicNameValuePair(next.getName(), next.getValue().replace(' ', '+'));
+				parameters.set(i, newPair);
+			}
+		}
+
 		return parameters;
 	}
 }

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2224-allow-plus-in-match-url.yaml

@@ -0,0 +1,6 @@
+---
+type: add
+issue: 2234
+title: "When performing a conditional create/update containing the email search parameter, any `+` characters will now be interpreted
+   as actually being a plus symbol instead of being unescaped into a space character. This is technically a deviation from how URLs should
+   be parsed, but allows for a sensible behaviour in a spot where no spaces are allowed."

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

@@ -268,7 +268,7 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao<Bundle, MetaDt> {
 			requestDetails.setParameters(new HashMap<String, String[]>());
 			if (qIndex != -1) {
 				String params = url.substring(qIndex);
-				List<NameValuePair> parameters = myMatchUrlService.translateMatchUrl(params);
+                List<NameValuePair> parameters = UrlUtil.translateMatchUrl(params);
 				for (NameValuePair next : parameters) {
 					paramValues.put(next.getName(), next.getValue());
 				}

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java

@@ -19,6 +19,7 @@ import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.CanonicalType;
 import org.hl7.fhir.r4.model.CodeSystem;
 import org.hl7.fhir.r4.model.Coding;
+import org.hl7.fhir.r4.model.ContactPoint;
 import org.hl7.fhir.r4.model.IdType;
 import org.hl7.fhir.r4.model.InstantType;
 import org.hl7.fhir.r4.model.Meta;
@@ -112,6 +113,59 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
 
 	}
 
+	@Test
+	public void testUpdateConditionalOnEmailParameterWithPlusSymbol() {
+		IBundleProvider outcome;
+
+		myCaptureQueriesListener.clear();
+		Patient p = new Patient();
+		p.addTelecom()
+			.setSystem(ContactPoint.ContactPointSystem.EMAIL)
+			.setValue("help-im+a@bug.com");
+		myPatientDao.update(p, "Patient?email=help-im+a@bug.com");
+		myCaptureQueriesListener.logSelectQueries();
+
+		outcome = myPatientDao.search(SearchParameterMap.newSynchronous());
+		assertEquals(1, outcome.sizeOrThrowNpe());
+
+		p = new Patient();
+		p.addTelecom()
+			.setSystem(ContactPoint.ContactPointSystem.EMAIL)
+			.setValue("help-im+a@bug.com");
+		myPatientDao.update(p, "Patient?email=help-im+a@bug.com");
+
+		outcome = myPatientDao.search(SearchParameterMap.newSynchronous());
+		assertEquals(1, outcome.sizeOrThrowNpe());
+
+	}
+
+	@Test
+	public void testUpdateConditionalOnEmailParameterWithPlusSymbolCorrectlyEscaped() {
+		IBundleProvider outcome;
+
+		myCaptureQueriesListener.clear();
+		Patient p = new Patient();
+		p.addTelecom()
+			.setSystem(ContactPoint.ContactPointSystem.EMAIL)
+			.setValue("help-im+a@bug.com");
+		myPatientDao.update(p, "Patient?email=help-im%2Ba@bug.com");
+		myCaptureQueriesListener.logSelectQueries();
+
+		outcome = myPatientDao.search(SearchParameterMap.newSynchronous());
+		assertEquals(1, outcome.sizeOrThrowNpe());
+
+		p = new Patient();
+		p.addTelecom()
+			.setSystem(ContactPoint.ContactPointSystem.EMAIL)
+			.setValue("help-im+a@bug.com");
+		myPatientDao.update(p, "Patient?email=help-im%2Ba@bug.com");
+
+		outcome = myPatientDao.search(SearchParameterMap.newSynchronous());
+		assertEquals(1, outcome.sizeOrThrowNpe());
+
+	}
+
+
 	/**
 	 * Just in case any hash values are missing
 	 */

hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java

@@ -54,7 +54,7 @@ public class MatchUrlService {
 
 	public SearchParameterMap translateMatchUrl(String theMatchUrl, RuntimeResourceDefinition theResourceDefinition, Flag... theFlags) {
 		SearchParameterMap paramMap = new SearchParameterMap();
-		List<NameValuePair> parameters = translateMatchUrl(theMatchUrl);
+		List<NameValuePair> parameters = UrlUtil.translateMatchUrl(theMatchUrl);
 
 		ArrayListMultimap<String, QualifiedParamList> nameToParamLists = ArrayListMultimap.create();
 		for (NameValuePair next : parameters) {
@@ -147,10 +147,6 @@ public class MatchUrlService {
 		return paramMap;
 	}
 
-	public List<NameValuePair> translateMatchUrl(String theMatchUrl) {
-		return UrlUtil.translateMatchUrl(theMatchUrl);
-	}
-
 	private IQueryParameterAnd<?> newInstanceAnd(String theParamType) {
 		Class<? extends IQueryParameterAnd<?>> clazz = ResourceMetaParams.RESOURCE_META_AND_PARAMS.get(theParamType);
 		return ReflectionUtil.newInstance(clazz);