AuthorizationInterceptor concurrency failure (#3528)

* Fix #3256 - AuthorizationInterceptor concurrency failure

* Test fixes

hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeResourceDefinition.java

@@ -217,6 +217,12 @@ public class RuntimeResourceDefinition extends BaseRuntimeElementCompositeDefini
 				}
 			}
 		}
+
+		// Make the map of lists completely unmodifiable
+		for (String nextKey : new ArrayList<>(compartmentNameToSearchParams.keySet())) {
+			List<RuntimeSearchParam> nextList = compartmentNameToSearchParams.get(nextKey);
+			compartmentNameToSearchParams.put(nextKey, Collections.unmodifiableList(nextList));
+		}
 		myCompartmentNameToSearchParams = Collections.unmodifiableMap(compartmentNameToSearchParams);
 
 		Class<?> target = getImplementingClass();

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

@@ -748,7 +748,7 @@ public class FhirTerser {
 
 		List<RuntimeSearchParam> params = sourceDef.getSearchParamsForCompartmentName(theCompartmentName);
 
-		//If passed an additional set of searchparameter names, add them for comparison purposes.
+		// If passed an additional set of searchparameter names, add them for comparison purposes.
 		if (theAdditionalCompartmentParamNames != null) {
 			List<RuntimeSearchParam> additionalParams = theAdditionalCompartmentParamNames.stream().map(sourceDef::getSearchParam)
 				.filter(Objects::nonNull)
@@ -756,6 +756,9 @@ public class FhirTerser {
 			if (params == null || params.isEmpty()) {
 				params = additionalParams;
 			} else {
+				List<RuntimeSearchParam> existingParams = params;
+				params = new ArrayList<>(existingParams.size() + additionalParams.size());
+				params.addAll(existingParams);
 				params.addAll(additionalParams);
 			}
 		}

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

@@ -62,6 +62,10 @@ public class UrlUtil {
 	private static final String URL_FORM_PARAMETER_OTHER_SAFE_CHARS = "-_.*";
 	private static final Escaper PARAMETER_ESCAPER = new PercentEscaper(URL_FORM_PARAMETER_OTHER_SAFE_CHARS, false);
 
+	public static String sanitizeBaseUrl(String theBaseUrl) {
+		return theBaseUrl.replaceAll("[^a-zA-Z0-9:/._-]", "");
+	}
+
 	public static class UrlParts {
 		private String myParams;
 		private String myResourceId;

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3256-avoid-concurrency-failure.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 3256
+title: "An occasional concurrency failure in AuthorizationInterceptor has been resolved. Thanks to 
+  Martin Visser for reporting and providing a reproducible test case!"

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java

@@ -789,7 +789,12 @@ public class ResponseHighlighterInterceptor {
 
 			InputStream jsStream = ResponseHighlighterInterceptor.class.getResourceAsStream("ResponseHighlighter.js");
 			String jsStr = jsStream != null ? IOUtils.toString(jsStream, StandardCharsets.UTF_8) : "console.log('ResponseHighlighterInterceptor: javascript theResource not found')";
-			jsStr = jsStr.replace("FHIR_BASE", theRequestDetails.getServerBaseForRequest());
+
+			String baseUrl = theRequestDetails.getServerBaseForRequest();
+
+			baseUrl = UrlUtil.sanitizeBaseUrl(baseUrl);
+
+			jsStr = jsStr.replace("FHIR_BASE", baseUrl);
 			outputBuffer.append("<script type=\"text/javascript\">");
 			outputBuffer.append(jsStr);
 			outputBuffer.append("</script>\n");

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleImplOp.java

@@ -398,6 +398,9 @@ class RuleImplOp extends BaseRule /* implements IAuthRule */ {
 					if (params == null || params.isEmpty()) {
 						params = additionalParams;
 					} else {
+						List<RuntimeSearchParam> existingParams = params;
+						params = new ArrayList<>(existingParams.size() + additionalParams.size());
+						params.addAll(existingParams);
 						params.addAll(additionalParams);
 					}
 

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

@@ -19,6 +19,7 @@ import org.hl7.fhir.r4.model.DocumentReference;
 import org.hl7.fhir.r4.model.Enumeration;
 import org.hl7.fhir.r4.model.Enumerations;
 import org.hl7.fhir.r4.model.Extension;
+import org.hl7.fhir.r4.model.IdType;
 import org.hl7.fhir.r4.model.Identifier;
 import org.hl7.fhir.r4.model.MarkdownType;
 import org.hl7.fhir.r4.model.Medication;
@@ -51,6 +52,10 @@ import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import java.util.stream.Collectors;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -1427,6 +1432,40 @@ public class FhirTerserR4Test {
 		assertEquals("cid:device@bundle", elems.get(1).getReferenceElement().getValue());
 	}
 
+	@Test
+	public void testConcurrentTerserCalls() throws ExecutionException, InterruptedException {
+		ExecutorService executor = Executors.newFixedThreadPool(10);
+		try {
+
+			FhirContext ctx = FhirContext.forR4();
+
+			List<Future<?>> futures = new ArrayList<>();
+			for (int i = 0; i < 10; i++) {
+
+				Runnable runnable = () -> {
+
+					Observation observation = new Observation();
+					observation.setSubject(new Reference("Patient/123"));
+					HashSet<String> additionalCompartmentParamNames = new HashSet<>();
+					additionalCompartmentParamNames.add("test");
+					boolean outcome = ctx.newTerser().isSourceInCompartmentForTarget("Patient", observation, new IdType("Patient/123"), additionalCompartmentParamNames);
+					assertTrue(outcome);
+
+				};
+				Future<?> future = executor.submit(runnable);
+				futures.add(future);
+			}
+
+			for (var next : futures) {
+				next.get();
+			}
+
+		}finally {
+			executor.shutdown();
+		}
+	}
+
+
 
 	private List<String> toStrings(List<StringType> theStrings) {
 		ArrayList<String> retVal = new ArrayList<>();

pom.xml

@@ -1713,6 +1713,20 @@
 				<groupId>org.quartz-scheduler</groupId>
 				<artifactId>quartz</artifactId>
 				<version>2.3.2</version>
+				<exclusions>
+					<exclusion>
+						<groupId>com.zaxxer</groupId>
+						<artifactId>HikariCP-java7</artifactId>
+					</exclusion>
+					<exclusion>
+						<groupId>com.mchange</groupId>
+						<artifactId>c3p0</artifactId>
+					</exclusion>
+					<exclusion>
+						<groupId>com.mchange</groupId>
+						<artifactId>mchange-commons-java</artifactId>
+					</exclusion>
+				</exclusions>
 			</dependency>
 			<dependency>
 				<groupId>org.slf4j</groupId>