Add timeout to gathering host certificates #567

eclipse-equinox · Nov 30, 2024 · 4d5d6e8 · 4d5d6e8
1 parent b4a50b8
commit 4d5d6e8
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 7 deletions.
diff --git a/bundles/org.eclipse.equinox.p2.engine/META-INF/MANIFEST.MF b/bundles/org.eclipse.equinox.p2.engine/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.equinox.p2.engine;singleton:=true
-Bundle-Version: 2.10.300.qualifier
+Bundle-Version: 2.10.400.qualifier
 Bundle-Activator: org.eclipse.equinox.internal.p2.engine.EngineActivator
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin

diff --git a/...es/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/Messages.java b/...es/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/Messages.java
@@ -25,6 +25,7 @@ public class Messages extends NLS {
 	public static String ActionManager_Required_Touchpoint_Not_Found;
 
 	public static String AuthorityChecker_UntrustedAuthorities;
+	public static String AuthorityChecker_GatherCertificatesFailure;
 
 	public static String actions_not_found;
 	private static final String BUNDLE_NAME = "org.eclipse.equinox.internal.p2.engine.messages"; //$NON-NLS-1$

diff --git a/....eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/messages.properties b/....eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/messages.properties
@@ -15,6 +15,7 @@
 ActionManager_Exception_Creating_Action_Extension=Error creating action with id: {0}.
 ActionManager_Required_Touchpoint_Not_Found=The required {0} touchpoint for the {1} action is not included in the installation manager configuration.
 AuthorityChecker_UntrustedAuthorities=One or more authorities is not trusted. Cannot proceed with installation.
+AuthorityChecker_GatherCertificatesFailure=Obtaining a host certificate resulted in an exception. This host will be ignored.
 action_syntax_error=Invalid action syntax: {0}.
 download_artifact=Downloading artifacts
 download_no_repository=No artifact repository available.

diff --git a/...equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/phases/AuthorityChecker.java b/...equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/phases/AuthorityChecker.java
@@ -16,6 +16,7 @@
 import java.net.http.HttpRequest.BodyPublishers;
 import java.net.http.HttpResponse.BodyHandlers;
 import java.security.cert.Certificate;
+import java.time.Duration;
 import java.util.*;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
@@ -25,6 +26,7 @@
 import javax.net.ssl.SSLPeerUnverifiedException;
 import org.eclipse.core.runtime.*;
 import org.eclipse.core.runtime.preferences.IEclipsePreferences;
+import org.eclipse.equinox.internal.p2.core.helpers.LogHelper;
 import org.eclipse.equinox.internal.p2.engine.EngineActivator;
 import org.eclipse.equinox.internal.p2.engine.Messages;
 import org.eclipse.equinox.p2.core.*;
@@ -52,6 +54,11 @@ public class AuthorityChecker {
 	private static final Pattern HIERARCHICAL_URI_PATTERN = Pattern
 			.compile("((?:[^/:]+):(?://[^/]+|///|/)?)([^?#]*)([#?].*)?"); //$NON-NLS-1$
 
+	private static final int REQUEST_TIMEOUT_MS = Integer.getInteger("org.eclipse.equinox.p2.engine.requestTimeout", 5000); //$NON-NLS-1$
+
+	private static final int MAX_REQUEST_RETRIES = Integer.getInteger("org.eclipse.equinox.p2.engine.requestRetries", //$NON-NLS-1$
+			2);
+
 	private final IProvisioningAgent agent;
 	private final ProvisioningContext context;
 	private final IProfile profile;
@@ -251,9 +258,10 @@ public static void gatherCertificates(Map<URI, List<Certificate>> authorities, I
 		var client = HttpClient.newBuilder().build();
 		var requests = authorities.keySet().stream().collect(Collectors.toMap(Function.identity(), uri -> {
 			try {
-				return Optional.of(client.sendAsync(
-						HttpRequest.newBuilder().uri(uri).method("HEAD", BodyPublishers.noBody()).build(), //$NON-NLS-1$
-						BodyHandlers.ofString()));
+				var request = HttpRequest.newBuilder().uri(uri).timeout(Duration.ofMillis(REQUEST_TIMEOUT_MS))
+						.method("HEAD", BodyPublishers.noBody()) //$NON-NLS-1$
+						.build();
+				return Optional.of(sendHttpRequestOrRetry(client, request, MAX_REQUEST_RETRIES));
 			} catch (RuntimeException ex) {
 				return Optional.<CompletableFuture<HttpResponse<String>>>ofNullable(null);
 			}
@@ -270,16 +278,29 @@ public static void gatherCertificates(Map<URI, List<Certificate>> authorities, I
 							var peerCertificates = sslSession.getPeerCertificates();
 							entry.getValue().addAll(Arrays.asList(peerCertificates));
 						} catch (SSLPeerUnverifiedException e) {
-							//$FALL-THROUGH$
+							LogHelper.log(new Status(IStatus.WARNING, EngineActivator.ID,
+									Messages.AuthorityChecker_GatherCertificatesFailure, e));
 						}
 					});
-				} catch (RuntimeException | InterruptedException | ExecutionException e) {
-					//$FALL-THROUGH$
+				} catch (InterruptedException e) {
+					Thread.currentThread().interrupt();
+				} catch (RuntimeException | ExecutionException e) {
+					LogHelper.log(new Status(IStatus.WARNING, EngineActivator.ID,
+							Messages.AuthorityChecker_GatherCertificatesFailure, e));
 				}
 			});
 		}
 	}
 
+	private static CompletableFuture<HttpResponse<String>> sendHttpRequestOrRetry(HttpClient client,
+			HttpRequest request, int retriesLeft) {
+		var future = client.sendAsync(request, BodyHandlers.ofString());
+		if (retriesLeft > 0) {
+			future = future.exceptionallyComposeAsync(e -> sendHttpRequestOrRetry(client, request, retriesLeft - 1));
+		}
+		return future;
+	}
+
 	/**
 	 * <p>
 	 * Returns a list of URIs representing the hierarchical chain, starting from the