Validate include statements for JPA (#3331)

* Validate include statements for JPA

* Add changelog

* Optmize test fix

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.
  * </p>
+ * <p>
+ * Note on thrwead safety: This class is not thread safe.
+ * </p>
  */
 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 <b>non-recursive</b> include
@@ -50,8 +57,7 @@ public class Include implements Serializable {
 	 *           The <code>_include</code> value, e.g. "Patient:name"
 	 */
 	public Include(String theValue) {
-		myValue = theValue;
+		this(theValue, false);
-		myImmutable = false;
 	}
 
 	/**
@@ -63,9 +69,7 @@ public class Include implements Serializable {
 	 *           Should the include recurse
 	 */
 	public Include(String theValue, boolean theIterate) {
-		myValue = theValue;
+		this(theValue, theIterate, false);
-		myIterate = theIterate;
-		myImmutable = 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(':');
+		return myParamType;
-		if (firstColon == -1 || firstColon == myValue.length() - 1) {
-			return null;
-		}
-		return myValue.substring(0, firstColon);
 	}
 
 	/**
 	 * Returns the portion of the value after the first colon but before the second colon
 	 */
 	public String getParamName() {
-		int firstColon = myValue.indexOf(':');
+		return myParamName;
-		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);
 	}
 
 	/**
 	 * 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(':');
+		return myParamTargetType;
-		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;
 
 	}
 
@@ -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;
+
 	}
 
 	/**

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;

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

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."

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<Include> 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<String> 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<Integer> 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);
-		}
-	}
-
 }

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);
+			myCarePlanDao.search(map);
-			List<String> ids = toUnqualifiedVersionlessIdValues(results);
-			assertThat(ids.toString(), ids, containsInAnyOrder("CarePlan/CP-1"));
-		} catch (Exception e) {
 			fail();
+		} catch (Exception e) {
+			// good
 		}
 
 		// Next verify it with the ":" syntax

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<ResourceIndexedSearchParamToken> 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(leftPad("", 200, 'A')));
-		or.addOr(new StringParam(StringUtils.leftPad("", 200, 'B')));
+		or.addOr(new StringParam(leftPad("", 200, 'B')));
-		or.addOr(new StringParam(StringUtils.leftPad("", 200, 'C')));
+		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(leftPad("", 200, 'A')));
-		or.addOr(new StringParam(StringUtils.leftPad("", 200, 'B')));
+		or.addOr(new StringParam(leftPad("", 200, 'B')));
-		or.addOr(new StringParam(StringUtils.leftPad("", 200, 'C')));
+		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) {

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 <code>getResources(0, this.size())</code>.  Return an empty list if <code>this.size()</code> is zero.
 	 * @throws ConfigurationException if size() is null
 	 */
 	@Nonnull

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/IncludeTest.java

@@ -508,21 +508,4 @@ public class IncludeTest {
 
 	}
 	
-	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();
-
-	}
-
 }