Fix #342 - The HTTP 400 if request fail to parse

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ResourceParameter.java

@@ -42,6 +42,7 @@ import ca.uhn.fhir.context.FhirVersionEnum;
 import ca.uhn.fhir.model.api.IResource;
 import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum;
 import ca.uhn.fhir.model.api.TagList;
+import ca.uhn.fhir.parser.DataFormatException;
 import ca.uhn.fhir.parser.IParser;
 import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
 import ca.uhn.fhir.rest.method.BaseMethodBinding;
@@ -180,12 +181,17 @@ public class ResourceParameter implements IParameter {
 		IParser parser = encoding.newParser(ctx);
 
 		T retVal;
-		if (theResourceType != null) {
+		try {
-			retVal = parser.parseResource(theResourceType, requestReader);
+			if (theResourceType != null) {
-		} else {
+				retVal = parser.parseResource(theResourceType, requestReader);
-			retVal = (T) parser.parseResource(requestReader);
+			} else {
+				retVal = (T) parser.parseResource(requestReader);
+			}
+		} catch (DataFormatException e) {
+			String msg = ctx.getLocalizer().getMessage(ResourceParameter.class, "failedToParseRequest", encoding.name(), e.getMessage());
+			throw new InvalidRequestException(msg);
 		}
-
+		
 		if (theRequest.getServer().getFhirContext().getVersion().getVersion().equals(FhirVersionEnum.DSTU1)) {
 			TagList tagList = new TagList();
 			for (String nextTagComplete : theRequest.getHeaders(Constants.HEADER_CATEGORY)) {

hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties

@@ -33,6 +33,7 @@ ca.uhn.fhir.rest.method.SummaryEnumParameter.cantCombineText=Can not combine _su
 
 ca.uhn.fhir.rest.param.ResourceParameter.invalidContentTypeInRequest=Incorrect Content-Type header value of "{0}" was provided in the request. A FHIR Content-Type is required for "{1}" operation
 ca.uhn.fhir.rest.param.ResourceParameter.noContentTypeInRequest=No Content-Type header was provided in the request. This is required for "{0}" operation
+ca.uhn.fhir.rest.param.ResourceParameter.failedToParseRequest=Failed to parse request body as {0} resource. Error was: {1} 
 
 ca.uhn.fhir.parser.ParserState.wrongResourceTypeFound=Incorrect resource type found, expected "{0}" but found "{1}"
 

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java

@@ -166,7 +166,6 @@ public class JsonParserDstu3Test {
 		assertEquals("CHILD", ((StringType) given2ext2.getValue()).getValue());
 
 	}
-	
 
 	@Test
 	public void testEncodeAndParseMetaProfileAndTags() {
@@ -245,6 +244,7 @@ public class JsonParserDstu3Test {
 		assertEquals("sec_term2", tagList.get(1).getCode());
 		assertEquals("sec_label2", tagList.get(1).getDisplay());
 	}
+	
 
 	/**
 	 * See #336
@@ -359,7 +359,6 @@ public class JsonParserDstu3Test {
 		assertEquals("DISPLAY2", label.getDisplay());
 		assertEquals("VERSION2", label.getVersion());
 	}
-	
 
 	@Test
 	public void testEncodeBundleNewBundleNoText() {
@@ -379,6 +378,7 @@ public class JsonParserDstu3Test {
 
 	}
 	
+
 	/**
 	 * See #326
 	 */
@@ -414,7 +414,6 @@ public class JsonParserDstu3Test {
 		));
 		//@formatter:on
 	}
-
 	
 	@Test
 	public void testEncodeDoesntIncludeUuidId() {
@@ -426,13 +425,14 @@ public class JsonParserDstu3Test {
 		assertThat(actual, not(containsString("78ef6f64c2f2")));
 	}
 
-
+	
 	@Test
 	public void testEncodeEmptyBinary() {
 		String output = ourCtx.newJsonParser().encodeResourceToString(new Binary());
 		assertEquals("{\"resourceType\":\"Binary\"}", output);
 	}
-	
+
+
 	/**
 	 * #158
 	 */
@@ -448,7 +448,6 @@ public class JsonParserDstu3Test {
 		String encoded = ourCtx.newJsonParser().encodeResourceToString(p);
 		assertThat(encoded, not(containsString("tag")));
 	}
-
 	
 	/**
 	 * #158
@@ -467,6 +466,7 @@ public class JsonParserDstu3Test {
 		assertThat(encoded, containsString("scheme"));
 		assertThat(encoded, not(containsString("Label")));
 	}
+
 	
 	@Test
 	public void testEncodeExtensionInPrimitiveElement() {
@@ -496,7 +496,7 @@ public class JsonParserDstu3Test {
 		assertEquals(encoded, "{\"resourceType\":\"Conformance\",\"acceptUnknown\":\"elements\",\"_acceptUnknown\":{\"extension\":[{\"url\":\"http://foo\",\"valueString\":\"AAA\"}]}}");
 
 	}
-
+	
 	@Test
 	public void testEncodeExtensionUndeclaredNonModifier() {
 		Observation obs = new Observation();
@@ -608,7 +608,7 @@ public class JsonParserDstu3Test {
 
 		assertThat(enc, containsString("\"valueId\":\"1\""));
 	}
-	
+
 	@Test
 	public void testEncodeSummary() {
 		Patient patient = new Patient();
@@ -629,8 +629,7 @@ public class JsonParserDstu3Test {
 		assertThat(encoded, containsString("family"));
 		assertThat(encoded, not(containsString("maritalStatus")));
 	}
-
+	
-
 	@Test
 	public void testEncodeSummary2() {
 		Patient patient = new Patient();
@@ -651,6 +650,7 @@ public class JsonParserDstu3Test {
 		assertThat(encoded, not(containsString("maritalStatus")));
 	}
 
+
 	/**
 	 * See #205
 	 */
@@ -1249,6 +1249,14 @@ public class JsonParserDstu3Test {
 		assertEquals("a2", family1.getId());
 	}
 
+	/**
+	 * See #342
+	 */
+	@Test(expected=DataFormatException.class)
+	public void testParseInvalid() {
+		ourCtx.newJsonParser().parseResource("FOO");
+	}
+
 	@Test
 	public void testParseMetadata() throws Exception {
 		//@formatter:off

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java

@@ -168,7 +168,7 @@ public class XmlParserDstu3Test {
 		o = (Organization) rr.getResource();
 		assertEquals("ORG", o.getName());
 	}
-
+	
 	@Test
 	public void testDuration() {
 		Encounter enc = new Encounter();
@@ -1179,8 +1179,6 @@ public class XmlParserDstu3Test {
 		
 	}
 
-	
-
 	/**
 	 * See #312
 	 */
@@ -1200,6 +1198,8 @@ public class XmlParserDstu3Test {
 		assertEquals("<Patient xmlns=\"http://hl7.org/fhir\"><extension url=\"http://hello.world\"><valueString value=\"Hello World\"/></extension></Patient>", xml);
 	}
 
+	
+
 	@Test
 	public void testEncodeReferenceUsingUnqualifiedResourceWorksCorrectly() {
 		
@@ -1254,7 +1254,7 @@ public class XmlParserDstu3Test {
 		assertThat(encoded, containsString("family"));
 		assertThat(encoded, not(containsString("maritalStatus")));
 	}
-	
+
 	@Test
 	public void testEncodeSummary2() {
 		Patient patient = new Patient();
@@ -1275,7 +1275,7 @@ public class XmlParserDstu3Test {
 		assertThat(encoded, containsString("family"));
 		assertThat(encoded, not(containsString("maritalStatus")));
 	}
-
+	
 	@Test
 	public void testEncodeUndeclaredExtensionWithEnumerationContent() {
 		IParser parser = ourCtx.newXmlParser();
@@ -2343,6 +2343,14 @@ public class XmlParserDstu3Test {
 		assertNotNull(((Reference) actual.getContent().get(0).getP()).getResource());
 	}
 
+	/**
+	 * See #342
+	 */
+	@Test(expected=DataFormatException.class)
+	public void testParseInvalid() {
+		ourCtx.newXmlParser().parseResource("FOO");
+	}
+
 	@Test
 	public void testParseInvalidTextualNumber() {
 		Observation obs = new Observation();

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/CreateDstu3Test.java

@@ -13,6 +13,9 @@ import java.util.concurrent.TimeUnit;
 import org.apache.commons.io.IOUtils;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
@@ -28,9 +31,12 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 
 import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.rest.annotation.Create;
 import ca.uhn.fhir.rest.annotation.IdParam;
 import ca.uhn.fhir.rest.annotation.Read;
+import ca.uhn.fhir.rest.annotation.ResourceParam;
 import ca.uhn.fhir.rest.annotation.Search;
+import ca.uhn.fhir.rest.api.MethodOutcome;
 import ca.uhn.fhir.rest.client.MyPatientWithExtensions;
 import ca.uhn.fhir.util.PortUtil;
 import ca.uhn.fhir.util.TestUtil;
@@ -70,6 +76,25 @@ public class CreateDstu3Test {
 		//@formatter:on
 	}
 
+	/**
+	 * #342
+	 */
+	@Test
+	public void testCreateWithInvalidContent() throws Exception {
+		HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient");
+		httpPost.setEntity(new StringEntity("FOO", ContentType.parse("application/xml+fhir; charset=utf-8")));
+		HttpResponse status = ourClient.execute(httpPost);
+
+		String responseContent = IOUtils.toString(status.getEntity().getContent());
+		IOUtils.closeQuietly(status.getEntity().getContent());
+
+		ourLog.info("Response was:\n{}", responseContent);
+
+		assertEquals(400, status.getStatusLine().getStatusCode());
+		String expected = "<OperationOutcome xmlns=\"http://hl7.org/fhir\"><issue><severity value=\"error\"/><code value=\"processing\"/><diagnostics value=\"Failed to parse request body as XML resource. Error was: com.ctc.wstx.exc.WstxUnexpectedCharException: Unexpected character 'F' (code 70) in prolog; expected '&lt;'&#xa; at [row,col {unknown-source}]: [1,1]\"/></issue></OperationOutcome>";
+		assertEquals(expected, responseContent);
+		
+	}
 
 	@Test
 	public void testSearch() throws Exception {
@@ -144,6 +169,11 @@ public class CreateDstu3Test {
 			return p0;
 		}
 
+		@Create()
+		public MethodOutcome read(@ResourceParam Patient theIdParam) {
+			return new MethodOutcome(new IdType("Patient", "1"), true);
+		}
+
 		@Search
 		public List<IBaseResource> search() {
 			ArrayList<IBaseResource> retVal = new ArrayList<IBaseResource>();

src/changes/changes.xml

@@ -11,6 +11,11 @@
 				ResponseValidatingInterceptor threw an InternalErrorException (HTTP 500) for operations
 				that do not return any content (e.g. delete). Thanks to Mohammad Jafari for reporting!
 			</action>
+			<action type="fix" issue="342">
+				REST server now throws an HTTP 400 instead of an HTTP 500 if an operation which takes
+				a FHIR resource in the request body (e.g. create, update) contains invalid content that
+				the parser is unable to parse. Thanks to Jim Steel for the suggestion!				
+			</action>
 		</release>
 		<release version="1.5" date="2016-04-20">
 			<action type="fix" issue="339">