honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix #1483 - Don't let RP methods handle a request with includes if the 

method doesn't understand them
This commit is contained in:
James Agnew 2019-09-25 10:54:37 -04:00
parent 0b630f6851
commit 57377f5557
4 changed files with 243 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java
View File

@ -19,15 +19,6 @@ package ca.uhn.fhir.rest.server.method;
* limitations under the License.
* #L%
*/
import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import java.lang.reflect.Method;
import java.util.*;
import org.apache.commons.lang3.StringUtils;
import org.hl7.fhir.instance.model.api.IAnyResource;
import org.hl7.fhir.instance.model.api.IBaseResource;
import ca.uhn.fhir.context.ConfigurationException;
import ca.uhn.fhir.context.FhirContext;
@ -44,27 +35,38 @@ import ca.uhn.fhir.rest.param.ParameterUtil;
import ca.uhn.fhir.rest.param.QualifierDetails;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import org.apache.commons.lang3.StringUtils;
import org.hl7.fhir.instance.model.api.IAnyResource;
import org.hl7.fhir.instance.model.api.IBaseResource;
import javax.annotation.Nonnull;
import java.lang.reflect.Method;
import java.util.*;
import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
public class SearchMethodBinding extends BaseResourceReturningMethodBinding {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchMethodBinding.class);
private static final Set<String> SPECIAL_SEARCH_PARAMS;
private String myCompartmentName;
private String myDescription;
private Integer myIdParamIndex;
private String myQueryName;
private boolean myAllowUnknownParams;
private final String myResourceProviderResourceName;
static {
HashSet<String> specialSearchParams = new HashSet<>();
specialSearchParams.add(IAnyResource.SP_RES_ID);
specialSearchParams.add(IAnyResource.SP_RES_LANGUAGE);
specialSearchParams.add(Constants.PARAM_INCLUDE);
specialSearchParams.add(Constants.PARAM_REVINCLUDE);
SPECIAL_SEARCH_PARAMS = Collections.unmodifiableSet(specialSearchParams);
}
private final String myResourceProviderResourceName;
private String myCompartmentName;
private String myDescription;
private Integer myIdParamIndex;
private String myQueryName;
private boolean myAllowUnknownParams;
public SearchMethodBinding(Class<? extends IBaseResource> theReturnResourceType, Class<? extends IBaseResource> theResourceProviderResourceType, Method theMethod, FhirContext theContext, Object theProvider) {
super(theReturnResourceType, theMethod, theContext, theProvider);
Search search = theMethod.getAnnotation(Search.class);
@ -129,22 +131,8 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding {
@Override
public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) {
String clientPreference = theRequest.getHeader(Constants.HEADER_PREFER);
boolean lenientHandling = false;
if(clientPreference != null)
{
String[] preferences = clientPreference.split(";");
for( String p : preferences){
if("handling:lenient".equalsIgnoreCase(p))
{
lenientHandling = true;
break;
}
}
}
if (theRequest.getId() != null && myIdParamIndex == null) {
ourLog.trace("Method {} doesn't match because ID is not null: {}", theRequest.getId());
ourLog.trace("Method {} doesn't match because ID is not null: {}", getMethod(), theRequest.getId());
return false;
}
if (theRequest.getRequestType() == RequestTypeEnum.GET && theRequest.getOperation() != null && !Constants.PARAM_SEARCH.equals(theRequest.getOperation())) {
@ -156,40 +144,39 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding {
return false;
}
if (theRequest.getRequestType() != RequestTypeEnum.GET && theRequest.getRequestType() != RequestTypeEnum.POST) {
ourLog.trace("Method {} doesn't match because request type is {}", getMethod());
ourLog.trace("Method {} doesn't match because request type is {}", getMethod(), theRequest.getRequestType());
return false;
}
if (!StringUtils.equals(myCompartmentName, theRequest.getCompartmentName())) {
ourLog.trace("Method {} doesn't match because it is for compartment {} but request is compartment {}", new Object[] { getMethod(), myCompartmentName, theRequest.getCompartmentName() });
ourLog.trace("Method {} doesn't match because it is for compartment {} but request is compartment {}", getMethod(), myCompartmentName, theRequest.getCompartmentName());
return false;
}
// This is used to track all the parameters so we can reject queries that
// have additional params we don't understand
Set<String> methodParamsTemp = new HashSet<String>();
Set<String> methodParamsTemp = new HashSet<>();
Set<String> unqualifiedNames = theRequest.getUnqualifiedToQualifiedNames().keySet();
Set<String> qualifiedParamNames = theRequest.getParameters().keySet();
for (int i = 0; i < this.getParameters().size(); i++) {
if (!(getParameters().get(i) instanceof BaseQueryParameter)) {
for (IParameter nextParameter : getParameters()) {
if (!(nextParameter instanceof BaseQueryParameter)) {
continue;
}
BaseQueryParameter temp = (BaseQueryParameter) getParameters().get(i);
String name = temp.getName();
if (temp.isRequired()) {
BaseQueryParameter nextQueryParameter = (BaseQueryParameter) nextParameter;
String name = nextQueryParameter.getName();
if (nextQueryParameter.isRequired()) {
if (qualifiedParamNames.contains(name)) {
QualifierDetails qualifiers = extractQualifiersFromParameterName(name);
if (qualifiers.passes(temp.getQualifierWhitelist(), temp.getQualifierBlacklist())) {
if (qualifiers.passes(nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist())) {
methodParamsTemp.add(name);
}
}
if (unqualifiedNames.contains(name)) {
List<String> qualifiedNames = theRequest.getUnqualifiedToQualifiedNames().get(name);
qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, temp.getQualifierWhitelist(), temp.getQualifierBlacklist());
qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist());
methodParamsTemp.addAll(qualifiedNames);
}
if (!qualifiedParamNames.contains(name) && !unqualifiedNames.contains(name))
{
if (!qualifiedParamNames.contains(name) && !unqualifiedNames.contains(name)) {
ourLog.trace("Method {} doesn't match param '{}' is not present", getMethod().getName(), name);
return false;
}
@ -197,13 +184,13 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding {
} else {
if (qualifiedParamNames.contains(name)) {
QualifierDetails qualifiers = extractQualifiersFromParameterName(name);
if (qualifiers.passes(temp.getQualifierWhitelist(), temp.getQualifierBlacklist())) {
if (qualifiers.passes(nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist())) {
methodParamsTemp.add(name);
}
}
if (unqualifiedNames.contains(name)) {
List<String> qualifiedNames = theRequest.getUnqualifiedToQualifiedNames().get(name);
qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, temp.getQualifierWhitelist(), temp.getQualifierBlacklist());
qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist());
methodParamsTemp.addAll(qualifiedNames);
}
if (!qualifiedParamNames.contains(name)) {
@ -237,8 +224,6 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding {
}
}
Set<String> keySet = theRequest.getParameters().keySet();
if(lenientHandling == true)
return true;
if (myAllowUnknownParams == false) {
for (String next : keySet) {
@ -272,7 +257,7 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding {
if (theQualifierWhitelist == null && theQualifierBlacklist == null) {
return theQualifiedNames;
}
ArrayList<String> retVal = new ArrayList<String>(theQualifiedNames.size());
ArrayList<String> retVal = new ArrayList<>(theQualifiedNames.size());
for (String next : theQualifiedNames) {
QualifierDetails qualifiers = extractQualifiersFromParameterName(next);
if (!qualifiers.passes(theQualifierWhitelist, theQualifierBlacklist)) {
@ -287,6 +272,7 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding {
public String toString() {
return getMethod().toString();
}
public static QualifierDetails extractQualifiersFromParameterName(String theParamName) {
QualifierDetails retVal = new QualifierDetails();
if (theParamName == null || theParamName.length() == 0) {

8
hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java
View File

@ -35,15 +35,15 @@ public class SearchMethodBindingTest {
}
@Test // fails
public void methodShouldNotMatchWhenExtraUnderscoreQueryParameter() throws NoSuchMethodException {
public void methodShouldNotMatchWhenUnderscoreQueryParameter() throws NoSuchMethodException {
Assert.assertThat(getBinding("param", String.class).incomingServerRequestMatchesMethod(
mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_extra", new String[]{"test"}))),
mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_include", new String[]{"test"}))),
Matchers.is(false));
Assert.assertThat(getBinding("paramAndTest", String.class, String.class).incomingServerRequestMatchesMethod(
mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_extra", new String[]{"test"}))),
mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_include", new String[]{"test"}))),
Matchers.is(false));
Assert.assertThat(getBinding("paramAndUnderscoreTest", String.class, String.class).incomingServerRequestMatchesMethod(
mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_extra", new String[]{"test"}))),
mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_include", new String[]{"test"}))),
Matchers.is(false));
}

190
hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/ServerMethodSelectionR4Test.java Normal file
View File

@ -0,0 +1,190 @@
package ca.uhn.fhir.rest.server;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.context.api.BundleInclusionRule;
import ca.uhn.fhir.model.api.Include;
import ca.uhn.fhir.rest.annotation.IncludeParam;
import ca.uhn.fhir.rest.annotation.OptionalParam;
import ca.uhn.fhir.rest.annotation.Search;
import ca.uhn.fhir.rest.api.EncodingEnum;
import ca.uhn.fhir.rest.client.api.IGenericClient;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.test.utilities.JettyUtil;
import com.google.common.collect.Lists;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.Patient;
import org.hl7.fhir.r4.model.StringType;
import org.junit.After;
import org.junit.Test;
import java.util.List;
import java.util.Set;
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.*;
public class ServerMethodSelectionR4Test {
private FhirContext myCtx = FhirContext.forR4();
private Server myServer;
private IGenericClient myClient;
@After
public void after() throws Exception {
JettyUtil.closeServer(myServer);
}
/**
* Server method with no _include
* Client request with _include
* <p>
* See #1421
*/
@Test
public void testRejectIncludeIfNotProvided() throws Exception {
class MyProvider extends MyBaseProvider {
@Search
public List<IBaseResource> search(@OptionalParam(name = "name") StringType theName) {
return Lists.newArrayList(new Patient().setActive(true).setId("Patient/123"));
}
}
MyProvider provider = new MyProvider();
startServer(provider);
try {
myClient
.search()
.forResource(Patient.class)
.where(Patient.NAME.matches().value("foo"))
.include(Patient.INCLUDE_ORGANIZATION)
.execute();
fail();
} catch (InvalidRequestException e) {
assertThat(e.getMessage(), containsString("this server does not know how to handle GET operation[Patient] with parameters [[_include, name]]"));
}
}
/**
* Server method with no _include
* Client request with _include
* <p>
* See #1421
*/
@Test
public void testAllowIncludeIfProvided() throws Exception {
class MyProvider extends MyBaseProvider {
@Search
public List<IBaseResource> search(@OptionalParam(name = "name") StringType theName, @IncludeParam Set<Include> theIncludes) {
return Lists.newArrayList(new Patient().setActive(true).setId("Patient/123"));
}
}
MyProvider provider = new MyProvider();
startServer(provider);
Bundle results = myClient
.search()
.forResource(Patient.class)
.where(Patient.NAME.matches().value("foo"))
.include(Patient.INCLUDE_ORGANIZATION)
.returnBundle(Bundle.class)
.execute();
assertEquals(1, results.getEntry().size());
}
/**
* Server method with no _revinclude
* Client request with _revinclude
* <p>
* See #1421
*/
@Test
public void testRejectRevIncludeIfNotProvided() throws Exception {
class MyProvider extends MyBaseProvider {
@Search
public List<IBaseResource> search(@OptionalParam(name = "name") StringType theName) {
return Lists.newArrayList(new Patient().setActive(true).setId("Patient/123"));
}
}
MyProvider provider = new MyProvider();
startServer(provider);
try {
myClient
.search()
.forResource(Patient.class)
.where(Patient.NAME.matches().value("foo"))
.revInclude(Patient.INCLUDE_ORGANIZATION)
.execute();
fail();
} catch (InvalidRequestException e) {
assertThat(e.getMessage(), containsString("this server does not know how to handle GET operation[Patient] with parameters [[_revinclude, name]]"));
}
}
/**
* Server method with no _revInclude
* Client request with _revInclude
* <p>
* See #1421
*/
@Test
public void testAllowRevIncludeIfProvided() throws Exception {
class MyProvider extends MyBaseProvider {
@Search
public List<IBaseResource> search(@OptionalParam(name = "name") StringType theName, @IncludeParam(reverse = true) Set<Include> theRevIncludes) {
return Lists.newArrayList(new Patient().setActive(true).setId("Patient/123"));
}
}
MyProvider provider = new MyProvider();
startServer(provider);
Bundle results = myClient
.search()
.forResource(Patient.class)
.where(Patient.NAME.matches().value("foo"))
.revInclude(Patient.INCLUDE_ORGANIZATION)
.returnBundle(Bundle.class)
.execute();
assertEquals(1, results.getEntry().size());
}
private void startServer(Object theProvider) throws Exception {
RestfulServer servlet = new RestfulServer(myCtx);
servlet.registerProvider(theProvider);
ServletHandler proxyHandler = new ServletHandler();
servlet.setDefaultResponseEncoding(EncodingEnum.XML);
servlet.setBundleInclusionRule(BundleInclusionRule.BASED_ON_RESOURCE_PRESENCE);
ServletHolder servletHolder = new ServletHolder(servlet);
proxyHandler.addServletWithMapping(servletHolder, "/*");
myServer = new Server(0);
myServer.setHandler(proxyHandler);
JettyUtil.startServer(myServer);
int port = JettyUtil.getPortForStartedServer(myServer);
myClient = myCtx.newRestfulGenericClient("http://localhost:" + port);
}
public static class MyBaseProvider implements IResourceProvider {
@Override
public Class<? extends IBaseResource> getResourceType() {
return Patient.class;
}
}
}

5
src/changes/changes.xml
View File

@ -212,6 +212,11 @@
with the new request id, resulting in an ever growing source.meta value. E.g. after the first update, it looks
like "#9f0a901387128111#5f37835ee38a89e2" when it should only be "#5f37835ee38a89e2". This has been corrected.
</action>
<action type="fix" issue="1421">
The Plain Server method selector was incorrectly allowing client requests with _include statements to be
handled by method implementations that did not have any <![CDATA[<code>@IncludeParam</code>]]> defined. This
is now corrected. Thanks to Tuomo Ala-Vannesluoma for reporting and providing a test case!
</action>
</release>
<release version="4.0.3" date="2019-09-03" description="Igloo (Point Release)">
<action type="fix">