Add timeout to gathering host certificates #567

Author: danthe1st

bundles/org.eclipse.equinox.p2.core/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.equinox.p2.core;singleton:=true
-Bundle-Version: 2.12.200.qualifier
+Bundle-Version: 2.12.300.qualifier
 Bundle-Activator: org.eclipse.equinox.internal.p2.core.Activator
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin

bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/p2/core/IProvisioningAgent.java

@@ -165,4 +165,32 @@ default boolean getBooleanProperty(String key, boolean defaultValue) {
 		return Boolean.parseBoolean(getProperty(key, Boolean.toString(defaultValue)));
 	}
 
+	/**
+	 * Returns an agent bound property as an int, the default implementation
+	 * delegates to {@link IProvisioningAgent#getIntProperty(String, int)} with
+	 * <code>0</code> as the default.
+	 *
+	 * @since 2.12
+	 */
+	default int getIntProperty(String key) {
+		return getIntProperty(key, 0);
+	}
+
+	/**
+	 * Returns an agent bound property as an int, the default implementation
+	 * delegates to {@link IProvisioningAgent#getProperty(String)} with the given
+	 * default as a String and parses the result as by
+	 * {@link Boolean#parseBoolean(String)}.
+	 *
+	 * @since 2.12
+	 */
+	default int getIntProperty(String key, int defaultValue) {
+		String propertyValue = getProperty(key, Integer.toString(defaultValue));
+		try {
+			return Integer.parseInt(propertyValue);
+		} catch (NumberFormatException e) {
+			return defaultValue;
+		}
+	}
+
 }

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

bundles/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$

bundles/org.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.

bundles/org.eclipse.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,10 @@ public class AuthorityChecker {
 	private static final Pattern HIERARCHICAL_URI_PATTERN = Pattern
 			.compile("((?:[^/:]+):(?://[^/]+|///|/)?)([^?#]*)([#?].*)?"); //$NON-NLS-1$
 
+	private final int requestTimeoutMillis;
+
+	private final int maxRequestRetries;
+
 	private final IProvisioningAgent agent;
 	private final ProvisioningContext context;
 	private final IProfile profile;
@@ -75,6 +81,9 @@ public AuthorityChecker(IProvisioningAgent agent, IProfile profile) {
 		this.ius = ius;
 		this.artifacts = artifacts;
 		this.profile = profile;
+		requestTimeoutMillis = agent.getIntProperty("org.eclipse.equinox.p2.engine.certificateRequestTimeout", 5000);//$NON-NLS-1$
+		maxRequestRetries = agent.getIntProperty("org.eclipse.equinox.p2.engine.certificateRequestRetries", //$NON-NLS-1$
+				3);
 	}
 
 	IStatus start(IProgressMonitor monitor) {
@@ -235,7 +244,7 @@ public static List<URI> getFilteredAuthorities(Collection<? extends URI> authori
 		return filteredAuthorities;
 	}
 
-	public static Map<URI, List<Certificate>> getCertificates(Collection<? extends URI> uris,
+	public Map<URI, List<Certificate>> getCertificates(Collection<? extends URI> uris,
 			IProgressMonitor monitor) {
 		var certificates = new TreeMap<URI, List<Certificate>>();
 		var authorities = new TreeMap<URI, List<Certificate>>();
@@ -247,13 +256,14 @@ public static Map<URI, List<Certificate>> getCertificates(Collection<? extends U
 		return certificates;
 	}
 
-	public static void gatherCertificates(Map<URI, List<Certificate>> authorities, IProgressMonitor montior) {
+	public void gatherCertificates(Map<URI, List<Certificate>> authorities, IProgressMonitor montior) {
 		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(requestTimeoutMillis))
+						.method("HEAD", BodyPublishers.noBody()) //$NON-NLS-1$
+						.build();
+				return Optional.of(sendHttpRequestOrRetry(client, request, maxRequestRetries));
 			} catch (RuntimeException ex) {
 				return Optional.<CompletableFuture<HttpResponse<String>>>ofNullable(null);
 			}
@@ -270,16 +280,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 > 1) {
+			future = future.exceptionallyComposeAsync(e -> sendHttpRequestOrRetry(client, request, retriesLeft - 1));
+		}
+		return future;
+	}
+
 	/**
 	 * <p>
 	 * Returns a list of URIs representing the hierarchical chain, starting from the

bundles/org.eclipse.equinox.p2.ui.sdk/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %bundleName
 Bundle-SymbolicName: org.eclipse.equinox.p2.ui.sdk;singleton:=true
-Bundle-Version: 1.3.300.qualifier
+Bundle-Version: 1.3.400.qualifier
 Bundle-Activator: org.eclipse.equinox.internal.p2.ui.sdk.ProvSDKUIActivator
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin

bundles/org.eclipse.equinox.p2.ui.sdk/src/org/eclipse/equinox/internal/p2/ui/sdk/TrustPreferencePage.java

@@ -115,7 +115,7 @@ protected IStatus run(IProgressMonitor monitor) {
 					}
 				});
 
-				var certificates = AuthorityChecker.getCertificates(uris, monitor);
+				var certificates = authorityChecker.getCertificates(uris, monitor);
 				authorityCertificates.putAll(certificates);
 
 				if (!parent.isDisposed()) {

features/org.eclipse.equinox.p2.core.feature/feature.xml

@@ -2,7 +2,7 @@
 <feature
       id="org.eclipse.equinox.p2.core.feature"
       label="%featureName"
-      version="1.7.400.qualifier"
+      version="1.7.500.qualifier"
       provider-name="%providerName"
       license-feature="org.eclipse.license"
       license-feature-version="0.0.0">

features/org.eclipse.equinox.p2.discovery.feature/feature.xml

@@ -2,7 +2,7 @@
 <feature
       id="org.eclipse.equinox.p2.discovery.feature"
       label="%featureName"
-      version="1.3.600.qualifier"
+      version="1.3.700.qualifier"
       provider-name="%providerName"
       license-feature="org.eclipse.license"
       license-feature-version="0.0.0">