5103 - Adjust our handling of space escape characters in URLs (#5104)

* 5103 - Adjust our handling of space escape characters in URLs

* 5103 - Fix formatting

* 5103 - Adjust URL escaping and update test expectations

* 5103 - additional test for values with literal plus signs

* 5103 add changelog for this issue

* 5103 - documentation change to describe proper URL escaping

* 5103 - Minor documentation addition

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

@@ -577,8 +577,8 @@ public class UrlUtil {
 			matchUrl = matchUrl.substring(questionMarkIndex + 1);
 		}
 
-		final String[] searchList = new String[] {"+", "|", "=>=", "=<=", "=>", "=<"};
-		final String[] replacementList = new String[] {"%2B", "%7C", "=%3E%3D", "=%3C%3D", "=%3E", "=%3C"};
+		final String[] searchList = new String[] {"|", "=>=", "=<=", "=>", "=<"};
+		final String[] replacementList = new String[] {"%7C", "=%3E%3D", "=%3C%3D", "=%3E", "=%3C"};
 		matchUrl = StringUtils.replaceEach(matchUrl, searchList, replacementList);
 		if (matchUrl.contains(" ")) {
 			throw new InvalidRequestException(Msg.code(1744) + "Failed to parse match URL[" + theMatchUrl

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5103-fix-handling-of-spaces-in-urls.yaml

@@ -0,0 +1,8 @@
+---
+type: fix
+issue: 5103
+title: "
+      Remove + from the list of characters that are escaped automatically
+      since + is already an escaped character (representing a space)
+      in the query part of URLs.
+"

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_plain/rest_operations_operations.md

@@ -78,6 +78,29 @@ For example:
 {{snippet:classpath:/ca/uhn/hapi/fhir/docs/ServerOperations.java|searchParamAdvanced}}
 ``` 
 
+If the string value to be searched contains a space character, you should encode it with a `+` sign or with `%20`, as in the following examples with an Organization named "Acme Corporation":
+
+```url
+http://fhir.example.com/Organization?name=Acme+Corporation
+http://fhir.example.com/Organization?name=Acme%20Corporation
+```
+
+If the string value to be searched contains a literal `+` character, you should escape it with `%2B`, as in the following example with an Organization named "H+K":
+
+```url
+http://fhir.example.com/Organization?name=H%2BK
+```
+
+Certain strings are automatically escaped when the FHIR server parses URLs:
+
+```url
+"|"   -> "%7C"
+"=>=" -> "=%3E%3D"
+"=<=" -> "=%3C%3D"
+"=>"  -> "=%3E"
+"=<"  -> "=%3C"
+```
+
 # Returning Multiple OUT Parameters
 
 In all of the Operation examples above, the return type specified for the operation is a single Resource instance. This is a common pattern in FHIR defined operations. However, it is also possible for an extended operation to be defined with multiple and/or repeating OUT parameters. In this case, you can return a [Parameters](/hapi-fhir/apidocs/hapi-fhir-structures-r4/org/hl7/fhir/r4/model/Parameters.html) resource directly.

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

@@ -196,7 +196,7 @@ public class FhirResourceDaoR4ConcurrentCreateTest extends BaseJpaR4Test {
 
 			try {
 				ourLog.info("Creating resource");
-				DaoMethodOutcome outcome = myObservationDao.create(obs, "identifier=20210427133226.444+0800", requestDetails);
+				DaoMethodOutcome outcome = myObservationDao.create(obs, "identifier=20210427133226.444%2B0800", requestDetails);
 			} catch (Throwable t) {
 				ourLog.info("create threw an exception {}", t.getMessage());
 				fail();

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

@@ -347,14 +347,14 @@ public class FhirResourceDaoR4CreateTest extends BaseJpaR4Test {
 	public void testConditionalCreateWithPlusInUrl() {
 		Observation obs = new Observation();
 		obs.addIdentifier().setValue("20210427133226.444+0800");
-		DaoMethodOutcome outcome = myObservationDao.create(obs, "identifier=20210427133226.444+0800", new SystemRequestDetails());
+		DaoMethodOutcome outcome = myObservationDao.create(obs, "identifier=20210427133226.444%2B0800", new SystemRequestDetails());
 		assertTrue(outcome.getCreated());
 
 		logAllTokenIndexes();
 		myCaptureQueriesListener.clear();
 		obs = new Observation();
 		obs.addIdentifier().setValue("20210427133226.444+0800");
-		outcome = myObservationDao.create(obs, "identifier=20210427133226.444+0800", new SystemRequestDetails());
+		outcome = myObservationDao.create(obs, "identifier=20210427133226.444%2B0800", new SystemRequestDetails());
 		myCaptureQueriesListener.logSelectQueriesForCurrentThread();
 		assertFalse(outcome.getCreated());
 	}
@@ -403,7 +403,7 @@ public class FhirResourceDaoR4CreateTest extends BaseJpaR4Test {
 	public void testCreateResource_withConditionalCreate_willAddSearchUrlEntity(){
 		// given
 		String identifierCode = "20210427133226.4440+800";
-		String matchUrl = "identifier=" + identifierCode;
+		String matchUrl = "identifier=" + identifierCode.replace("+", "%2B");
 		Observation obs = new Observation();
 		obs.addIdentifier().setValue(identifierCode);
 		// when
@@ -411,7 +411,7 @@ public class FhirResourceDaoR4CreateTest extends BaseJpaR4Test {
 
 		// then
 		Long expectedResId = outcome.getId().getIdPartAsLong();
-		String expectedNormalizedMatchUrl = obs.fhirType() + "?" + StringUtils.replace(matchUrl, "+", "%2B");
+		String expectedNormalizedMatchUrl = obs.fhirType() + "?" + matchUrl;
 
 		assertTrue(outcome.getCreated());
 		ResourceSearchUrlEntity searchUrlEntity = myResourceSearchUrlDao.findAll().get(0);

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

@@ -223,7 +223,7 @@ public class FhirResourceDaoR4DeleteTest extends BaseJpaR4Test {
 	@Test
 	public void testDeleteResourceCreatedWithConditionalUrl_willRemoveEntryInSearchUrlTable() {
 		String identifierCode = "20210427133226.4440+800";
-		String matchUrl = "identifier=20210427133226.4440+800";
+		String matchUrl = "identifier=20210427133226.4440%2B800";
 		Observation obs = new Observation();
 		obs.addIdentifier().setValue(identifierCode);
 		IIdType firstObservationId = myObservationDao.create(obs, matchUrl, new SystemRequestDetails()).getId();

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

@@ -657,7 +657,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
 	@Test
 	public void testUpdateResourceCreatedWithConditionalUrl_willRemoveEntryInSearchUrlTable(){
 		String identifierCode = "20210427133226.4440+800";
-		String matchUrl = "identifier=20210427133226.4440+800";
+		String matchUrl = "identifier=20210427133226.4440%2B800";
 		Observation obs = new Observation();
 		obs.addIdentifier().setValue(identifierCode);
 		myObservationDao.create(obs, matchUrl, new SystemRequestDetails());

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/UrlUtilTest.java

@@ -111,14 +111,20 @@ public class UrlUtilTest {
 
 	@Test
 	public void testTranslateMatchUrl_UrlWithSpaces() {
-		// Real space
+		// %20 is an encoded space character
 		assertThat(UrlUtil.translateMatchUrl("Observation?names=homer%20simpson"),
 			containsInAnyOrder(new BasicNameValuePair("names", "homer simpson")));
 
-		// Treat + as an actual + and not a space
+		// + is also an encoded space character
 		assertThat(UrlUtil.translateMatchUrl("Observation?names=homer+simpson"),
-			containsInAnyOrder(new BasicNameValuePair("names", "homer+simpson")));
+			containsInAnyOrder(new BasicNameValuePair("names", "homer simpson")));
+	}
 
+	@Test
+	public void testTranslateMatchUrl_UrlWithPlusSign() {
+		// %2B is an encoded plus sign
+		assertThat(UrlUtil.translateMatchUrl("Observation?names=homer%2Bsimpson"),
+			containsInAnyOrder(new BasicNameValuePair("names", "homer+simpson")));
 	}
 
 	@Test