Fix #305 - Don't allow update resource body IDs to be missing or incorrect

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

@@ -186,10 +186,6 @@ public class ResourceParameter implements IParameter {
 			retVal = (T) parser.parseResource(requestReader);
 		}
 
-		if (theRequest.getId() != null && theRequest.getId().hasIdPart()) {
-			retVal.setId(theRequest.getId());
-		}
-
 		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

@@ -60,6 +60,8 @@ ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.failedToCreateWithInvalidId=Can not
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.incorrectResourceType=Incorrect resource type detected for endpoint, found {0} but expected {1}
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.failedToCreateWithClientAssignedNumericId=Can not create resource with ID[{0}], no resource with this ID exists and clients may only assign IDs which contain at least one non-numeric character
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.failedToCreateWithClientAssignedId=Can not create resource with ID[{0}], ID must not be supplied on a create (POST) operation
+ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.incorrectIdForUpdate=Update failed: resource body ID of "{0}" does not match URL ID of "{1}"
+ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.missingIdForUpdate=Update failed: resource body does not have an ID
 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

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

@@ -954,6 +954,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 
 		preProcessResourceForStorage(theResource);
 
+		
 		final ResourceTable entity;
 
 		IIdType resourceId;
@@ -972,8 +973,14 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 		} else {
 			resourceId = theResource.getIdElement();
 			if (resourceId == null || isBlank(resourceId.getIdPart())) {
-				throw new InvalidRequestException("Can not update a resource with no ID");
+				String msg = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "missingIdForUpdate");
+				throw new InvalidRequestException(msg);
 			}
+			if (!theResource.getIdElement().getIdPart().equals(theRequestDetails.getId().getIdPart())) {
+				String msg = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "incorrectIdForUpdate", theResource.getIdElement().getIdPart(), theRequestDetails.getId().getIdPart());
+				throw new InvalidRequestException(msg);
+			}
+
 			try {
 				entity = readEntityLatestVersion(resourceId);
 			} catch (ResourceNotFoundException e) {

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/JpaResourceProviderDstu3.java

@@ -146,7 +146,6 @@ public class JpaResourceProviderDstu3<T extends IAnyResource> extends BaseJpaRes
 			if (theConditional != null) {
 				return getDao().update(theResource, theConditional, theRequestDetails);
 			} else {
-				theResource.setId(theId);
 				return getDao().update(theResource, theRequestDetails);
 			}
 		} finally {

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java

@@ -66,6 +66,7 @@ import org.hl7.fhir.dstu3.model.MedicationOrder;
 import org.hl7.fhir.dstu3.model.Meta;
 import org.hl7.fhir.dstu3.model.Narrative.NarrativeStatus;
 import org.hl7.fhir.dstu3.model.Observation;
+import org.hl7.fhir.dstu3.model.OperationOutcome;
 import org.hl7.fhir.dstu3.model.Organization;
 import org.hl7.fhir.dstu3.model.Parameters;
 import org.hl7.fhir.dstu3.model.Patient;
@@ -1996,7 +1997,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 			.where(Patient.IDENTIFIER.exactly().systemAndCode("urn:system", methodName))
 			.sort().ascending(Patient.FAMILY)
 			.sort().ascending(Patient.GIVEN)
-			.limitTo(100)
+			.count(100)
 			.returnBundle(Bundle.class)
 			.execute();
 		//@formatter:on
@@ -2132,6 +2133,65 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 
 	}
 
+	@Test
+	public void testUpdateRejectsIncorrectIds() throws Exception {
+
+		Patient p1 = new Patient();
+		p1.addIdentifier().setSystem("urn:system").setValue("testUpdateRejectsInvalidTypes");
+		p1.addName().addFamily("Tester").addGiven("testUpdateRejectsInvalidTypes");
+		IdType p1id = (IdType) ourClient.create().resource(p1).execute().getId();
+
+		// Try to update with the wrong ID in the resource body
+		p1.setId("FOO");
+		p1.addAddress().addLine("NEWLINE");
+		
+		String encoded = myFhirCtx.newJsonParser().encodeResourceToString(p1);
+		ourLog.info(encoded);
+		
+		HttpPut put = new HttpPut(ourServerBase + "/Patient/" + p1id.getIdPart());
+		put.setEntity(new StringEntity(encoded, ContentType.create(Constants.CT_FHIR_JSON, "UTF-8")));
+		put.addHeader("Accept", Constants.CT_FHIR_JSON);
+		CloseableHttpResponse response = ourHttpClient.execute(put);
+		try {
+			assertEquals(400, response.getStatusLine().getStatusCode());
+			OperationOutcome oo = myFhirCtx.newJsonParser().parseResource(OperationOutcome.class, new InputStreamReader(response.getEntity().getContent()));
+			assertEquals("Update failed: resource body ID of \"FOO\" does not match URL ID of \"" + p1id.getIdPart() + "\"", oo.getIssue().get(0).getDiagnostics());
+		} finally {
+			response.close();
+		}
+		
+		// Try to update with the no ID in the resource body
+		p1.setId((String)null);
+		
+		encoded = myFhirCtx.newJsonParser().encodeResourceToString(p1);
+		put = new HttpPut(ourServerBase + "/Patient/" + p1id.getIdPart());
+		put.setEntity(new StringEntity(encoded, ContentType.create(Constants.CT_FHIR_JSON, "UTF-8")));
+		put.addHeader("Accept", Constants.CT_FHIR_JSON);
+		response = ourHttpClient.execute(put);
+		try {
+			assertEquals(400, response.getStatusLine().getStatusCode());
+			OperationOutcome oo = myFhirCtx.newJsonParser().parseResource(OperationOutcome.class, new InputStreamReader(response.getEntity().getContent()));
+			assertEquals("Update failed: resource body does not have an ID", oo.getIssue().get(0).getDiagnostics());
+		} finally {
+			response.close();
+		}
+
+		// Try to update with the to correct ID in the resource body
+		p1.setId(p1id.getIdPart());
+		
+		encoded = myFhirCtx.newJsonParser().encodeResourceToString(p1);
+		put = new HttpPut(ourServerBase + "/Patient/" + p1id.getIdPart());
+		put.setEntity(new StringEntity(encoded, ContentType.create(Constants.CT_FHIR_JSON, "UTF-8")));
+		response = ourHttpClient.execute(put);
+		try {
+			assertEquals(200, response.getStatusLine().getStatusCode());
+		} finally {
+			response.close();
+		}
+
+	}
+
+	
 	@Test
 	public void testUpdateResourceConditional() throws IOException {
 		String methodName = "testUpdateResourceConditional";

src/changes/changes.xml

@@ -206,6 +206,12 @@
 				JPA server returned the wrong Bundle.type value (COLLECTION, should be SEARCHSET)
 				for $everything operation responses. Thanks to Sonali Somase for reporting!				
 			</action>
+			<action type="fix" issue="305">
+				JPA server should reject update requests where the resource body does not
+				contain an ID, or contains an ID which does not match the URL. Previously these
+				were accepted (the URL ID was trusted) which is incorrect according to the
+				FHIR specification. Thanks to GitHub user ametke for reporting!				
+			</action>
 		</release>
 		<release version="1.4" date="2016-02-04">
 			<action type="add">