From 221d7ed5b7fdbb5a191269b2118f5f9be70ae1f7 Mon Sep 17 00:00:00 2001 From: TipzCM Date: Mon, 17 Jan 2022 11:46:32 -0500 Subject: [PATCH] 3274 transactions with full request url should be rejected (#3280) * 3274 throw if url is full url * 3274 will throw on invalid urls * 3274 fixing the tests * 3274 cleanup * anonymize the data * review fixes * 3274 fixed for backward compatibility * added review fixes * fixing tests * some more test fixes * test fixes * 3274 fixing the check to only look for resource at begining * 3274 fixed test * test fixes Co-authored-by: leif stawnyczy --- ...with-full-request-url-will-be-rejected.yml | 4 ++ .../FhirResourceDaoDstu3SearchNoFtTest.java | 6 +- .../jpa/dao/dstu3/FhirSystemDaoDstu3Test.java | 19 +----- ...irResourceDaoR4VersionedReferenceTest.java | 34 ++++++++++ .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 23 ++----- .../dstu3/ResourceProviderDstu3Test.java | 7 +- ...rceProviderDstu3ValueSetVersionedTest.java | 4 +- .../r4/AuthorizationInterceptorJpaR4Test.java | 8 ++- ...rceProviderR4ValueSetNoVerCSNoVerTest.java | 2 +- ...ourceProviderR4ValueSetVerCSNoVerTest.java | 2 +- ...esourceProviderR4ValueSetVerCSVerTest.java | 4 +- .../r5/ResourceProviderR5ValueSetTest.java | 2 +- ...sourceProviderR5ValueSetVersionedTest.java | 4 +- .../transaction-with-full-request-url.json | 16 +++++ ...tion-with-preceding-slash-request-url.json | 16 +++++ .../jpa/dao/BaseTransactionProcessor.java | 68 ++++++++++++++++++- .../parser/jsonlike/JsonLikeParserTest.java | 3 +- 17 files changed, 161 insertions(+), 61 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3274-transactions-with-full-request-url-will-be-rejected.yml create mode 100644 hapi-fhir-jpaserver-base/src/test/resources/transaction-bundles/transaction-with-full-request-url.json create mode 100644 hapi-fhir-jpaserver-base/src/test/resources/transaction-bundles/transaction-with-preceding-slash-request-url.json diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3274-transactions-with-full-request-url-will-be-rejected.yml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3274-transactions-with-full-request-url-will-be-rejected.yml new file mode 100644 index 00000000000..772a9e11f4e --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3274-transactions-with-full-request-url-will-be-rejected.yml @@ -0,0 +1,4 @@ +--- +issue: 3274 +title: "Transactions with entries that have request.url values that are fully + qualified urls will now be rejected." diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java index beff9c92c6f..274ce5c472c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java @@ -42,6 +42,7 @@ import ca.uhn.fhir.rest.param.TokenParamModifier; import ca.uhn.fhir.rest.param.UriParam; import ca.uhn.fhir.rest.param.UriParamQualifierEnum; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import ca.uhn.fhir.util.UrlUtil; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.hl7.fhir.dstu3.model.Appointment; @@ -94,7 +95,6 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.test.context.TestPropertySource; import org.springframework.transaction.TransactionDefinition; @@ -110,7 +110,6 @@ import java.util.Date; import java.util.List; import java.util.Set; import java.util.TreeSet; -import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; @@ -272,7 +271,8 @@ public class FhirResourceDaoDstu3SearchNoFtTest extends BaseJpaDstu3Test { Set allIds = new TreeSet<>(); for (BundleEntryComponent nextEntry : inputBundle.getEntry()) { nextEntry.getRequest().setMethod(HTTPVerb.PUT); - nextEntry.getRequest().setUrl(nextEntry.getResource().getId()); + UrlUtil.UrlParts parts = UrlUtil.parseUrl(nextEntry.getResource().getId()); + nextEntry.getRequest().setUrl(parts.getResourceType() + "/" + parts.getResourceId()); allIds.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java index a97890cbe02..2b387b05312 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java @@ -1108,21 +1108,6 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest { } } - @Test - public void testTransactionCreateWithPutUsingAbsoluteUrl() { - String methodName = "testTransactionCreateWithPutUsingAbsoluteUrl"; - Bundle request = new Bundle(); - request.setType(BundleType.TRANSACTION); - - Patient p = new Patient(); - p.addIdentifier().setSystem("urn:system").setValue(methodName); - request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.PUT).setUrl("http://localhost/server/base/Patient/" + methodName); - - mySystemDao.transaction(mySrd, request); - - myPatientDao.read(new IdType("Patient/" + methodName), mySrd); - } - @Test public void testTransactionCreateWithPutUsingUrl() { String methodName = "testTransactionCreateWithPutUsingUrl"; @@ -2685,7 +2670,7 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest { Patient p = new Patient(); p.setActive(true); p.setId(IdType.newRandomUuid()); - request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl(p.getId()); + request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl("Patient/" + p.getId()); Observation o = new Observation(); o.getCode().setText("Some Observation"); @@ -2765,7 +2750,7 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest { Patient p = new Patient(); p.setActive(true); p.setId(IdType.newRandomUuid()); - request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl(p.getId()); + request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST); Observation o = new Observation(); o.getCode().setText("Some Observation"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java index 0975df2548e..2722cb4238c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java @@ -8,6 +8,8 @@ import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.util.BundleBuilder; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -27,6 +29,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import java.io.IOException; import java.io.InputStreamReader; import java.util.Arrays; import java.util.Date; @@ -43,6 +46,7 @@ import static org.hamcrest.Matchers.matchesPattern; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4VersionedReferenceTest.class); @@ -832,6 +836,36 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { assertThat(versionedPatientReference, is(equalTo("Patient/RED/_history/1"))); } + @Test + public void bundleTransaction_withRequestUrlNotRelativePath_doesNotProcess() throws IOException { + Bundle bundle = loadResourceFromClasspath(Bundle.class, "/transaction-bundles/transaction-with-full-request-url.json"); + + try { + // test + mySystemDao.transaction(new ServletRequestDetails(), + bundle); + fail("We expect invalid full urls to fail"); + } catch (InvalidRequestException ex) { + Assertions.assertTrue(ex.getMessage().contains("Unable to perform POST, URL provided is invalid:")); + } + } + + @Test + public void bundleTransaction_withRequestURLWithPrecedingSlash_processesAsExpected() throws IOException { + Bundle bundle = loadResourceFromClasspath(Bundle.class, "/transaction-bundles/transaction-with-preceding-slash-request-url.json"); + + // test + Bundle outcome = mySystemDao.transaction(new SystemRequestDetails(), + bundle); + + // verify it was created + Assertions.assertEquals(1, outcome.getEntry().size()); + IdType idType = new IdType(bundle.getEntry().get(0) + .getResource().getId()); + // the bundle above contains an observation, so we'll verify it was created here + Observation obs = myObservationDao.read(idType); + Assertions.assertNotNull(obs); + } @Test @DisplayName("Bundle transaction with AutoVersionReferenceAtPath on and with existing Patient resource should create") diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java index 3b1e7693b1e..deaed2b3ccc 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java @@ -2094,31 +2094,16 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { request.addEntry() .setResource(o) - .getRequest().setUrl("A").setMethod(HTTPVerb.PUT); + .getRequest().setUrl("Observation").setMethod(HTTPVerb.PUT); try { mySystemDao.transaction(mySrd, request); fail(); } catch (InvalidRequestException e) { - assertEquals("Invalid match URL[Observation?A] - URL has no search parameters", e.getMessage()); + assertEquals("Invalid match URL[Observation?Observation] - URL has no search parameters", e.getMessage()); } } - @Test - public void testTransactionCreateWithPutUsingAbsoluteUrl() { - String methodName = "testTransactionCreateWithPutUsingAbsoluteUrl"; - Bundle request = new Bundle(); - request.setType(BundleType.TRANSACTION); - - Patient p = new Patient(); - p.addIdentifier().setSystem("urn:system").setValue(methodName); - request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.PUT).setUrl("http://localhost/server/base/Patient/" + methodName); - - mySystemDao.transaction(mySrd, request); - - myPatientDao.read(new IdType("Patient/" + methodName), mySrd); - } - @Test public void testTransactionCreateWithPutUsingUrl() { String methodName = "testTransactionCreateWithPutUsingUrl"; @@ -4084,7 +4069,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { Patient p = new Patient(); p.setActive(true); p.setId(IdType.newRandomUuid()); - request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl(p.getId()); + request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl("Patient/" + p.getId()); Observation o = new Observation(); o.getCode().setText("Some Observation"); @@ -4164,7 +4149,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { Patient p = new Patient(); p.setActive(true); p.setId(IdType.newRandomUuid()); - request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl(p.getId()); + request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl("Patient/" + p.getId()); Observation o = new Observation(); o.getCode().setText("Some Observation"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index fe8c5d3d576..9d1eef6e44e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -5,7 +5,6 @@ import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.provider.r4.ResourceProviderR4Test; import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; -import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.model.api.TemporalPrecisionEnum; import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.model.primitive.UriDt; @@ -15,7 +14,6 @@ import ca.uhn.fhir.rest.api.CacheControlDirective; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.SummaryEnum; -import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.client.api.IClientInterceptor; import ca.uhn.fhir.rest.client.api.IGenericClient; import ca.uhn.fhir.rest.client.api.IHttpRequest; @@ -28,7 +26,6 @@ import ca.uhn.fhir.rest.param.ParamPrefixEnum; import ca.uhn.fhir.rest.param.StringAndListParam; import ca.uhn.fhir.rest.param.StringOrListParam; import ca.uhn.fhir.rest.param.StringParam; -import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; @@ -122,7 +119,6 @@ import org.hl7.fhir.dstu3.model.Task; import org.hl7.fhir.dstu3.model.UnsignedIntType; import org.hl7.fhir.dstu3.model.ValueSet; import org.hl7.fhir.dstu3.model.codesystems.DeviceStatus; -import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.junit.jupiter.api.AfterEach; @@ -1818,7 +1814,8 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { Set allIds = new TreeSet(); for (BundleEntryComponent nextEntry : inputBundle.getEntry()) { nextEntry.getRequest().setMethod(HTTPVerb.PUT); - nextEntry.getRequest().setUrl(nextEntry.getResource().getId()); + UrlUtil.UrlParts parts = UrlUtil.parseUrl(nextEntry.getResource().getId()); + nextEntry.getRequest().setUrl(parts.getResourceType() + "/" + parts.getResourceId()); allIds.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetVersionedTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetVersionedTest.java index 8d174d880f6..b2e509b8f2d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetVersionedTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetVersionedTest.java @@ -1040,7 +1040,7 @@ public class ResourceProviderDstu3ValueSetVersionedTest extends BaseResourceProv .setResource(updatedValueSet_v1) .getRequest() .setMethod(Bundle.HTTPVerb.PUT) - .setUrl(url); + .setUrl("ValueSet/" + updatedValueSet_v1.getIdElement().getIdPart()); ourLog.info("Transaction Bundle:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle)); ourClient.transaction().withBundle(bundle).execute(); @@ -1062,7 +1062,7 @@ public class ResourceProviderDstu3ValueSetVersionedTest extends BaseResourceProv .setResource(updatedValueSet_v2) .getRequest() .setMethod(Bundle.HTTPVerb.PUT) - .setUrl(url); + .setUrl("ValueSet/" + updatedValueSet_v2.getIdElement().getIdPart()); ourLog.info("Transaction Bundle:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle)); ourClient.transaction().withBundle(bundle).execute(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java index c67b198eaeb..6a826062982 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/AuthorizationInterceptorJpaR4Test.java @@ -1163,7 +1163,6 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes */ @Test public void testInstanceRuleOkForResourceWithNoId() { - ourRestServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { @@ -1183,8 +1182,11 @@ public class AuthorizationInterceptorJpaR4Test extends BaseResourceProviderR4Tes Patient p = new Patient(); p.setActive(true); - p.setId(IdType.newRandomUuid()); - request.addEntry().setResource(p).getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl(p.getId()); + p.setId("123"); + request.addEntry().setResource(p).getRequest().setMethod(Bundle.HTTPVerb.POST) + .setUrl( + "Patient/"+ + p.getId()); Observation o = new Observation(); o.getCode().setText("Some Observation"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetNoVerCSNoVerTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetNoVerCSNoVerTest.java index 42502bfc0c9..9945f8829f5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetNoVerCSNoVerTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetNoVerCSNoVerTest.java @@ -886,7 +886,7 @@ public class ResourceProviderR4ValueSetNoVerCSNoVerTest extends BaseResourceProv .setResource(updatedValueSet) .getRequest() .setMethod(Bundle.HTTPVerb.PUT) - .setUrl(url); + .setUrl(myExtensionalVsId.getValueAsString()); ourLog.info("Transaction Bundle:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle)); myClient.transaction().withBundle(bundle).execute(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSNoVerTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSNoVerTest.java index a9ef0ac1cb5..efc920fe139 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSNoVerTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSNoVerTest.java @@ -776,7 +776,7 @@ public class ResourceProviderR4ValueSetVerCSNoVerTest extends BaseResourceProvid .setResource(updatedValueSet) .getRequest() .setMethod(Bundle.HTTPVerb.PUT) - .setUrl(url); + .setUrl(myExtensionalVsId.getValueAsString()); ourLog.info("Transaction Bundle:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle)); myClient.transaction().withBundle(bundle).execute(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSVerTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSVerTest.java index e9e2300438a..e073edffe1a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSVerTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4ValueSetVerCSVerTest.java @@ -995,7 +995,7 @@ public class ResourceProviderR4ValueSetVerCSVerTest extends BaseResourceProvider .setResource(updatedValueSet_v1) .getRequest() .setMethod(Bundle.HTTPVerb.PUT) - .setUrl(url); + .setUrl(myExtensionalVsId_v1.getValueAsString()); ourLog.info("Transaction Bundle:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle)); myClient.transaction().withBundle(bundle).execute(); @@ -1017,7 +1017,7 @@ public class ResourceProviderR4ValueSetVerCSVerTest extends BaseResourceProvider .setResource(updatedValueSet_v2) .getRequest() .setMethod(Bundle.HTTPVerb.PUT) - .setUrl(url); + .setUrl(myExtensionalVsId_v2.getValueAsString()); ourLog.info("Transaction Bundle:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle)); myClient.transaction().withBundle(bundle).execute(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java index 85eb95fefcd..4c9c6a8964d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetTest.java @@ -1094,7 +1094,7 @@ public class ResourceProviderR5ValueSetTest extends BaseResourceProviderR5Test { .setResource(updatedValueSet) .getRequest() .setMethod(Bundle.HTTPVerb.PUT) - .setUrl(url); + .setUrl(myExtensionalVsId.getValueAsString()); ourLog.info("Transaction Bundle:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle)); myClient.transaction().withBundle(bundle).execute(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetVersionedTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetVersionedTest.java index aaff4459182..568286cab8e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetVersionedTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5ValueSetVersionedTest.java @@ -1025,7 +1025,7 @@ public class ResourceProviderR5ValueSetVersionedTest extends BaseResourceProvide .setResource(updatedValueSet_v1) .getRequest() .setMethod(Bundle.HTTPVerb.PUT) - .setUrl(url); + .setUrl(myExtensionalVsId_v1.getValueAsString()); ourLog.info("Transaction Bundle:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle)); myClient.transaction().withBundle(bundle).execute(); @@ -1047,7 +1047,7 @@ public class ResourceProviderR5ValueSetVersionedTest extends BaseResourceProvide .setResource(updatedValueSet_v2) .getRequest() .setMethod(Bundle.HTTPVerb.PUT) - .setUrl(url); + .setUrl(myExtensionalVsId_v2.getValueAsString()); ourLog.info("Transaction Bundle:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle)); myClient.transaction().withBundle(bundle).execute(); diff --git a/hapi-fhir-jpaserver-base/src/test/resources/transaction-bundles/transaction-with-full-request-url.json b/hapi-fhir-jpaserver-base/src/test/resources/transaction-bundles/transaction-with-full-request-url.json new file mode 100644 index 00000000000..f72d93e6834 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/resources/transaction-bundles/transaction-with-full-request-url.json @@ -0,0 +1,16 @@ +{ + "resourceType": "Bundle", + "type": "transaction", + "entry": [ + { + "request": { + "method": "POST", + "url": "http://localhost:8000/fhir/Goal" + }, + "resource": { + "resourceType": "Goal", + "id": "4bebccef-de86-4b49-9ca8-7567bffcc6f5" + } + } + ] +} diff --git a/hapi-fhir-jpaserver-base/src/test/resources/transaction-bundles/transaction-with-preceding-slash-request-url.json b/hapi-fhir-jpaserver-base/src/test/resources/transaction-bundles/transaction-with-preceding-slash-request-url.json new file mode 100644 index 00000000000..68f86662588 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/resources/transaction-bundles/transaction-with-preceding-slash-request-url.json @@ -0,0 +1,16 @@ +{ + "resourceType": "Bundle", + "type": "transaction", + "entry": [ + { + "request": { + "method": "POST", + "url": "/Observation" + }, + "resource": { + "resourceType": "Observation", + "id": "4bebccef-de86-4b49-9ca8-7567bffcc6f5" + } + } + ] +} diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java index 37959cd043c..cdd94a7d98b 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java @@ -894,6 +894,16 @@ public abstract class BaseTransactionProcessor { switch (verb) { case "POST": { // CREATE + /* + * To preserve existing functionality, + * we will only verify that the request url is + * valid if it's provided at all. + * Otherwise, we'll ignore it + */ + String url = myVersionAdapter.getEntryRequestUrl(nextReqEntry); + if (isNotBlank(url)) { + extractAndVerifyTransactionUrlForEntry(nextReqEntry, verb); + } validateResourcePresent(res, order, verb); @SuppressWarnings("rawtypes") IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass()); @@ -920,7 +930,7 @@ public abstract class BaseTransactionProcessor { } case "DELETE": { // DELETE - String url = extractTransactionUrlOrThrowException(nextReqEntry, verb); + String url = extractAndVerifyTransactionUrlForEntry(nextReqEntry, verb); UrlUtil.UrlParts parts = UrlUtil.parseUrl(url); IFhirResourceDao dao = toDao(parts, verb, url); int status = Constants.STATUS_HTTP_204_NO_CONTENT; @@ -959,7 +969,7 @@ public abstract class BaseTransactionProcessor { @SuppressWarnings("rawtypes") IFhirResourceDao resourceDao = getDaoOrThrowException(res.getClass()); - String url = extractTransactionUrlOrThrowException(nextReqEntry, verb); + String url = extractAndVerifyTransactionUrlForEntry(nextReqEntry, verb); DaoMethodOutcome outcome; UrlUtil.UrlParts parts = UrlUtil.parseUrl(url); @@ -1003,7 +1013,7 @@ public abstract class BaseTransactionProcessor { // PATCH validateResourcePresent(res, order, verb); - String url = extractTransactionUrlOrThrowException(nextReqEntry, verb); + String url = extractAndVerifyTransactionUrlForEntry(nextReqEntry, verb); UrlUtil.UrlParts parts = UrlUtil.parseUrl(url); String matchUrl = toMatchUrl(nextReqEntry); @@ -1543,6 +1553,58 @@ public abstract class BaseTransactionProcessor { myContext = theContext; } + /** + * Extracts the transaction url from the entry and verifies it's: + * * not null or bloack + * * is a relative url matching the resourceType it is about + * + * Returns the transaction url (or throws an InvalidRequestException if url is not valid) + */ + private String extractAndVerifyTransactionUrlForEntry(IBase theEntry, String theVerb) { + String url = extractTransactionUrlOrThrowException(theEntry, theVerb); + + if (!isValidResourceTypeUrl(url)) { + ourLog.debug("Invalid url. Should begin with a resource type: {}", url); + String msg = myContext.getLocalizer().getMessage(BaseStorageDao.class, "transactionInvalidUrl", theVerb, url); + throw new InvalidRequestException(msg); + } + return url; + } + + /** + * Returns true if the provided url is a valid entry request.url. + * + * This means: + * a) not an absolute url (does not start with http/https) + * b) starts with either a ResourceType or /ResourceType + */ + private boolean isValidResourceTypeUrl(@Nonnull String theUrl) { + if (UrlUtil.isAbsolute(theUrl)) { + return false; + } else { + int queryStringIndex = theUrl.indexOf("?"); + String url; + if (queryStringIndex > 0) { + url = theUrl.substring(0, theUrl.indexOf("?")); + } else { + url = theUrl; + } + String[] parts; + if (url.startsWith("/")) { + parts = url.substring(1).split("/"); + } else { + parts = url.split("/"); + } + Set allResourceTypes = myContext.getResourceTypes(); + + return allResourceTypes.contains(parts[0]); + } + } + + /** + * Extracts the transaction url from the entry and verifies that it is not null/blank + * and returns it + */ private String extractTransactionUrlOrThrowException(IBase nextEntry, String verb) { String url = myVersionAdapter.getEntryRequestUrl(nextEntry); if (isBlank(url)) { diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/jsonlike/JsonLikeParserTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/jsonlike/JsonLikeParserTest.java index 12a700f128d..f268c474f1e 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/jsonlike/JsonLikeParserTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/jsonlike/JsonLikeParserTest.java @@ -32,7 +32,6 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.Stack; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -99,7 +98,7 @@ public class JsonLikeParserTest { assertEquals("x1", extension.get("url"), "Expecting '/extension[]/url' = 'x1'; found '"+extension.get("url")+"'"); } - + /** * Repeat the "View" tests with custom JSON-Like structure */