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

Fix #345 - ResponseValidatingInterceptor throws an exception with response has no content

This commit is contained in:
jamesagnew 2016-04-22 07:04:14 -04:00
parent 246a7c5e16
commit dd3f1dd33c
5 changed files with 134 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/BaseValidatingInterceptor.java
View File

@ -244,6 +244,10 @@ abstract class BaseValidatingInterceptor<T> extends InterceptorAdapter {
}
}
if (theRequest == null) {
return;
}
ValidationResult validationResult = doValidate(validator, theRequest);
if (myAddResponseIssueHeaderOnSeverity != null) {

52
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java
View File

@ -120,7 +120,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
TestUtil.clearAllStaticFieldsForUnitTest();
}
@Override
public void before() throws Exception {
super.before();
@ -438,8 +437,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
ourLog.info(responseString);
assertEquals(400, response.getStatusLine().getStatusCode());
OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, responseString);
assertEquals("Can not create resource with ID \"2\", ID must not be supplied on a create (POST) operation (use an HTTP PUT / update operation if you wish to supply an ID)",
oo.getIssue().get(0).getDiagnostics());
assertEquals("Can not create resource with ID \"2\", ID must not be supplied on a create (POST) operation (use an HTTP PUT / update operation if you wish to supply an ID)", oo.getIssue().get(0).getDiagnostics());
} finally {
response.getEntity().getContent().close();
@ -522,8 +520,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
//@formatter:on
fail();
} catch (PreconditionFailedException e) {
assertEquals("HTTP 412 Precondition Failed: Failed to DELETE resource with match URL \"Patient?identifier=testDeleteConditionalMultiple\" because this search matched 2 resources",
e.getMessage());
assertEquals("HTTP 412 Precondition Failed: Failed to DELETE resource with match URL \"Patient?identifier=testDeleteConditionalMultiple\" because this search matched 2 resources", e.getMessage());
}
// Not deleted yet..
@ -568,6 +565,27 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
}
}
/**
* Test for #345
*/
@Test
public void testDeleteNormal() throws IOException {
Patient p = new Patient();
p.addName().addFamily("FAM");
IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless();
ourClient.read().resource(Patient.class).withId(id).execute();
ourClient.delete().resourceById(id).execute();
try {
ourClient.read().resource(Patient.class).withId(id).execute();
fail();
} catch (ResourceGoneException e) {
// good
}
}
@Test
public void testDeleteResourceConditional1() throws IOException {
String methodName = "testDeleteResourceConditional1";
@ -634,7 +652,8 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
}
/*
* Try it with a raw socket call. The Apache client won't let us use the unescaped "|" in the URL but we want to make sure that works too..
* Try it with a raw socket call. The Apache client won't let us use the unescaped "|" in the URL but we want to
* make sure that works too..
*/
Socket sock = new Socket();
sock.setSoTimeout(3000);
@ -1168,8 +1187,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
response.close();
}
get = new HttpGet(ourServerBase + "/Patient/" + pId.getIdPart() + "/$everything?_lastUpdated=%3E" + new InstantType(new Date(time2)).getValueAsString() + "&_lastUpdated=%3C"
+ new InstantType(new Date(time3)).getValueAsString());
get = new HttpGet(ourServerBase + "/Patient/" + pId.getIdPart() + "/$everything?_lastUpdated=%3E" + new InstantType(new Date(time2)).getValueAsString() + "&_lastUpdated=%3C" + new InstantType(new Date(time3)).getValueAsString());
response = ourHttpClient.execute(get);
try {
assertEquals(200, response.getStatusLine().getStatusCode());
@ -1761,8 +1779,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
Patient patient = new Patient();
patient.addIdentifier().setSystem("urn:system").setValue("testSearchTokenParam001");
patient.addName().addFamily("Tester").addGiven("testSearchTokenParam1");
patient.addCommunication().getLanguage().setText("testSearchTokenParamComText").addCoding().setCode("testSearchTokenParamCode").setSystem("testSearchTokenParamSystem")
.setDisplay("testSearchTokenParamDisplay");
patient.addCommunication().getLanguage().setText("testSearchTokenParamComText").addCoding().setCode("testSearchTokenParamCode").setSystem("testSearchTokenParamSystem").setDisplay("testSearchTokenParamDisplay");
myPatientDao.create(patient, mySrd);
patient = new Patient();
@ -2474,8 +2491,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
ourLog.info(responseString);
assertEquals(400, response.getStatusLine().getStatusCode());
OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, responseString);
assertThat(oo.getIssue().get(0).getDiagnostics(), containsString(
"Can not update resource, resource body must contain an ID element which matches the request URL for update (PUT) operation - Resource body ID of \"333\" does not match URL ID of \"2\""));
assertThat(oo.getIssue().get(0).getDiagnostics(), containsString("Can not update resource, resource body must contain an ID element which matches the request URL for update (PUT) operation - Resource body ID of \"333\" does not match URL ID of \"2\""));
} finally {
response.close();
}
@ -2499,10 +2515,8 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
ourLog.info(resp);
assertEquals(412, response.getStatusLine().getStatusCode());
assertThat(resp, not(containsString("Resource has no id")));
assertThat(resp,
stringContainsInOrder(">ERROR<", "/f:Patient/f:contact", "<pre>SHALL at least contain a contact's details or a reference to an organization</pre>", "<issue><severity value=\"error\"/>",
"<code value=\"processing\"/>", "<diagnostics value=\"SHALL at least contain a contact's details or a reference to an organization\"/>",
"<location value=\"/f:Patient/f:contact\"/>"));
assertThat(resp, stringContainsInOrder(">ERROR<", "/f:Patient/f:contact", "<pre>SHALL at least contain a contact's details or a reference to an organization</pre>", "<issue><severity value=\"error\"/>", "<code value=\"processing\"/>",
"<diagnostics value=\"SHALL at least contain a contact's details or a reference to an organization\"/>", "<location value=\"/f:Patient/f:contact\"/>"));
} finally {
IOUtils.closeQuietly(response.getEntity().getContent());
response.close();
@ -2585,8 +2599,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
assertEquals(200, response.getStatusLine().getStatusCode());
assertThat(resp, not(containsString("Resource has no id")));
assertThat(resp, containsString("<pre>No issues detected during validation</pre>"));
assertThat(resp,
stringContainsInOrder("<issue>", "<severity value=\"information\"/>", "<code value=\"informational\"/>", "<diagnostics value=\"No issues detected during validation\"/>", "</issue>"));
assertThat(resp, stringContainsInOrder("<issue>", "<severity value=\"information\"/>", "<code value=\"informational\"/>", "<diagnostics value=\"No issues detected during validation\"/>", "</issue>"));
} finally {
IOUtils.closeQuietly(response.getEntity().getContent());
response.close();
@ -2611,8 +2624,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
assertEquals(200, response.getStatusLine().getStatusCode());
assertThat(resp, not(containsString("Resource has no id")));
assertThat(resp, containsString("<pre>No issues detected during validation</pre>"));
assertThat(resp,
stringContainsInOrder("<issue>", "<severity value=\"information\"/>", "<code value=\"informational\"/>", "<diagnostics value=\"No issues detected during validation\"/>", "</issue>"));
assertThat(resp, stringContainsInOrder("<issue>", "<severity value=\"information\"/>", "<code value=\"informational\"/>", "<diagnostics value=\"No issues detected during validation\"/>", "</issue>"));
} finally {
IOUtils.closeQuietly(response.getEntity().getContent());
response.close();

45
hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/RequestValidatingInterceptorDstu3Test.java
View File

@ -11,6 +11,8 @@ import java.util.concurrent.TimeUnit;
import org.apache.commons.io.IOUtils;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpDelete;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ContentType;
@ -22,9 +24,9 @@ import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator;
import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender;
import org.hl7.fhir.dstu3.model.IdType;
import org.hl7.fhir.dstu3.model.Patient;
import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.junit.AfterClass;
import org.junit.Before;
@ -35,6 +37,7 @@ import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.model.api.IResource;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.annotation.Create;
import ca.uhn.fhir.rest.annotation.Delete;
import ca.uhn.fhir.rest.annotation.IdParam;
import ca.uhn.fhir.rest.annotation.OptionalParam;
import ca.uhn.fhir.rest.annotation.ResourceParam;
@ -61,7 +64,6 @@ public class RequestValidatingInterceptorDstu3Test {
private RequestValidatingInterceptor myInterceptor;
@Before
public void before() {
ourLastRequestWasSearch = false;
@ -70,10 +72,10 @@ public class RequestValidatingInterceptorDstu3Test {
}
myInterceptor = new RequestValidatingInterceptor();
// myInterceptor.setFailOnSeverity(ResultSeverityEnum.ERROR);
// myInterceptor.setAddResponseHeaderOnSeverity(ResultSeverityEnum.INFORMATION);
// myInterceptor.setResponseHeaderName("X-RESP");
// myInterceptor.setResponseHeaderValue(RequestValidatingInterceptor.DEFAULT_RESPONSE_HEADER_VALUE);
// myInterceptor.setFailOnSeverity(ResultSeverityEnum.ERROR);
// myInterceptor.setAddResponseHeaderOnSeverity(ResultSeverityEnum.INFORMATION);
// myInterceptor.setResponseHeaderName("X-RESP");
// myInterceptor.setResponseHeaderValue(RequestValidatingInterceptor.DEFAULT_RESPONSE_HEADER_VALUE);
ourServlet.registerInterceptor(myInterceptor);
}
@ -258,7 +260,6 @@ public class RequestValidatingInterceptorDstu3Test {
assertThat(status.toString(), containsString("X-FHIR-Request-Validation: {\"resourceType\":\"OperationOutcome"));
}
@Test
public void testCreateXmlValidNoValidatorsSpecified() throws Exception {
Patient patient = new Patient();
@ -281,6 +282,27 @@ public class RequestValidatingInterceptorDstu3Test {
assertThat(status.toString(), not(containsString("X-FHIR-Request-Validation")));
}
/**
* Test for #345
*/
@Test
public void testDelete() throws Exception {
myInterceptor.setFailOnSeverity(null);
myInterceptor.setAddResponseHeaderOnSeverity(ResultSeverityEnum.INFORMATION);
HttpDelete httpDelete = new HttpDelete("http://localhost:" + ourPort + "/Patient/123");
CloseableHttpResponse status = ourClient.execute(httpDelete);
try {
ourLog.info("Response was:\n{}", status);
assertEquals(204, status.getStatusLine().getStatusCode());
assertThat(status.toString(), not(containsString("X-FHIR-Request-Validation")));
} finally {
IOUtils.closeQuietly(status);
}
}
@Test
public void testFetchMetadata() throws Exception {
myInterceptor.setAddResponseHeaderOnSeverity(ResultSeverityEnum.INFORMATION);
@ -346,21 +368,26 @@ public class RequestValidatingInterceptorDstu3Test {
ourClient = builder.build();
}
public static class PatientProvider implements IResourceProvider {
public static class PatientProvider implements IResourceProvider {
@Create()
public MethodOutcome createPatient(@ResourceParam Patient thePatient, @IdParam IdType theIdParam) {
return new MethodOutcome(new IdDt("Patient/001/_history/002"));
}
@Delete
public MethodOutcome delete(@IdParam IdType theId) {
return new MethodOutcome(theId.withVersion("2"));
}
@Override
public Class<? extends IBaseResource> getResourceType() {
return Patient.class;
}
@Search
public List<IResource> search(@OptionalParam(name="foo") StringParam theString) {
public List<IResource> search(@OptionalParam(name = "foo") StringParam theString) {
ourLastRequestWasSearch = true;
return new ArrayList<IResource>();
}

33
hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/ResponseValidatingInterceptorDstu3Test.java
View File

@ -12,6 +12,8 @@ import java.util.concurrent.TimeUnit;
import org.apache.commons.io.IOUtils;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpDelete;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
@ -20,6 +22,7 @@ import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator;
import org.hl7.fhir.dstu3.model.IdType;
import org.hl7.fhir.dstu3.model.Patient;
import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender;
import org.hl7.fhir.dstu3.model.Identifier.IdentifierUse;
@ -31,8 +34,11 @@ import org.junit.Ignore;
import org.junit.Test;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.rest.annotation.Delete;
import ca.uhn.fhir.rest.annotation.IdParam;
import ca.uhn.fhir.rest.annotation.OptionalParam;
import ca.uhn.fhir.rest.annotation.Search;
import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
import ca.uhn.fhir.rest.param.StringParam;
import ca.uhn.fhir.rest.server.interceptor.ResponseValidatingInterceptor;
@ -68,6 +74,28 @@ public class ResponseValidatingInterceptorDstu3Test {
ourServlet.registerInterceptor(myInterceptor);
}
/**
* Test for #345
*/
@Test
public void testDelete() throws Exception {
myInterceptor.setFailOnSeverity(null);
myInterceptor.setAddResponseHeaderOnSeverity(ResultSeverityEnum.INFORMATION);
HttpDelete httpDelete = new HttpDelete("http://localhost:" + ourPort + "/Patient/123");
CloseableHttpResponse status = ourClient.execute(httpDelete);
try {
ourLog.info("Response was:\n{}", status);
assertEquals(204, status.getStatusLine().getStatusCode());
assertThat(status.toString(), not(containsString("X-FHIR-Request-Validation")));
} finally {
IOUtils.closeQuietly(status);
}
}
@Test
public void testLongHeaderTruncated() throws Exception {
IValidatorModule module = new FhirInstanceValidator();
@ -351,6 +379,11 @@ public class ResponseValidatingInterceptorDstu3Test {
public static class PatientProvider implements IResourceProvider {
@Delete
public MethodOutcome delete(@IdParam IdType theId) {
return new MethodOutcome(theId.withVersion("2"));
}
@Override
public Class<? extends IBaseResource> getResourceType() {
return Patient.class;

8
src/changes/changes.xml
View File

@ -6,7 +6,13 @@
<title>HAPI FHIR Changelog</title>
</properties>
<body>
<release version="1.5" date="TBD">
<release version="1.6" date="TBD">
<action type="fix" issue="345">
ResponseValidatingInterceptor threw an InternalErrorException (HTTP 500) for operations
that do not return any content (e.g. delete). Thanks to Mohammad Jafari for reporting!
</action>
</release>
<release version="1.5" date="2016-04-20">
<action type="fix" issue="339">
Security Fix: XML parser was vulnerable to XXE (XML External Entity)
processing, which could result in local files on disk being disclosed.