From 3c4c6f7925b35c8684748e92317e7a504741815a Mon Sep 17 00:00:00 2001
From: James Agnew <jamesagnew@gmail.com>
Date: Fri, 4 Oct 2019 09:55:14 -0400
Subject: [PATCH] Don't allow Content-Location header to dictate version for
 updates

---
 .../BaseOutcomeReturningMethodBinding.java    | 24 ++++++------------
 .../server/method/UpdateMethodBinding.java    | 25 -------------------
 src/changes/changes.xml                       |  6 +++++
 3 files changed, 14 insertions(+), 41 deletions(-)

diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java
index 5a4e95c5543..e6112605cc9 100644
--- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java
+++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java
@@ -78,9 +78,7 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
 	private int getOperationStatus(MethodOutcome response) {
 		switch (getRestOperationType()) {
 			case CREATE:
-				if (response == null) {
-					throw new InternalErrorException("Method " + getMethod().getName() + " in type " + getMethod().getDeclaringClass().getCanonicalName() + " returned null, which is not allowed for create operation");
-				}
+				validateResponseNotNullIfItShouldntBe(response);
 				if (response.getCreated() == null || Boolean.TRUE.equals(response.getCreated())) {
 					return Constants.STATUS_HTTP_201_CREATED;
 				}
@@ -95,10 +93,8 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
 			case VALIDATE:
 			case DELETE:
 			default:
+				validateResponseNotNullIfItShouldntBe(response);
 				if (response == null) {
-					if (isReturnVoid() == false) {
-						throw new InternalErrorException("Method " + getMethod().getName() + " in type " + getMethod().getDeclaringClass().getCanonicalName() + " returned null");
-					}
 					return Constants.STATUS_HTTP_204_NO_CONTENT;
 				}
 				if (response.getOperationOutcome() == null) {
@@ -108,6 +104,12 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
 		}
 	}
 
+	private void validateResponseNotNullIfItShouldntBe(MethodOutcome response) {
+		if (response == null && !isReturnVoid()) {
+			throw new InternalErrorException("Method " + getMethod().getName() + " in type " + getMethod().getDeclaringClass().getCanonicalName() + " returned null");
+		}
+	}
+
 	@Override
 	public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) {
 		Set<RequestTypeEnum> allowableRequestTypes = provideAllowableRequestTypes();
@@ -239,14 +241,4 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
 		return restfulResponse.streamResponseAsResource(responseDetails.getResponseResource(), prettyPrint, summaryMode, responseDetails.getResponseCode(), null, theRequest.isRespondGzip(), true);
 	}
 
-	protected static void parseContentLocation(FhirContext theContext, MethodOutcome theOutcomeToPopulate, String theLocationHeader) {
-		if (StringUtils.isBlank(theLocationHeader)) {
-			return;
-		}
-
-		IIdType id = theContext.getVersion().newIdType();
-		id.setValue(theLocationHeader);
-		theOutcomeToPopulate.setId(id);
-	}
-
 }
diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/UpdateMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/UpdateMethodBinding.java
index e349f33df2b..9c29a187141 100644
--- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/UpdateMethodBinding.java
+++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/UpdateMethodBinding.java
@@ -49,38 +49,13 @@ public class UpdateMethodBinding extends BaseOutcomeReturningMethodBindingWithRe
 
 	@Override
 	protected void addParametersForServerRequest(RequestDetails theRequest, Object[] theParams) {
-		/*
-		 * We are being a bit lenient here, since technically the client is supposed to include the version in the
-		 * Content-Location header, but we allow it in the PUT URL as well..
-		 */
-		String locationHeader = theRequest.getHeader(Constants.HEADER_CONTENT_LOCATION);
 		IIdType id = theRequest.getId();
-		if (isNotBlank(locationHeader)) {
-			id.setValue(locationHeader);
-			if (isNotBlank(id.getResourceType())) {
-				if (!getResourceName().equals(id.getResourceType())) {
-					throw new InvalidRequestException(
-							"Attempting to update '" + getResourceName() + "' but content-location header specifies different resource type '" + id.getResourceType() + "' - header value: " + locationHeader);
-				}
-			}
-		}
-
 		id = applyETagAsVersion(theRequest, id);
-
 		if (theRequest.getId() != null && theRequest.getId().hasVersionIdPart() == false) {
 			if (id != null && id.hasVersionIdPart()) {
 				theRequest.getId().setValue(id.getValue());
 			}
 		}
-
-		if (isNotBlank(locationHeader)) {
-			MethodOutcome mo = new MethodOutcome();
-			parseContentLocation(getContext(), mo, locationHeader);
-			if (mo.getId() == null || mo.getId().isEmpty()) {
-				throw new InvalidRequestException("Invalid Content-Location header for resource " + getResourceName() + ": " + locationHeader);
-			}
-		}
-
 		super.addParametersForServerRequest(theRequest, theParams);
 	}
 
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index e3709ab6f6e..bad68af70cc 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -334,6 +334,12 @@
 				The JPA server did not correctly process _has queries where the linked search parameter was
 				the _id parameter.
 			</action>
+			<action type="remove">
+				HTTP PUT (resource update) operations will no longer allow the version to be specified in a
+				Content-Location header. This behaviour was allowed in DSTU1 and was never removed from HAPI even though
+				it hasn't been permitted in the spec for a very long time. Hopefully this change will not
+				impact anyone!
+			</action>
 		</release>
 		<release version="4.0.3" date="2019-09-03" description="Igloo (Point Release)">
 			<action type="fix">