5123 - Use DEFAULT partition for server-based requests if none specified

hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlPathTokenizer.java

@@ -19,18 +19,45 @@
  */
 package ca.uhn.fhir.util;
 
-import java.util.StringTokenizer;
+import ca.uhn.fhir.i18n.Msg;
+import org.apache.commons.lang3.ArrayUtils;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+
+import static org.apache.commons.lang3.StringUtils.isBlank;
 
 public class UrlPathTokenizer {
 
-	private final StringTokenizer myTok;
+	private String[] tokens;
+	private int curPos;
 
 	public UrlPathTokenizer(String theRequestPath) {
-		myTok = new StringTokenizer(theRequestPath, "/");
+		if (theRequestPath == null) {
+			theRequestPath = "";
+		}
+		tokens = removeBlanksAndSanitize(theRequestPath.split("/"));
+		curPos = 0;
 	}
 
 	public boolean hasMoreTokens() {
-		return myTok.hasMoreTokens();
+		return curPos < tokens.length;
+	}
+
+	public int countTokens() {
+		return tokens.length;
+	}
+
+	/**
+	 * Returns the next token without updating the current position.
+	 * Will throw NoSuchElementException if there are no more tokens.
+	 */
+	public String peek() {
+		if (!hasMoreTokens()) {
+			throw new NoSuchElementException(Msg.code(2420) + "Attempt to retrieve URL token out of bounds");
+		}
+		return tokens[curPos];
 	}
 
 	/**
@@ -43,6 +70,22 @@ public boolean hasMoreTokens() {
 	 * @see UrlUtil#unescape(String)
 	 */
 	public String nextTokenUnescapedAndSanitized() {
-		return UrlUtil.sanitizeUrlPart(UrlUtil.unescape(myTok.nextToken()));
+		String token = peek();
+		curPos++;
+		return token;
+	}
+
+	/**
+	 * Given an array of Strings, this method will return all the non-blank entries in that
+	 * array, after running sanitizeUrlPart() and unescape() on them.
+	 */
+	private static String[] removeBlanksAndSanitize(String[] theInput) {
+		List<String> output = new ArrayList<>();
+		for (String s : theInput) {
+			if (!isBlank(s)) {
+				output.add(UrlUtil.sanitizeUrlPart(UrlUtil.unescape(s)));
+			}
+		}
+		return output.toArray(ArrayUtils.EMPTY_STRING_ARRAY);
 	}
 }

hapi-fhir-server-cds-hooks/src/test/java/ca/uhn/hapi/fhir/cdshooks/svc/CdsHooksContextBooterTest.java

@@ -28,7 +28,7 @@ void validateJsonReturnsNullWhenInputIsEmptyString() {
 	@Test
 	void validateJsonThrowsExceptionWhenInputIsInvalid() {
 		// setup
-		final String expected = "Invalid JSON: Unrecognized token 'abc': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')\n" +
+		final String expected = "HAPI-2378: Invalid JSON: Unrecognized token 'abc': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')\n" +
 			" at [Source: (String)\"abc\"; line: 1, column: 4]";
 		// execute
 		final UnprocessableEntityException actual = assertThrows(UnprocessableEntityException.class, () -> myFixture.validateJson("abc"));

hapi-fhir-server-cds-hooks/src/test/java/ca/uhn/hapi/fhir/cdshooks/svc/prefetch/CdsPrefetchFhirClientSvcTest.java

@@ -93,7 +93,7 @@ void testParseResourceWithNoResourceType() {
 			IBaseResource srq = myCdsPrefetchFhirClientSvc.resourceFromUrl(cdsServiceRequestJson, "1234");
 			fail("should throw, no resource present");
 		} catch (InvalidRequestException e) {
-			assertEquals("Unable to translate url 1234 into a resource or a bundle.", e.getMessage());
+			assertEquals("HAPI-2384: Unable to translate url 1234 into a resource or a bundle.", e.getMessage());
 		}
 	}
 
@@ -106,7 +106,7 @@ void testParseResourceWithNoResourceTypeAndSlash() {
 			IBaseResource srq = myCdsPrefetchFhirClientSvc.resourceFromUrl(cdsServiceRequestJson, "/1234");
 			fail("should throw, no resource present");
 		} catch (InvalidRequestException e) {
-			assertEquals("Failed to resolve /1234. Url does not start with a resource type.", e.getMessage());
+			assertEquals("HAPI-2383: Failed to resolve /1234. Url does not start with a resource type.", e.getMessage());
 		}
 	}
 }

hapi-fhir-server-cds-hooks/src/test/java/ca/uhn/hapi/fhir/cdshooks/svc/prefetch/PrefetchTemplateUtilTest.java

@@ -36,7 +36,7 @@ public void testShouldThrowForMissingPrefetchTokens() {
 			PrefetchTemplateUtil.substituteTemplate(template, context, FhirContext.forR4());
 			fail();
 		} catch (InvalidRequestException e) {
-			assertEquals("Either request context was empty or it did not provide a value for key <userId>.  Please make sure you are including a context with valid keys.", e.getMessage());
+			assertEquals("HAPI-2375: Either request context was empty or it did not provide a value for key <userId>.  Please make sure you are including a context with valid keys.", e.getMessage());
 		}
 	}
 
@@ -50,7 +50,7 @@ public void testShouldThrow400ForMissingContext() {
 			PrefetchTemplateUtil.substituteTemplate(template, context, FhirContext.forR4());
 			fail();
 		} catch (InvalidRequestException e) {
-			assertEquals("Either request context was empty or it did not provide a value for key <userId>.  Please make sure you are including a context with valid keys.", e.getMessage());
+			assertEquals("HAPI-2375: Either request context was empty or it did not provide a value for key <userId>.  Please make sure you are including a context with valid keys.", e.getMessage());
 		}
 	}
 
@@ -63,7 +63,7 @@ public void testShouldThrowForMissingNestedPrefetchTokens() {
 			PrefetchTemplateUtil.substituteTemplate(template, context, FhirContext.forR4());
 			fail();
 		} catch (InvalidRequestException e) {
-			assertEquals("Request context did not provide a value for key <draftOrders>.  Available keys in context are: [patientId]", e.getMessage());
+			assertEquals("HAPI-2372: Request context did not provide a value for key <draftOrders>.  Available keys in context are: [patientId]", e.getMessage());
 		}
 	}
 
@@ -119,7 +119,7 @@ public void testShouldThrowForDaVinciTemplateIfResourcesAreNotFoundInContextForR
 			PrefetchTemplateUtil.substituteTemplate(template, context, FhirContext.forR4());
 			fail("substituteTemplate call was successful with a null context field.");
 		} catch (InvalidRequestException e) {
-			assertEquals("Request context did not provide for resource(s) matching template. ResourceType missing is: ServiceRequest", e.getMessage());
+			assertEquals("HAPI-2373: Request context did not provide for resource(s) matching template. ResourceType missing is: ServiceRequest", e.getMessage());
 		}
 	}
 
@@ -134,7 +134,7 @@ public void testShouldThrowForDaVinciTemplateIfResourceIsNotBundle() {
 			PrefetchTemplateUtil.substituteTemplate(template, context, FhirContext.forR4());
 			fail();
 		} catch (InvalidRequestException e) {
-			assertEquals("Request context did not provide valid " + fhirContextR4.getVersion().getVersion() + " Bundle resource for template key <draftOrders>" , e.getMessage());
+			assertEquals("HAPI-2374: Request context did not provide valid " + fhirContextR4.getVersion().getVersion() + " Bundle resource for template key <draftOrders>", e.getMessage());
 		}
 	}
 

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/tenant/UrlBaseTenantIdentificationStrategy.java

@@ -22,10 +22,10 @@
 import ca.uhn.fhir.i18n.HapiLocalizer;
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.rest.api.server.RequestDetails;
+import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
 import ca.uhn.fhir.rest.server.RestfulServer;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.util.UrlPathTokenizer;
-import org.apache.commons.lang3.Validate;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -51,12 +51,41 @@ public class UrlBaseTenantIdentificationStrategy implements ITenantIdentificatio
 	@Override
 	public void extractTenant(UrlPathTokenizer theUrlPathTokenizer, RequestDetails theRequestDetails) {
 		String tenantId = null;
-		if (theUrlPathTokenizer.hasMoreTokens()) {
-			tenantId = defaultIfBlank(theUrlPathTokenizer.nextTokenUnescapedAndSanitized(), null);
-			ourLog.trace("Found tenant ID {} in request string", tenantId);
-			theRequestDetails.setTenantId(tenantId);
+		boolean isSystemRequest = (theRequestDetails instanceof SystemRequestDetails);
+
+		// If we were given no partition for a system request, use DEFAULT:
+		if (!theUrlPathTokenizer.hasMoreTokens()) {
+			if (isSystemRequest) {
+				tenantId = "DEFAULT";
+				theRequestDetails.setTenantId(tenantId);
+				ourLog.trace("No tenant ID found for system request; using DEFAULT.");
+			}
 		}
 
+		// We were given at least one URL token:
+		else {
+
+			// peek() won't consume this token:
+			tenantId = defaultIfBlank(theUrlPathTokenizer.peek(), null);
+
+			// If it's "metadata" or starts with "$", use DEFAULT partition and don't consume this token:
+			if (tenantId != null && (tenantId.equals("metadata") || tenantId.startsWith("$"))) {
+				tenantId = "DEFAULT";
+				theRequestDetails.setTenantId(tenantId);
+				ourLog.trace("No tenant ID found for metadata or system request; using DEFAULT.");
+			}
+
+			// It isn't metadata or $, so assume that this first token is the partition name and consume it:
+			else {
+				tenantId = defaultIfBlank(theUrlPathTokenizer.nextTokenUnescapedAndSanitized(), null);
+				if (tenantId != null) {
+					theRequestDetails.setTenantId(tenantId);
+					ourLog.trace("Found tenant ID {} in request string", tenantId);
+				}
+			}
+		}
+
+		// If we get to this point without a tenant, it's an invalid request:
 		if (tenantId == null) {
 			HapiLocalizer localizer =
 					theRequestDetails.getServer().getFhirContext().getLocalizer();
@@ -67,7 +96,10 @@ public void extractTenant(UrlPathTokenizer theUrlPathTokenizer, RequestDetails t
 
 	@Override
 	public String massageServerBaseUrl(String theFhirServerBase, RequestDetails theRequestDetails) {
-		Validate.notNull(theRequestDetails.getTenantId(), "theTenantId is not populated on this request");
-		return theFhirServerBase + '/' + theRequestDetails.getTenantId();
+		String result = theFhirServerBase;
+		if (theRequestDetails.getTenantId() != null) {
+			result += "/" + theRequestDetails.getTenantId();
+		}
+		return result;
 	}
 }

hapi-fhir-storage-cr/pom.xml

@@ -212,7 +212,7 @@
 		<dependency>
 			<groupId>io.specto</groupId>
 			<artifactId>hoverfly-java-junit5</artifactId>
-			<version>0.14.3</version>
+			<version>0.14.4</version>
 			<scope>test</scope>
 		</dependency>
 		<dependency>

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/UrlPathTokenizerTest.java

@@ -0,0 +1,78 @@
+package ca.uhn.fhir.util;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class UrlPathTokenizerTest {
+
+	@Test
+	void urlPathTokenizer_withValidPath_tokenizesCorrectly() {
+		UrlPathTokenizer tokenizer = new UrlPathTokenizer("/root/subdir/subsubdir/file.html");
+		assertTrue(tokenizer.hasMoreTokens());
+		assertEquals(4, tokenizer.countTokens());
+		assertEquals("root", tokenizer.nextTokenUnescapedAndSanitized());
+		assertEquals("subdir", tokenizer.nextTokenUnescapedAndSanitized());
+		assertEquals("subsubdir", tokenizer.nextTokenUnescapedAndSanitized());
+		assertEquals("file.html", tokenizer.nextTokenUnescapedAndSanitized());
+		assertFalse(tokenizer.hasMoreTokens());
+	}
+
+	@ParameterizedTest
+	@ValueSource(strings = {
+		"",               // actually empty
+		"///////",        // effectively empty
+		"//  / / /  /   " // effectively empty with extraneous whitespace
+	})
+	void urlPathTokenizer_withEmptyPath_returnsEmpty(String thePath) {
+		UrlPathTokenizer tokenizer = new UrlPathTokenizer(thePath);
+		assertEquals(0, tokenizer.countTokens());
+	}
+
+	@Test
+	void urlPathTokenizer_withNullPath_returnsEmpty() {
+		UrlPathTokenizer tokenizer = new UrlPathTokenizer(null);
+		assertEquals(0, tokenizer.countTokens());
+	}
+
+	@Test
+	void urlPathTokenizer_withSinglePathElement_returnsSingleToken() {
+		UrlPathTokenizer tokenizer = new UrlPathTokenizer("hello");
+		assertTrue(tokenizer.hasMoreTokens());
+		assertEquals("hello", tokenizer.nextTokenUnescapedAndSanitized());
+	}
+
+	@Test
+	void urlPathTokenizer_withEscapedPath_shouldUnescape() {
+		UrlPathTokenizer tokenizer = new UrlPathTokenizer("Homer%20Simpson");
+		assertTrue(tokenizer.hasMoreTokens());
+		assertEquals("Homer Simpson", tokenizer.nextTokenUnescapedAndSanitized());
+
+		tokenizer = new UrlPathTokenizer("hack%2Fslash");
+		assertTrue(tokenizer.hasMoreTokens());
+		assertEquals("hack/slash", tokenizer.nextTokenUnescapedAndSanitized());
+	}
+
+	@Test
+	void urlPathTokenizer_peek_shouldNotConsumeTokens() {
+		UrlPathTokenizer tokenizer = new UrlPathTokenizer("this/that");
+		assertEquals(2, tokenizer.countTokens());
+		tokenizer.peek();
+		assertEquals(2, tokenizer.countTokens());
+	}
+
+	@Test
+	void urlPathTokenizer_withSuspiciousCharacters_sanitizesCorrectly() {
+		UrlPathTokenizer tokenizer = new UrlPathTokenizer("<DROP TABLE USERS>");
+		assertTrue(tokenizer.hasMoreTokens());
+		assertEquals("&lt;DROP TABLE USERS&gt;", tokenizer.nextTokenUnescapedAndSanitized());
+
+		tokenizer = new UrlPathTokenizer("'\n\r\"");
+		assertTrue(tokenizer.hasMoreTokens());
+		assertEquals("&apos;&#10;&#13;&quot;", tokenizer.nextTokenUnescapedAndSanitized());
+	}
+}