From 3f424142875973b974d49b754d93a7a358950a36 Mon Sep 17 00:00:00 2001
From: James Agnew
Date: Wed, 26 Jan 2022 09:16:28 -0500
Subject: [PATCH] Validate include statements for JPA (#3331)
* Validate include statements for JPA
* Add changelog
* Optmize test fix
---
.../java/ca/uhn/fhir/model/api/Include.java | 69 ++---
.../java/ca/uhn/fhir/rest/api/Constants.java | 1 +
.../ca/uhn/fhir/i18n/hapi-messages.properties | 1 +
.../5_7_0/3331-validate-includes-for-jpa.yaml | 5 +
.../jpa/search/SearchCoordinatorSvcImpl.java | 240 +++++++++++-------
.../FhirResourceDaoR4SearchIncludeTest.java | 7 +-
.../r4/FhirResourceDaoR4SearchNoFtTest.java | 81 +++++-
.../fhir/rest/api/server/IBundleProvider.java | 2 +-
.../ca/uhn/fhir/rest/server/IncludeTest.java | 19 +-
9 files changed, 275 insertions(+), 150 deletions(-)
create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3331-validate-includes-for-jpa.yaml
diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Include.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Include.java
index 899975bc2ee..e09ae1d4c5b 100644
--- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Include.java
+++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/Include.java
@@ -1,5 +1,6 @@
package ca.uhn.fhir.model.api;
+import static org.apache.commons.lang3.StringUtils.defaultString;
import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
@@ -34,6 +35,9 @@ import org.apache.commons.lang3.builder.ToStringBuilder;
* equality. Prior to HAPI 1.2 (and FHIR DSTU2) the recurse property did not exist, so this may merit consideration when
* upgrading servers.
*
+ *
+ * Note on thrwead safety: This class is not thread safe.
+ *
*/
public class Include implements Serializable {
@@ -42,6 +46,9 @@ public class Include implements Serializable {
private final boolean myImmutable;
private boolean myIterate;
private String myValue;
+ private String myParamType;
+ private String myParamName;
+ private String myParamTargetType;
/**
* Constructor for non-recursive include
@@ -50,8 +57,7 @@ public class Include implements Serializable {
* The _include
value, e.g. "Patient:name"
*/
public Include(String theValue) {
- myValue = theValue;
- myImmutable = false;
+ this(theValue, false);
}
/**
@@ -63,9 +69,7 @@ public class Include implements Serializable {
* Should the include recurse
*/
public Include(String theValue, boolean theIterate) {
- myValue = theValue;
- myIterate = theIterate;
- myImmutable = false;
+ this(theValue, theIterate, false);
}
/**
@@ -77,7 +81,7 @@ public class Include implements Serializable {
* Should the include recurse
*/
public Include(String theValue, boolean theIterate, boolean theImmutable) {
- myValue = theValue;
+ setValue(theValue);
myIterate = theIterate;
myImmutable = theImmutable;
}
@@ -128,41 +132,21 @@ public class Include implements Serializable {
* Returns the portion of the value before the first colon
*/
public String getParamType() {
- int firstColon = myValue.indexOf(':');
- if (firstColon == -1 || firstColon == myValue.length() - 1) {
- return null;
- }
- return myValue.substring(0, firstColon);
+ return myParamType;
}
/**
* Returns the portion of the value after the first colon but before the second colon
*/
public String getParamName() {
- int firstColon = myValue.indexOf(':');
- if (firstColon == -1 || firstColon == myValue.length() - 1) {
- return null;
- }
- int secondColon = myValue.indexOf(':', firstColon + 1);
- if (secondColon != -1) {
- return myValue.substring(firstColon + 1, secondColon);
- }
- return myValue.substring(firstColon + 1);
+ return myParamName;
}
/**
* Returns the portion of the string after the second colon, or null if there are not two colons in the value.
*/
public String getParamTargetType() {
- int firstColon = myValue.indexOf(':');
- if (firstColon == -1 || firstColon == myValue.length() - 1) {
- return null;
- }
- int secondColon = myValue.indexOf(':', firstColon + 1);
- if (secondColon != -1) {
- return myValue.substring(secondColon + 1);
- }
- return null;
+ return myParamTargetType;
}
@@ -207,7 +191,34 @@ public class Include implements Serializable {
if (myImmutable) {
throw new IllegalStateException("Can not change the value of this include");
}
+
+ String value = defaultString(theValue);
+
+ int firstColon = value.indexOf(':');
+ String paramType;
+ String paramName;
+ String paramTargetType;
+ if (firstColon == -1 || firstColon == value.length() - 1) {
+ paramType = null;
+ paramName = null;
+ paramTargetType = null;
+ } else {
+ paramType = value.substring(0, firstColon);
+ int secondColon = value.indexOf(':', firstColon + 1);
+ if (secondColon == -1) {
+ paramName = value.substring(firstColon + 1);
+ paramTargetType = null;
+ } else {
+ paramName = value.substring(firstColon + 1, secondColon);
+ paramTargetType = value.substring(secondColon + 1);
+ }
+ }
+
+ myParamType = paramType;
+ myParamName = paramName;
+ myParamTargetType = paramTargetType;
myValue = theValue;
+
}
/**
diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/Constants.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/Constants.java
index faa909dd42d..7269495194e 100644
--- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/Constants.java
+++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/Constants.java
@@ -290,6 +290,7 @@ public class Constants {
public static final String SUBSCRIPTION_MULTITYPE_SUFFIX = "]";
public static final String SUBSCRIPTION_MULTITYPE_STAR = "*";
public static final String SUBSCRIPTION_STAR_CRITERIA = SUBSCRIPTION_MULTITYPE_PREFIX + SUBSCRIPTION_MULTITYPE_STAR + SUBSCRIPTION_MULTITYPE_SUFFIX;
+ public static final String INCLUDE_STAR = "*";
static {
CHARSET_UTF8 = StandardCharsets.UTF_8;
diff --git a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties
index 56e6f8eb8b5..7b40ee0371a 100644
--- a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties
+++ b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties
@@ -121,6 +121,7 @@ ca.uhn.fhir.jpa.patch.FhirPatch.invalidMoveSourceIndex=Invalid move source index
ca.uhn.fhir.jpa.patch.FhirPatch.invalidMoveDestinationIndex=Invalid move destination index {0} for path {1} - Only have {2} existing entries
ca.uhn.fhir.jpa.searchparam.extractor.BaseSearchParamExtractor.externalReferenceNotAllowed=Resource contains external reference to URL "{0}" but this server is not configured to allow external references
ca.uhn.fhir.jpa.searchparam.extractor.BaseSearchParamExtractor.failedToExtractPaths=Failed to extract values from resource using FHIRPath "{0}": {1}
+ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl.invalidInclude=Invalid {0} parameter value: "{1}". {2}
ca.uhn.fhir.jpa.dao.LegacySearchBuilder.invalidQuantityPrefix=Unable to handle quantity prefix "{0}" for value: {1}
ca.uhn.fhir.jpa.dao.LegacySearchBuilder.invalidNumberPrefix=Unable to handle number prefix "{0}" for value: {1}
ca.uhn.fhir.jpa.dao.LegacySearchBuilder.sourceParamDisabled=The _source parameter is disabled on this server
diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3331-validate-includes-for-jpa.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3331-validate-includes-for-jpa.yaml
new file mode 100644
index 00000000000..8b9176263f2
--- /dev/null
+++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3331-validate-includes-for-jpa.yaml
@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 3331
+title: "The JPA server will now validate `_include` and `_revinclude` statements before beginning
+ search processing. This prevents an ugly JPA error when some invalid params are used."
diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java
index 760dd8fc8fe..a7cc2b60c4f 100644
--- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java
+++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java
@@ -30,9 +30,11 @@ import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
import ca.uhn.fhir.jpa.api.dao.IDao;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
import ca.uhn.fhir.jpa.api.svc.ISearchCoordinatorSvc;
+import ca.uhn.fhir.jpa.dao.BaseStorageDao;
import ca.uhn.fhir.jpa.dao.IResultIterator;
import ca.uhn.fhir.jpa.dao.ISearchBuilder;
import ca.uhn.fhir.jpa.dao.SearchBuilderFactory;
+import ca.uhn.fhir.jpa.dao.predicate.PredicateBuilderReference;
import ca.uhn.fhir.jpa.entity.Search;
import ca.uhn.fhir.jpa.entity.SearchInclude;
import ca.uhn.fhir.jpa.entity.SearchTypeEnum;
@@ -49,6 +51,7 @@ import ca.uhn.fhir.model.api.IQueryParameterType;
import ca.uhn.fhir.model.api.Include;
import ca.uhn.fhir.rest.api.CacheControlDirective;
import ca.uhn.fhir.rest.api.Constants;
+import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum;
import ca.uhn.fhir.rest.api.SearchTotalModeEnum;
import ca.uhn.fhir.rest.api.SummaryEnum;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
@@ -65,8 +68,10 @@ import ca.uhn.fhir.rest.server.method.PageMethodBinding;
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster;
import ca.uhn.fhir.rest.server.util.ICachedSearchDetails;
+import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
import ca.uhn.fhir.util.AsyncUtil;
import ca.uhn.fhir.util.StopWatch;
+import ca.uhn.fhir.util.UrlUtil;
import co.elastic.apm.api.ElasticApm;
import co.elastic.apm.api.Span;
import co.elastic.apm.api.Transaction;
@@ -109,8 +114,11 @@ import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
+import static org.apache.commons.lang3.StringUtils.isBlank;
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
@Component("mySearchCoordinatorSvc")
public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
@@ -152,6 +160,8 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
private PersistedJpaBundleProviderFactory myPersistedJpaBundleProviderFactory;
@Autowired
private IRequestPartitionHelperSvc myRequestPartitionHelperService;
+ @Autowired
+ private ISearchParamRegistry mySearchParamRegistry;
/**
* Constructor
@@ -318,6 +328,8 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
.add(SearchParameterMap.class, theParams);
CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_PRESEARCH_REGISTERED, params);
+ validateSearch(theParams);
+
Class extends IBaseResource> resourceTypeClass = myContext.getResourceDefinition(theResourceType).getImplementingClass();
final ISearchBuilder sb = mySearchBuilderFactory.newSearchBuilder(theCallingDao, theResourceType, resourceTypeClass);
sb.setFetchSize(mySyncSize);
@@ -356,6 +368,56 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
return retVal;
}
+ private void validateSearch(SearchParameterMap theParams) {
+ validateIncludes(theParams.getIncludes(), Constants.PARAM_INCLUDE);
+ validateIncludes(theParams.getRevIncludes(), Constants.PARAM_REVINCLUDE);
+ }
+
+ private void validateIncludes(Set includes, String name) {
+ for (Include next : includes) {
+ String value = next.getValue();
+ if (value.equals(Constants.INCLUDE_STAR) || isBlank(value)) {
+ continue;
+ }
+
+ String paramType = next.getParamType();
+ String paramName = next.getParamName();
+ String paramTargetType = next.getParamTargetType();
+
+ if (isBlank(paramType) || isBlank(paramName)) {
+ String msg = myContext.getLocalizer().getMessageSanitized(SearchCoordinatorSvcImpl.class, "invalidInclude", name, value, "");
+ throw new InvalidRequestException(msg);
+ }
+
+ if (!myDaoRegistry.isResourceTypeSupported(paramType)) {
+ String resourceTypeMsg = myContext.getLocalizer().getMessageSanitized(PredicateBuilderReference.class, "invalidResourceType", paramType);
+ String msg = myContext.getLocalizer().getMessage(SearchCoordinatorSvcImpl.class, "invalidInclude", UrlUtil.sanitizeUrlPart(name), UrlUtil.sanitizeUrlPart(value), resourceTypeMsg); // last param is pre-sanitized
+ throw new InvalidRequestException(msg);
+ }
+
+ if (isNotBlank(paramTargetType) && !myDaoRegistry.isResourceTypeSupported(paramTargetType)) {
+ String resourceTypeMsg = myContext.getLocalizer().getMessageSanitized(PredicateBuilderReference.class, "invalidResourceType", paramTargetType);
+ String msg = myContext.getLocalizer().getMessage(SearchCoordinatorSvcImpl.class, "invalidInclude", UrlUtil.sanitizeUrlPart(name), UrlUtil.sanitizeUrlPart(value), resourceTypeMsg); // last param is pre-sanitized
+ throw new InvalidRequestException(msg);
+ }
+
+ if (!Constants.INCLUDE_STAR.equals(paramName) && mySearchParamRegistry.getActiveSearchParam(paramType, paramName) == null) {
+ List validNames = mySearchParamRegistry
+ .getActiveSearchParams(paramType)
+ .values()
+ .stream()
+ .filter(t -> t.getParamType() == RestSearchParameterTypeEnum.REFERENCE)
+ .map(t -> UrlUtil.sanitizeUrlPart(t.getName()))
+ .sorted()
+ .collect(Collectors.toList());
+ String searchParamMessage = myContext.getLocalizer().getMessage(BaseStorageDao.class, "invalidSearchParameter", UrlUtil.sanitizeUrlPart(paramName), UrlUtil.sanitizeUrlPart(paramType), validNames);
+ String msg = myContext.getLocalizer().getMessage(SearchCoordinatorSvcImpl.class, "invalidInclude", UrlUtil.sanitizeUrlPart(name), UrlUtil.sanitizeUrlPart(value), searchParamMessage); // last param is pre-sanitized
+ throw new InvalidRequestException(msg);
+ }
+
+ }
+ }
+
@Override
public Optional getSearchTotal(String theUuid) {
SearchTask task = myIdToSearchTask.get(theUuid);
@@ -674,6 +736,95 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
(myParams.getSearchTotalMode() == null && SearchTotalModeEnum.ACCURATE.equals(myDaoConfig.getDefaultTotalMode()));
}
+ private static boolean isWantOnlyCount(SearchParameterMap myParams) {
+ return SummaryEnum.COUNT.equals(myParams.getSummaryMode())
+ | INTEGER_0.equals(myParams.getCount());
+ }
+
+ public static void populateSearchEntity(SearchParameterMap theParams, String theResourceType, String theSearchUuid, String theQueryString, Search theSearch, RequestPartitionId theRequestPartitionId) {
+ theSearch.setDeleted(false);
+ theSearch.setUuid(theSearchUuid);
+ theSearch.setCreated(new Date());
+ theSearch.setTotalCount(null);
+ theSearch.setNumFound(0);
+ theSearch.setPreferredPageSize(theParams.getCount());
+ theSearch.setSearchType(theParams.getEverythingMode() != null ? SearchTypeEnum.EVERYTHING : SearchTypeEnum.SEARCH);
+ theSearch.setLastUpdated(theParams.getLastUpdated());
+ theSearch.setResourceType(theResourceType);
+ theSearch.setStatus(SearchStatusEnum.LOADING);
+ theSearch.setSearchQueryString(theQueryString, theRequestPartitionId);
+
+ if (theParams.hasIncludes()) {
+ for (Include next : theParams.getIncludes()) {
+ theSearch.addInclude(new SearchInclude(theSearch, next.getValue(), false, next.isRecurse()));
+ }
+ }
+
+ for (Include next : theParams.getRevIncludes()) {
+ theSearch.addInclude(new SearchInclude(theSearch, next.getValue(), true, next.isRecurse()));
+ }
+ }
+
+ /**
+ * Creates a {@link Pageable} using a start and end index
+ */
+ @SuppressWarnings("WeakerAccess")
+ @Nullable
+ public static Pageable toPage(final int theFromIndex, int theToIndex) {
+ int pageSize = theToIndex - theFromIndex;
+ if (pageSize < 1) {
+ return null;
+ }
+
+ int pageIndex = theFromIndex / pageSize;
+
+ Pageable page = new AbstractPageRequest(pageIndex, pageSize) {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public long getOffset() {
+ return theFromIndex;
+ }
+
+ @Override
+ public Sort getSort() {
+ return Sort.unsorted();
+ }
+
+ @Override
+ public Pageable next() {
+ return null;
+ }
+
+ @Override
+ public Pageable previous() {
+ return null;
+ }
+
+ @Override
+ public Pageable first() {
+ return null;
+ }
+
+ @Override
+ public Pageable withPage(int theI) {
+ return null;
+ }
+ };
+
+ return page;
+ }
+
+ static void verifySearchHasntFailedOrThrowInternalErrorException(Search theSearch) {
+ if (theSearch.getStatus() == SearchStatusEnum.FAILED) {
+ Integer status = theSearch.getFailureCode();
+ status = defaultIfNull(status, 500);
+
+ String message = theSearch.getFailureMessage();
+ throw BaseServerResponseException.newInstance(status, message);
+ }
+ }
+
/**
* A search task is a Callable task that runs in
* a thread pool to handle an individual search. One instance
@@ -1253,93 +1404,4 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
}
- private static boolean isWantOnlyCount(SearchParameterMap myParams) {
- return SummaryEnum.COUNT.equals(myParams.getSummaryMode())
- | INTEGER_0.equals(myParams.getCount());
- }
-
- public static void populateSearchEntity(SearchParameterMap theParams, String theResourceType, String theSearchUuid, String theQueryString, Search theSearch, RequestPartitionId theRequestPartitionId) {
- theSearch.setDeleted(false);
- theSearch.setUuid(theSearchUuid);
- theSearch.setCreated(new Date());
- theSearch.setTotalCount(null);
- theSearch.setNumFound(0);
- theSearch.setPreferredPageSize(theParams.getCount());
- theSearch.setSearchType(theParams.getEverythingMode() != null ? SearchTypeEnum.EVERYTHING : SearchTypeEnum.SEARCH);
- theSearch.setLastUpdated(theParams.getLastUpdated());
- theSearch.setResourceType(theResourceType);
- theSearch.setStatus(SearchStatusEnum.LOADING);
- theSearch.setSearchQueryString(theQueryString, theRequestPartitionId);
-
- if (theParams.hasIncludes()) {
- for (Include next : theParams.getIncludes()) {
- theSearch.addInclude(new SearchInclude(theSearch, next.getValue(), false, next.isRecurse()));
- }
- }
-
- for (Include next : theParams.getRevIncludes()) {
- theSearch.addInclude(new SearchInclude(theSearch, next.getValue(), true, next.isRecurse()));
- }
- }
-
- /**
- * Creates a {@link Pageable} using a start and end index
- */
- @SuppressWarnings("WeakerAccess")
- @Nullable
- public static Pageable toPage(final int theFromIndex, int theToIndex) {
- int pageSize = theToIndex - theFromIndex;
- if (pageSize < 1) {
- return null;
- }
-
- int pageIndex = theFromIndex / pageSize;
-
- Pageable page = new AbstractPageRequest(pageIndex, pageSize) {
- private static final long serialVersionUID = 1L;
-
- @Override
- public long getOffset() {
- return theFromIndex;
- }
-
- @Override
- public Sort getSort() {
- return Sort.unsorted();
- }
-
- @Override
- public Pageable next() {
- return null;
- }
-
- @Override
- public Pageable previous() {
- return null;
- }
-
- @Override
- public Pageable first() {
- return null;
- }
-
- @Override
- public Pageable withPage(int theI) {
- return null;
- }
- };
-
- return page;
- }
-
- static void verifySearchHasntFailedOrThrowInternalErrorException(Search theSearch) {
- if (theSearch.getStatus() == SearchStatusEnum.FAILED) {
- Integer status = theSearch.getFailureCode();
- status = defaultIfNull(status, 500);
-
- String message = theSearch.getFailureMessage();
- throw BaseServerResponseException.newInstance(status, message);
- }
- }
-
}
diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java
index a95130ace11..c7dfd5527f0 100644
--- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java
+++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java
@@ -106,11 +106,10 @@ public class FhirResourceDaoR4SearchIncludeTest extends BaseJpaR4Test {
SearchParameterMap map = SearchParameterMap.newSynchronous()
.addInclude(new Include("CarePlan.patient"));
try {
- IBundleProvider results = myCarePlanDao.search(map);
- List ids = toUnqualifiedVersionlessIdValues(results);
- assertThat(ids.toString(), ids, containsInAnyOrder("CarePlan/CP-1"));
- } catch (Exception e) {
+ myCarePlanDao.search(map);
fail();
+ } catch (Exception e) {
+ // good
}
// Next verify it with the ":" syntax
diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java
index 779a5748e1e..349f203f88a 100644
--- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java
+++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java
@@ -155,6 +155,7 @@ import java.util.stream.Collectors;
import static ca.uhn.fhir.rest.api.Constants.PARAM_TYPE;
import static org.apache.commons.lang3.StringUtils.countMatches;
+import static org.apache.commons.lang3.StringUtils.leftPad;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
@@ -5118,7 +5119,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test {
StringOrListParam or = new StringOrListParam();
or.addOr(new StringParam("A1"));
for (int i = 0; i < 50; i++) {
- or.addOr(new StringParam(StringUtils.leftPad("", 200, (char) ('A' + i))));
+ or.addOr(new StringParam(leftPad("", 200, (char) ('A' + i))));
}
map.add(Patient.SP_NAME, or);
IBundleProvider results = myPatientDao.search(map);
@@ -5130,7 +5131,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test {
or.addOr(new StringParam("A1"));
or.addOr(new StringParam("A1"));
for (int i = 0; i < 50; i++) {
- or.addOr(new StringParam(StringUtils.leftPad("", 200, (char) ('A' + i))));
+ or.addOr(new StringParam(leftPad("", 200, (char) ('A' + i))));
}
map.add(Patient.SP_NAME, or);
results = myPatientDao.search(map);
@@ -5154,7 +5155,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test {
.setCode("MR");
IIdType id1 = myPatientDao.create(patient).getId().toUnqualifiedVersionless();
- runInTransaction(()->{
+ runInTransaction(() -> {
List params = myResourceIndexedSearchParamTokenDao
.findAll()
.stream()
@@ -5401,9 +5402,9 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test {
SearchParameterMap map = new SearchParameterMap();
StringOrListParam or = new StringOrListParam();
or.addOr(new StringParam("A1"));
- or.addOr(new StringParam(StringUtils.leftPad("", 200, 'A')));
- or.addOr(new StringParam(StringUtils.leftPad("", 200, 'B')));
- or.addOr(new StringParam(StringUtils.leftPad("", 200, 'C')));
+ or.addOr(new StringParam(leftPad("", 200, 'A')));
+ or.addOr(new StringParam(leftPad("", 200, 'B')));
+ or.addOr(new StringParam(leftPad("", 200, 'C')));
map.add(Patient.SP_NAME, or);
IBundleProvider results = myPatientDao.search(map);
assertEquals(1, results.getResources(0, 10).size());
@@ -5412,9 +5413,9 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test {
map = new SearchParameterMap();
or = new StringOrListParam();
or.addOr(new StringParam("A1"));
- or.addOr(new StringParam(StringUtils.leftPad("", 200, 'A')));
- or.addOr(new StringParam(StringUtils.leftPad("", 200, 'B')));
- or.addOr(new StringParam(StringUtils.leftPad("", 200, 'C')));
+ or.addOr(new StringParam(leftPad("", 200, 'A')));
+ or.addOr(new StringParam(leftPad("", 200, 'B')));
+ or.addOr(new StringParam(leftPad("", 200, 'C')));
map.add(Patient.SP_NAME, or);
results = myPatientDao.search(map);
assertEquals(1, results.getResources(0, 10).size());
@@ -5552,6 +5553,68 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test {
}
+ @Test
+ public void testInvalidInclude() {
+
+ // Empty is ignored (should not fail)
+ {
+ SearchParameterMap map = new SearchParameterMap()
+ .addInclude(new Include(""));
+ assertEquals(0, myPatientDao.search(map, mySrd).sizeOrThrowNpe());
+ }
+
+ // Very long
+ String longString = leftPad("", 10000, 'A');
+ try {
+ SearchParameterMap map = new SearchParameterMap()
+ .addInclude(new Include("Patient:" + longString));
+ myPatientDao.search(map, mySrd);
+ fail();
+ } catch (InvalidRequestException e) {
+ assertEquals("Invalid _include parameter value: \"Patient:" + longString + "\". Unknown search parameter \"" + longString + "\" for resource type \"Patient\". Valid search parameters for this search are: [general-practitioner, link, organization]", e.getMessage());
+ }
+
+ // Invalid
+ try {
+ SearchParameterMap map = new SearchParameterMap()
+ .addInclude(new Include(":"));
+ myPatientDao.search(map, mySrd);
+ fail();
+ } catch (InvalidRequestException e) {
+ assertEquals("Invalid _include parameter value: \":\". ", e.getMessage());
+ }
+
+ // Unknown resource
+ try {
+ SearchParameterMap map = new SearchParameterMap()
+ .addInclude(new Include("Foo:patient"));
+ myPatientDao.search(map, mySrd);
+ fail();
+ } catch (InvalidRequestException e) {
+ assertEquals("Invalid _include parameter value: \"Foo:patient\". Invalid/unsupported resource type: \"Foo\"", e.getMessage());
+ }
+
+ // Unknown param
+ try {
+ SearchParameterMap map = new SearchParameterMap()
+ .addInclude(new Include("Patient:foo"));
+ myPatientDao.search(map, mySrd);
+ fail();
+ } catch (InvalidRequestException e) {
+ assertEquals("Invalid _include parameter value: \"Patient:foo\". Unknown search parameter \"foo\" for resource type \"Patient\". Valid search parameters for this search are: [general-practitioner, link, organization]", e.getMessage());
+ }
+
+ // Unknown target type
+ try {
+ SearchParameterMap map = new SearchParameterMap()
+ .addInclude(new Include("Patient:organization:Foo"));
+ myPatientDao.search(map, mySrd);
+ fail();
+ } catch (InvalidRequestException e) {
+ assertEquals("Invalid _include parameter value: \"Patient:organization:Foo\". Invalid/unsupported resource type: \"Foo\"", e.getMessage());
+ }
+ }
+
private String toStringMultiline(List> theResults) {
StringBuilder b = new StringBuilder();
for (Object next : theResults) {
diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java
index 0ef4c4ed0c1..a71e946b2b8 100644
--- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java
+++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java
@@ -138,7 +138,7 @@ public interface IBundleProvider {
/**
* Get all resources
*
- * @return getResources(0, this.size ()). Return an empty list if size() is zero.
+ * @return getResources(0, this.size())
. Return an empty list if this.size()
is zero.
* @throws ConfigurationException if size() is null
*/
@Nonnull
diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/IncludeTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/IncludeTest.java
index bea32a32981..247c425c19a 100644
--- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/IncludeTest.java
+++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/IncludeTest.java
@@ -507,22 +507,5 @@ public class IncludeTest {
ourClient = builder.build();
}
-
- public static void main(String[] args) {
-
- Organization org = new Organization();
- org.setId("Organization/65546");
- org.getNameElement().setValue("Contained Test Organization");
-
- Patient patient = new Patient();
- patient.setId("Patient/1333");
- patient.addIdentifier().setSystem("urn:mrns").setValue("253345");
- patient.getManagingOrganization().setResource(patient);
-
- System.out.println(FhirContext.forR4().newXmlParser().setPrettyPrint(true).encodeResourceToString(patient));
-
- patient.getManagingOrganization().getReference();
-
- }
-
+
}