From e53aaf095075db036b21e3ec11fe1a732b2a33f4 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Tue, 22 Jul 2014 14:25:33 -0400 Subject: [PATCH] Add content-disposition header for binary resources --- hapi-fhir-base/src/changes/changes.xml | 8 ++- .../ca/uhn/fhir/rest/server/Constants.java | 1 + .../uhn/fhir/rest/server/RestfulServer.java | 3 + .../ca/uhn/fhir/rest/server/ReadTest.java | 56 ++++++++++++++++++- 4 files changed, 66 insertions(+), 2 deletions(-) diff --git a/hapi-fhir-base/src/changes/changes.xml b/hapi-fhir-base/src/changes/changes.xml index 3b989507e9c..e5d8f2c4119 100644 --- a/hapi-fhir-base/src/changes/changes.xml +++ b/hapi-fhir-base/src/changes/changes.xml @@ -8,7 +8,7 @@ - Allow server methods to return wildcard genrric types (e.g. List<? extends IResource>) + Allow server methods to return wildcard generic types (e.g. List<? extends IResource>) Search parameters are not properly escaped and unescaped. E.g. for a token parameter such as @@ -47,6 +47,12 @@ Fix issue where vread invocations on server incorrectly get routed to instance history method if one is defined. Thanks to Neal Acharya from UHN for surfacing this one! + + Binary reads on a server not include the Content-Disposition header, to prevent HTML in binary + blobs from being used for nefarious purposes. See + FHIR Tracker Bug 3298]]> + for more information. + diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java index 1a254a04357..22747f967f4 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/Constants.java @@ -95,6 +95,7 @@ public class Constants { public static final String HEADERVALUE_CORS_ALLOW_METHODS_ALL = "GET, POST, PUT, DELETE"; public static final String HEADER_AUTHORIZATION = "Authorization"; public static final String PARAMQUALIFIER_TOKEN_TEXT = ":text"; + public static final String HEADER_CONTENT_DISPOSITION = "Content-Disposition"; static { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java index 23b99e7fa46..bc72e673ba1 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java @@ -1099,6 +1099,9 @@ public class RestfulServer extends HttpServlet { if (bin.getContent() == null || bin.getContent().length == 0) { return; } + + theHttpResponse.addHeader(Constants.HEADER_CONTENT_DISPOSITION, "Attachment;"); + theHttpResponse.setContentLength(bin.getContent().length); ServletOutputStream oos = theHttpResponse.getOutputStream(); oos.write(bin.getContent()); diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/server/ReadTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/server/ReadTest.java index 55dd80e9985..7694c0ce6ac 100644 --- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/server/ReadTest.java +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/server/ReadTest.java @@ -21,6 +21,7 @@ import org.junit.Test; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.dstu.composite.IdentifierDt; +import ca.uhn.fhir.model.dstu.resource.Binary; import ca.uhn.fhir.model.dstu.resource.Patient; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.annotation.IdParam; @@ -58,6 +59,36 @@ public class ReadTest { } } + + + @Test + public void testBinaryRead() throws Exception { + { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Binary/1"); + HttpResponse status = ourClient.execute(httpGet); + byte[] responseContent = IOUtils.toByteArray(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + + + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals("application/x-foo", status.getEntity().getContentType().getValue()); + + Header cl = status.getFirstHeader(Constants.HEADER_CONTENT_LOCATION_LC); + assertNotNull(cl); + assertEquals("http://localhost:" + ourPort + "/Binary/1/_history/1", cl.getValue()); + + Header cd = status.getFirstHeader("content-disposition"); + assertNotNull(cd); + assertEquals("Attachment;", cd.getValue()); + + assertEquals(4,responseContent.length); + for (int i = 0; i < 4; i++) { + assertEquals(i+1, responseContent[i]); // should be 1,2,3,4 + } + + } + + } @Test public void testVRead() throws Exception { @@ -93,7 +124,7 @@ public class ReadTest { ServletHandler proxyHandler = new ServletHandler(); RestfulServer servlet = new RestfulServer(); ourCtx = servlet.getFhirContext(); - servlet.setResourceProviders(patientProvider); + servlet.setResourceProviders(patientProvider, new DummyBinaryProvider()); ServletHolder servletHolder = new ServletHolder(servlet); proxyHandler.addServletWithMapping(servletHolder, "/*"); ourServer.setHandler(proxyHandler); @@ -126,4 +157,27 @@ public class ReadTest { } + + /** + * Created by dsotnikov on 2/25/2014. + */ + public static class DummyBinaryProvider implements IResourceProvider { + + @Read(version = true) + public Binary findPatient(@IdParam IdDt theId) { + Binary bin = new Binary(); + bin.setContentType("application/x-foo"); + bin.setContent(new byte[] {1,2,3,4}); + bin.setId("Binary/1/_history/1"); + return bin; + } + + @Override + public Class getResourceType() { + return Binary.class; + } + + } + + }