Return HTTP 400 for missing body on POST and PUT

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/annotation/ResourceParam.java

@@ -9,9 +9,9 @@ package ca.uhn.fhir.rest.annotation;
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -19,10 +19,14 @@ package ca.uhn.fhir.rest.annotation;
  * limitations under the License.
  * #L%
  */
-import java.lang.annotation.*;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
 
 /**
- * Denotes a parameter for a REST method which will contain the resource actually 
+ * Denotes a parameter for a REST method which will contain the resource actually
  * being created/updated/etc in an operation which contains a resource in the HTTP request.
  * <p>
  * For example, in a {@link Create} operation the method parameter annotated with this
@@ -32,7 +36,7 @@ import java.lang.annotation.*;
  * Parameters with this annotation should typically be of the type of resource being
  * operated on (see below for an exception when raw data is required). For example, in a
  * IResourceProvider for Patient resources, the parameter annotated with this
- * annotation should be of type Patient. 
+ * annotation should be of type Patient.
  * </p>
  * <p>
  * Note that in servers it is also acceptable to have parameters with this annotation
@@ -41,8 +45,11 @@ import java.lang.annotation.*;
  * have multiple parameters with this annotation, so you can have one parameter
  * which accepts the parsed resource, and another which accepts the raw request.
  * </p>
+ * <p>
+ * Also note that this parameter may be null if a client does not supply a body.
+ * </p>
  */
-@Target(value=ElementType.PARAMETER)
+@Target(value = ElementType.PARAMETER)
 @Retention(RetentionPolicy.RUNTIME)
 public @interface ResourceParam {
 

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

@@ -89,6 +89,7 @@ ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.failedToCreateWithClientAssignedIdNo
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.invalidParameterChain=Invalid parameter chain: {0}
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.invalidVersion=Version "{0}" is not valid for resource {1}
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.multipleParamsWithSameNameOneIsMissingTrue=This server does not know how to handle multiple "{0}" parameters where one has a value of :missing=true
+ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.missingBody=No body was supplied in request
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.unableToDeleteNotFound=Unable to find resource matching URL "{0}". Deletion failed.
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.successfulCreate=Successfully created resource "{0}" in {1}ms
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.successfulUpdate=Successfully updated resource "{0}" in {1}ms

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java

@@ -142,6 +142,11 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 
 	@Override
 	public DaoMethodOutcome create(T theResource, String theIfNoneExist, boolean thePerformIndexing, Date theUpdateTimestamp, RequestDetails theRequestDetails) {
+		if (theResource == null) {
+			String msg = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "missingBody");
+			throw new InvalidRequestException(msg);
+		}
+
 		if (isNotBlank(theResource.getIdElement().getIdPart())) {
 			if (getContext().getVersion().getVersion().isOlderThan(FhirVersionEnum.DSTU3)) {
 				String message = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "failedToCreateWithClientAssignedId", theResource.getIdElement().getIdPart());
@@ -1270,6 +1275,11 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 
 	@Override
 	public DaoMethodOutcome update(T theResource, String theMatchUrl, boolean thePerformIndexing, boolean theForceUpdateVersion, RequestDetails theRequestDetails) {
+		if (theResource == null) {
+			String msg = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "missingBody");
+			throw new InvalidRequestException(msg);
+		}
+
 		StopWatch w = new StopWatch();
 
 		preProcessResourceForStorage(theResource);

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java

@@ -319,6 +319,38 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 		}
 	}
 
+	@Test
+	public void testUpdateWithNoBody() throws IOException {
+
+		HttpPut httpPost = new HttpPut(ourServerBase + "/Patient/AAA");
+		try (CloseableHttpResponse status = ourHttpClient.execute(httpPost)) {
+			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
+			ourLog.info(status.getStatusLine().toString());
+			ourLog.info(responseContent);
+
+			assertEquals(400, status.getStatusLine().getStatusCode());
+			assertThat(responseContent, containsString("No body was supplied in request"));
+		}
+
+	}
+
+
+	@Test
+	public void testCreateWithNoBody() throws IOException {
+
+		HttpPost httpPost = new HttpPost(ourServerBase + "/Patient");
+		try (CloseableHttpResponse status = ourHttpClient.execute(httpPost)) {
+			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
+			ourLog.info(status.getStatusLine().toString());
+			ourLog.info(responseContent);
+
+			assertEquals(400, status.getStatusLine().getStatusCode());
+			assertThat(responseContent, containsString("No body was supplied in request"));
+		}
+
+	}
+
+
 
 	@Before
 	public void beforeDisableResultReuse() {

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBindingWithResourceParam.java

@@ -25,6 +25,7 @@ import static org.apache.commons.lang3.StringUtils.defaultIfBlank;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 
+import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IIdType;
 
@@ -98,18 +99,20 @@ abstract class BaseOutcomeReturningMethodBindingWithResourceParam extends BaseOu
 		}
 		if (myResourceParameterIndex != -1) {
 			IBaseResource resource = ((IBaseResource) theParams[myResourceParameterIndex]);
-			String resourceId = resource.getIdElement().getIdPart();
-			String urlId = theRequest.getId() != null ? theRequest.getId().getIdPart() : null;
-			if (getContext().getVersion().getVersion().isOlderThan(FhirVersionEnum.DSTU3) == false) {
-				resource.setId(theRequest.getId());
-			}
+			if (resource != null) {
+				String resourceId = resource.getIdElement().getIdPart();
+				String urlId = theRequest.getId() != null ? theRequest.getId().getIdPart() : null;
+				if (getContext().getVersion().getVersion().isOlderThan(FhirVersionEnum.DSTU3) == false) {
+					resource.setId(theRequest.getId());
+				}
 
-			String matchUrl = null;
-			if (myConditionalUrlIndex != -1) {
-				matchUrl = (String) theParams[myConditionalUrlIndex];
-				matchUrl = defaultIfBlank(matchUrl, null);
+				String matchUrl = null;
+				if (myConditionalUrlIndex != -1) {
+					matchUrl = (String) theParams[myConditionalUrlIndex];
+					matchUrl = defaultIfBlank(matchUrl, null);
+				}
+				validateResourceIdAndUrlIdForNonConditionalOperation(resource, resourceId, urlId, matchUrl);
 			}
-			validateResourceIdAndUrlIdForNonConditionalOperation(resource, resourceId, urlId, matchUrl);
 		}
 	}
 

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

@@ -1,7 +1,10 @@
 package ca.uhn.fhir.rest.server;
 
 import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.rest.annotation.Create;
+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.server.exceptions.AuthenticationException;
 import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
@@ -10,14 +13,17 @@ import ca.uhn.fhir.util.PortUtil;
 import ca.uhn.fhir.util.TestUtil;
 import com.google.common.base.Charsets;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.Validate;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.servlet.ServletHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
+import org.hl7.fhir.dstu3.model.IdType;
 import org.hl7.fhir.dstu3.model.OperationOutcome;
 import org.hl7.fhir.dstu3.model.OperationOutcome.IssueType;
 import org.hl7.fhir.dstu3.model.Patient;
@@ -26,6 +32,7 @@ import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -93,6 +100,22 @@ public class ServerExceptionDstu3Test {
 
 	}
 
+	@Test
+	public void testPostWithNoBody() throws IOException {
+
+		HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient");
+		try (CloseableHttpResponse status = ourClient.execute(httpPost)) {
+			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
+			ourLog.info(status.getStatusLine().toString());
+			ourLog.info(responseContent);
+
+			assertEquals(201, status.getStatusLine().getStatusCode());
+			assertThat(status.getFirstHeader("Location").getValue(), containsString("Patient/123"));
+		}
+
+	}
+
+
 	@Test
 	public void testAuthorize() throws Exception {
 
@@ -125,6 +148,12 @@ public class ServerExceptionDstu3Test {
 			throw ourException;
 		}
 
+		@Create()
+		public MethodOutcome create(@ResourceParam Patient thePatient) {
+			Validate.isTrue(thePatient == null);
+			return new MethodOutcome().setId(new IdType("Patient/123"));
+		}
+
 	}
 
 	@AfterClass

pom.xml

@@ -1464,7 +1464,7 @@
 				<plugin>
 					<groupId>org.apache.maven.plugins</groupId>
 					<artifactId>maven-failsafe-plugin</artifactId>
-					<version>2.21.0</version>
+					<version>3.0.0-M3</version>
 				</plugin>
 				<plugin>
 					<groupId>org.apache.maven.plugins</groupId>
@@ -1491,7 +1491,7 @@
 				<plugin>
 					<groupId>org.apache.maven.plugins</groupId>
 					<artifactId>maven-surefire-plugin</artifactId>
-					<version>2.21.0</version>
+					<version>3.0.0-M3</version>
 					<configuration>
 						<redirectTestOutputToFile>true</redirectTestOutputToFile>
 						<runOrder>random</runOrder>

src/changes/changes.xml

@@ -137,6 +137,11 @@
 				the resource version is not updated and no new version is created. In this situation,
 				the update time was modified however. It will no longer be updated.
 			</action>
+			<action type="fix">
+				Performing a PUT or POST against a HAPI FHIR Server with no request body caused an
+				HTTP 500 to be returned instead of a more appropriate HTTP 400. This has been
+				corrected.
+			</action>
 		</release>
 		<release version="3.7.0" date="2019-02-06" description="Gale">
 			<action type="add">