From 66f428d3563f8f3c87f50d7ffab280e8c70de4d6 Mon Sep 17 00:00:00 2001 From: Steve Corbett <137920358+steve-corbett-smilecdr@users.noreply.github.com> Date: Thu, 20 Jul 2023 14:14:28 -0600 Subject: [PATCH] 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 --- .../main/java/ca/uhn/fhir/util/UrlUtil.java | 4 ++-- .../5103-fix-handling-of-spaces-in-urls.yaml | 8 +++++++ .../rest_operations_operations.md | 23 +++++++++++++++++++ ...FhirResourceDaoR4ConcurrentCreateTest.java | 2 +- .../dao/r4/FhirResourceDaoR4CreateTest.java | 8 +++---- .../dao/r4/FhirResourceDaoR4DeleteTest.java | 2 +- .../dao/r4/FhirResourceDaoR4UpdateTest.java | 2 +- .../java/ca/uhn/fhir/util/UrlUtilTest.java | 12 +++++++--- 8 files changed, 49 insertions(+), 12 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5103-fix-handling-of-spaces-in-urls.yaml diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java index 9d35a50bf18..19c54ebd899 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java +++ b/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 diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5103-fix-handling-of-spaces-in-urls.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5103-fix-handling-of-spaces-in-urls.yaml new file mode 100644 index 00000000000..92a6a891987 --- /dev/null +++ b/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. +" diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_plain/rest_operations_operations.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_plain/rest_operations_operations.md index f0f7c2a7966..72a627af441 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_plain/rest_operations_operations.md +++ b/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. diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ConcurrentCreateTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ConcurrentCreateTest.java index e6b359306af..9e480fd81c7 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ConcurrentCreateTest.java +++ b/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(); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4CreateTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4CreateTest.java index 28689197a8e..1fe09812342 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4CreateTest.java +++ b/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); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4DeleteTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4DeleteTest.java index 2a4a3f1040c..77dee5a0a24 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4DeleteTest.java +++ b/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(); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java index 3dcd8508609..48a9de4e516 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java +++ b/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()); diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/UrlUtilTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/UrlUtilTest.java index 7da512e59d1..f4ef489e2e4 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/UrlUtilTest.java +++ b/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