From 82c6d824445424f132ad897d7d67911afb497594 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 18 Mar 2016 18:38:44 +0100 Subject: [PATCH] Sere up Binary resources as binary content even if the browser puts application/xml in the Accept header --- .../ca/uhn/fhir/rest/server/EncodingEnum.java | 21 ++++- .../fhir/rest/server/RestfulServerUtils.java | 20 ++++- .../ResponseHighlighterInterceptor.java | 10 +++ .../ca/uhn/fhir/rest/server/BinaryTest.java | 3 - .../uhn/fhir/rest/server/BinaryDstu2Test.java | 39 ++++++++- .../ResponseHighlightingInterceptorTest.java | 79 ++++++++++++++++++- src/changes/changes.xml | 9 ++- 7 files changed, 166 insertions(+), 15 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/EncodingEnum.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/EncodingEnum.java index 29ddc0cf1c4..7dc4380632a 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/EncodingEnum.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/EncodingEnum.java @@ -1,5 +1,7 @@ package ca.uhn.fhir.rest.server; +import java.util.Collections; + /* * #%L * HAPI FHIR - Core Library @@ -21,6 +23,7 @@ package ca.uhn.fhir.rest.server; */ import java.util.HashMap; +import java.util.Map; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.parser.IParser; @@ -43,8 +46,8 @@ public enum EncodingEnum { ; - private static HashMap ourContentTypeToEncoding; - private static HashMap ourContentTypeToEncodingStrict; + private static Map ourContentTypeToEncoding; + private static Map ourContentTypeToEncodingStrict; static { ourContentTypeToEncoding = new HashMap(); @@ -54,7 +57,7 @@ public enum EncodingEnum { } // Add before we add the lenient ones - ourContentTypeToEncodingStrict = new HashMap(ourContentTypeToEncoding); + ourContentTypeToEncodingStrict = Collections.unmodifiableMap(new HashMap(ourContentTypeToEncoding)); /* * These are wrong, but we add them just to be tolerant of other @@ -116,6 +119,18 @@ public enum EncodingEnum { return ourContentTypeToEncodingStrict.get(theContentType); } + /** + * Returns a map containing the encoding for a given content type, or null if no encoding + * is found. + *

+ * This method is NOT lenient! Things like "application/xml" will return null + *

+ * @see #forContentType(String) + */ + public static Map getContentTypeToEncodingStrict() { + return ourContentTypeToEncodingStrict; + } + public String getFormatContentType() { return myFormatContentType; } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java index 463e9071821..52e3e84bffa 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java @@ -251,6 +251,20 @@ public class RestfulServerUtils { } } + /* + * Some browsers (e.g. FF) request "application/xml" in their Accept header, + * and we generally want to treat this as a preference for FHIR XML even if + * it's not the FHIR version of the CT, which should be "application/xml+fhir". + * + * When we're serving up Binary resources though, we are a bit more strict, + * since Binary is supposed to use native content types unless the client has + * explicitly requested FHIR. + */ + Map contentTypeToEncoding = Constants.FORMAT_VAL_TO_ENCODING; + if ("Binary".equals(theReq.getResourceName())) { + contentTypeToEncoding = EncodingEnum.getContentTypeToEncodingStrict(); + } + /* * The Accept header is kind of ridiculous, e.g. */ @@ -288,12 +302,12 @@ public class RestfulServerUtils { EncodingEnum encoding; if (endSpaceIndex == -1) { if (startSpaceIndex == 0) { - encoding = Constants.FORMAT_VAL_TO_ENCODING.get(nextToken); + encoding = contentTypeToEncoding.get(nextToken); } else { - encoding = Constants.FORMAT_VAL_TO_ENCODING.get(nextToken.substring(startSpaceIndex)); + encoding = contentTypeToEncoding.get(nextToken.substring(startSpaceIndex)); } } else { - encoding = Constants.FORMAT_VAL_TO_ENCODING.get(nextToken.substring(startSpaceIndex, endSpaceIndex)); + encoding = contentTypeToEncoding.get(nextToken.substring(startSpaceIndex, endSpaceIndex)); String remaining = nextToken.substring(endSpaceIndex + 1); StringTokenizer qualifierTok = new StringTokenizer(remaining, ";"); while (qualifierTok.hasMoreTokens()) { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java index 641f62eceaa..f7a890ca108 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java @@ -207,6 +207,16 @@ public class ResponseHighlighterInterceptor extends InterceptorAdapter { return super.outgoingResponse(theRequestDetails, theResponseObject, theServletRequest, theServletResponse); } + /* + * Not binary + */ + if ("Binary".equals(theRequestDetails.getResourceName())) { + return super.outgoingResponse(theRequestDetails, theResponseObject, theServletRequest, theServletResponse); + } + + /* + * Request for _raw + */ String[] rawParamValues = theRequestDetails.getParameters().get(PARAM_RAW); if (rawParamValues != null && rawParamValues.length > 0 && rawParamValues[0].equals(PARAM_RAW_TRUE)) { return super.outgoingResponse(theRequestDetails, theResponseObject, theServletRequest, theServletResponse); diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/BinaryTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/BinaryTest.java index 3e3da4ecad1..21b870e167a 100644 --- a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/BinaryTest.java +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/BinaryTest.java @@ -38,9 +38,6 @@ import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.util.PortUtil; -/** - * Created by dsotnikov on 2/25/2014. - */ public class BinaryTest { private static CloseableHttpClient ourClient; diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/BinaryDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/BinaryDstu2Test.java index 69d82794c9e..78f99325ea2 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/BinaryDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/BinaryDstu2Test.java @@ -3,6 +3,7 @@ package ca.uhn.fhir.rest.server; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import java.util.Collections; @@ -126,17 +127,50 @@ public class BinaryDstu2Test { } @Test - public void testRead() throws Exception { + public void testBinaryReadAcceptMissing() throws Exception { HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Binary/foo"); + HttpResponse status = ourClient.execute(httpGet); byte[] responseContent = IOUtils.toByteArray(status.getEntity().getContent()); IOUtils.closeQuietly(status.getEntity().getContent()); assertEquals(200, status.getStatusLine().getStatusCode()); assertEquals("foo", status.getFirstHeader("content-type").getValue()); + assertEquals("Attachment;", status.getFirstHeader("Content-Disposition").getValue()); assertArrayEquals(new byte[] { 1, 2, 3, 4 }, responseContent); } + @Test + public void testBinaryReadAcceptBrowser() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Binary/foo"); + httpGet.addHeader("User-Agent", "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.1"); + httpGet.addHeader("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"); + + HttpResponse status = ourClient.execute(httpGet); + byte[] responseContent = IOUtils.toByteArray(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals("foo", status.getFirstHeader("content-type").getValue()); + assertEquals("Attachment;", status.getFirstHeader("Content-Disposition").getValue()); + assertArrayEquals(new byte[] { 1, 2, 3, 4 }, responseContent); + } + + @Test + public void testBinaryReadAcceptFhirJson() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Binary/foo"); + httpGet.addHeader("User-Agent", "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.1"); + httpGet.addHeader("Accept", Constants.CT_FHIR_JSON); + + HttpResponse status = ourClient.execute(httpGet); + String responseContent = IOUtils.toString(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(Constants.CT_FHIR_JSON + ";charset=utf-8", status.getFirstHeader("content-type").getValue().replace(" ", "").toLowerCase()); + assertNull(status.getFirstHeader("Content-Disposition")); + assertEquals("{\"resourceType\":\"Binary\",\"id\":\"1\",\"contentType\":\"foo\",\"content\":\"AQIDBA==\"}", responseContent); + + } + @Test public void testSearchJson() throws Exception { HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Binary?_pretty=true&_format=json"); @@ -200,9 +234,6 @@ public class BinaryDstu2Test { } - /** - * Created by dsotnikov on 2/25/2014. - */ public static class ResourceProvider implements IResourceProvider { @Create diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java index a5ceaa515bb..6542fc8aaba 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java @@ -3,8 +3,10 @@ package ca.uhn.fhir.rest.server.interceptor; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.stringContainsInOrder; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; @@ -23,6 +25,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.commons.io.IOUtils; +import org.apache.http.HttpResponse; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.impl.client.CloseableHttpClient; @@ -42,6 +45,7 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.dstu2.composite.HumanNameDt; import ca.uhn.fhir.model.dstu2.composite.IdentifierDt; +import ca.uhn.fhir.model.dstu2.resource.Binary; import ca.uhn.fhir.model.dstu2.resource.OperationOutcome; import ca.uhn.fhir.model.dstu2.resource.OperationOutcome.Issue; import ca.uhn.fhir.model.dstu2.resource.Organization; @@ -343,6 +347,51 @@ public class ResponseHighlightingInterceptorTest { assertThat(responseContent, not(containsString("entry"))); } + @Test + public void testBinaryReadAcceptMissing() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Binary/foo"); + + HttpResponse status = ourClient.execute(httpGet); + byte[] responseContent = IOUtils.toByteArray(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals("foo", status.getFirstHeader("content-type").getValue()); + assertEquals("Attachment;", status.getFirstHeader("Content-Disposition").getValue()); + assertArrayEquals(new byte[] { 1, 2, 3, 4 }, responseContent); + + } + + @Test + public void testBinaryReadAcceptBrowser() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Binary/foo"); + httpGet.addHeader("User-Agent", "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.1"); + httpGet.addHeader("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"); + + HttpResponse status = ourClient.execute(httpGet); + byte[] responseContent = IOUtils.toByteArray(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals("foo", status.getFirstHeader("content-type").getValue()); + assertEquals("Attachment;", status.getFirstHeader("Content-Disposition").getValue()); + assertArrayEquals(new byte[] { 1, 2, 3, 4 }, responseContent); + } + + @Test + public void testBinaryReadAcceptFhirJson() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Binary/foo"); + httpGet.addHeader("User-Agent", "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.1"); + httpGet.addHeader("Accept", Constants.CT_FHIR_JSON); + + HttpResponse status = ourClient.execute(httpGet); + String responseContent = IOUtils.toString(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(Constants.CT_FHIR_JSON + ";charset=utf-8", status.getFirstHeader("content-type").getValue().replace(" ", "").toLowerCase()); + assertNull(status.getFirstHeader("Content-Disposition")); + assertEquals("{\"resourceType\":\"Binary\",\"id\":\"1\",\"contentType\":\"foo\",\"content\":\"AQIDBA==\"}", responseContent); + + } + @BeforeClass public static void beforeClass() throws Exception { ourPort = PortUtil.findFreePort(); @@ -353,7 +402,7 @@ public class ResponseHighlightingInterceptorTest { ServletHandler proxyHandler = new ServletHandler(); ourServlet = new RestfulServer(ourCtx); ourServlet.registerInterceptor(new ResponseHighlighterInterceptor()); - ourServlet.setResourceProviders(patientProvider); + ourServlet.setResourceProviders(patientProvider, new DummyBinaryResourceProvider()); ourServlet.setBundleInclusionRule(BundleInclusionRule.BASED_ON_RESOURCE_PRESENCE); ServletHolder servletHolder = new ServletHolder(ourServlet); proxyHandler.addServletWithMapping(servletHolder, "/*"); @@ -366,6 +415,34 @@ public class ResponseHighlightingInterceptorTest { ourClient = builder.build(); } + + public static class DummyBinaryResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Binary.class; + } + + @Read + public Binary read(@IdParam IdDt theId) { + Binary retVal = new Binary(); + retVal.setId("1"); + retVal.setContent(new byte[] { 1, 2, 3, 4 }); + retVal.setContentType(theId.getIdPart()); + return retVal; + } + + @Search + public List search() { + Binary retVal = new Binary(); + retVal.setId("1"); + retVal.setContent(new byte[] { 1, 2, 3, 4 }); + retVal.setContentType("text/plain"); + return Collections.singletonList(retVal); + } + + } + /** * Created by dsotnikov on 2/25/2014. diff --git a/src/changes/changes.xml b/src/changes/changes.xml index f8f77944270..7048c0351e2 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -253,7 +253,14 @@ so that different tables use different sequences to generate their indexes, resulting in more sequential resource IDs being assigned by the server - + + + Server now correctly serves up Binary resources + using their native content type (instead of as a + FHIR resource) if the request contains an accept + header containing "application/xml" as some browsers + do. +