Sere up Binary resources as binary content even if the browser puts

application/xml in the Accept header

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<String, EncodingEnum> ourContentTypeToEncoding;
+	private static Map<String, EncodingEnum> ourContentTypeToEncoding;
-	private static HashMap<String, EncodingEnum> ourContentTypeToEncodingStrict;
+	private static Map<String, EncodingEnum> ourContentTypeToEncodingStrict;
 
 	static {
 		ourContentTypeToEncoding = new HashMap<String, EncodingEnum>();
@@ -54,7 +57,7 @@ public enum EncodingEnum {
 		}
 		
 		// Add before we add the lenient ones
-		ourContentTypeToEncodingStrict = new HashMap<String, EncodingEnum>(ourContentTypeToEncoding);
+		ourContentTypeToEncodingStrict = Collections.unmodifiableMap(new HashMap<String, EncodingEnum>(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 <code>null</code> if no encoding
+	 * is found. 
+	 * <p>
+	 * <b>This method is NOT lenient!</b> Things like "application/xml" will return <code>null</code>
+	 * </p>
+	 * @see #forContentType(String)
+	 */
+	public static Map<String, EncodingEnum> getContentTypeToEncodingStrict() {
+		return ourContentTypeToEncodingStrict;
+	}
+
 	public String getFormatContentType() {
 		return myFormatContentType;
 	}

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<String, EncodingEnum> 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()) {

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);

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;

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

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, "/*");
@@ -367,6 +416,34 @@ public class ResponseHighlightingInterceptorTest {
 
 	}
 	
+	public static class DummyBinaryResourceProvider implements IResourceProvider {
+
+		@Override
+		public Class<? extends IResource> 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<Binary> 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.
 	 */

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
-			<action>
+			</action>
+			<action type="fix">
+				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.
+			</action>
 		</release>
 		<release version="1.4" date="2016-02-04">
 			<action type="add">