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 899975bc2eee..e09ae1d4c5b5 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 @@ * 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 Include(String theValue) { * 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 Include(String theValue, boolean theIterate) { * 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 boolean equals(Object obj) { * 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 void setValue(String theValue) { 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 faa909dd42d5..7269495194e1 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 56e6f8eb8b5d..7b40ee0371a3 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 000000000000..8b9176263f27 --- /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 760dd8fc8fe1..a7cc2b60c4f6 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.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.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.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.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 IBundleProvider registerSearch(final IFhirResourceDao theCallingDao, f .add(SearchParameterMap.class, theParams); CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_PRESEARCH_REGISTERED, params); + validateSearch(theParams); + Class resourceTypeClass = myContext.getResourceDefinition(theResourceType).getImplementingClass(); final ISearchBuilder sb = mySearchBuilderFactory.newSearchBuilder(theCallingDao, theResourceType, resourceTypeClass); sb.setFetchSize(mySyncSize); @@ -356,6 +368,56 @@ public IBundleProvider registerSearch(final IFhirResourceDao theCallingDao, f 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 @@ private boolean isWantCount(SearchParameterMap myParams, boolean wantOnlyCount) (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 Void call() { } - 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 a95130ace11e..c7dfd5527f04 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 void testSearchWithIncludeSpecDoesNotCauseNPE() { 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 779a5748e1e2..349f203f88ab 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 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 void testSearchWithVeryLongUrlLonger() { 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 void testSearchWithVeryLongUrlLonger() { 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 void testTokenOfType() { .setCode("MR"); IIdType id1 = myPatientDao.create(patient).getId().toUnqualifiedVersionless(); - runInTransaction(()->{ + runInTransaction(() -> { List params = myResourceIndexedSearchParamTokenDao .findAll() .stream() @@ -5401,9 +5402,9 @@ public void testSearchWithVeryLongUrlShorter() { 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 void testSearchWithVeryLongUrlShorter() { 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 void testCircularReferencesDontBreakRevIncludes() { } + @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 0ef4c4ed0c11..a71e946b2b85 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 @@ default Integer getCurrentPageSize() { /** * 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 bea32a329819..247c425c19a7 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 static void beforeClass() throws Exception { 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(); - - } - + }