diff --git a/device-detection/device-detection.cloud/src/main/java/fiftyone/devicedetection/cloud/flowelements/DeviceDetectionCloudEngine.java b/device-detection/device-detection.cloud/src/main/java/fiftyone/devicedetection/cloud/flowelements/DeviceDetectionCloudEngine.java index 400ca1dad..85ebcd913 100644 --- a/device-detection/device-detection.cloud/src/main/java/fiftyone/devicedetection/cloud/flowelements/DeviceDetectionCloudEngine.java +++ b/device-detection/device-detection.cloud/src/main/java/fiftyone/devicedetection/cloud/flowelements/DeviceDetectionCloudEngine.java @@ -41,6 +41,8 @@ import org.json.JSONObject; import org.slf4j.Logger; +import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse; + import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -138,6 +140,14 @@ protected void processEngine(FlowData data, DeviceDataCloud aspectData) { String json = ""; json = requestData.getJsonResponse(); + // A failed cloud request leaves no response. The Cloud Request + // Engine reports why, so this engine adds no error of its own. + if (json == null || json.isEmpty()) { + logger.warn(String.format( + NoCloudResponse, getClass().getSimpleName())); + return; + } + // Extract data from json to the aspectData instance. JSONObject jsonObj = new JSONObject(json); JSONObject deviceObj = getElementJson(jsonObj); diff --git a/device-detection/device-detection.cloud/src/test/java/fiftyone/devicedetection/cloud/flowelements/ProductMissingTests.java b/device-detection/device-detection.cloud/src/test/java/fiftyone/devicedetection/cloud/flowelements/ProductMissingTests.java index 2cd708eb6..00e1abf90 100644 --- a/device-detection/device-detection.cloud/src/test/java/fiftyone/devicedetection/cloud/flowelements/ProductMissingTests.java +++ b/device-detection/device-detection.cloud/src/test/java/fiftyone/devicedetection/cloud/flowelements/ProductMissingTests.java @@ -97,6 +97,26 @@ public void ProductMissing_ProcessLogsAndThrowsNothing() throws Exception { } } + /** + * A failed cloud request leaves no response. The engine processes + * nothing, throws nothing and logs one warning, as the Cloud Request + * Engine's error says why. + */ + @Test + public void NoResponse_ProcessesNothing() throws Exception { + try (DeviceDetectionCloudEngine engine = buildEngine("device", "hardwarevendor")) { + for (String response : new String[] { null, "" }) { + DeviceDataCloud data = process(engine, response); + assertFalse(data.asKeyMap().containsKey("hardwarevendor")); + } + List logged = warningsAndErrors(); + assertEquals(logged.toString(), 2, logged.size()); + for (String line : logged) { + assertTrue(line, line.startsWith("warn ")); + } + } + } + /** * With the Device Detection product included, a response that holds no * part for it leaves the element data without values and throws diff --git a/ip-intelligence/ip-intelligence.cloud/src/main/java/fiftyone/ipintelligence/cloud/flowelements/IPIntelligenceCloudEngine.java b/ip-intelligence/ip-intelligence.cloud/src/main/java/fiftyone/ipintelligence/cloud/flowelements/IPIntelligenceCloudEngine.java index 99b84cbb3..8ca08e569 100644 --- a/ip-intelligence/ip-intelligence.cloud/src/main/java/fiftyone/ipintelligence/cloud/flowelements/IPIntelligenceCloudEngine.java +++ b/ip-intelligence/ip-intelligence.cloud/src/main/java/fiftyone/ipintelligence/cloud/flowelements/IPIntelligenceCloudEngine.java @@ -41,6 +41,8 @@ import org.json.JSONObject; import org.slf4j.Logger; +import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse; + import fiftyone.pipeline.core.data.IWeightedValue; import fiftyone.pipeline.core.data.WeightedValue; import fiftyone.pipeline.core.data.WktString; @@ -150,6 +152,14 @@ protected void processEngine(FlowData data, IPIntelligenceDataCloud aspectData) String json = ""; json = requestData.getJsonResponse(); + // A failed cloud request leaves no response. The Cloud Request + // Engine reports why, so this engine adds no error of its own. + if (json == null || json.isEmpty()) { + logger.warn(String.format( + NoCloudResponse, getClass().getSimpleName())); + return; + } + // Extract data from json to the aspectData instance. JSONObject jsonObj = new JSONObject(json); JSONObject ipiObj = getElementJson(jsonObj); diff --git a/ip-intelligence/ip-intelligence.cloud/src/test/java/fiftyone/ipintelligence/cloud/flowelements/ProductMissingTests.java b/ip-intelligence/ip-intelligence.cloud/src/test/java/fiftyone/ipintelligence/cloud/flowelements/ProductMissingTests.java index a597f4662..924a476bb 100644 --- a/ip-intelligence/ip-intelligence.cloud/src/test/java/fiftyone/ipintelligence/cloud/flowelements/ProductMissingTests.java +++ b/ip-intelligence/ip-intelligence.cloud/src/test/java/fiftyone/ipintelligence/cloud/flowelements/ProductMissingTests.java @@ -97,6 +97,26 @@ public void ProductMissing_ProcessLogsAndThrowsNothing() throws Exception { } } + /** + * A failed cloud request leaves no response. The engine processes + * nothing, throws nothing and logs one warning, as the Cloud Request + * Engine's error says why. + */ + @Test + public void NoResponse_ProcessesNothing() throws Exception { + try (IPIntelligenceCloudEngine engine = buildEngine("ip", "registeredname")) { + for (String response : new String[] { null, "" }) { + IPIntelligenceDataCloud data = process(engine, response); + assertFalse(data.asKeyMap().containsKey("registeredname")); + } + List logged = warningsAndErrors(); + assertEquals(logged.toString(), 2, logged.size()); + for (String line : logged) { + assertTrue(line, line.startsWith("warn ")); + } + } + } + /** * With the IP Intelligence product included, a response that holds no * part for it leaves the element data without values and throws diff --git a/location/geo-location.cloud/src/main/java/fiftyone/geolocation/flowelements/GeoLocationCloudEngine.java b/location/geo-location.cloud/src/main/java/fiftyone/geolocation/flowelements/GeoLocationCloudEngine.java index 6c8eae436..10da0267b 100644 --- a/location/geo-location.cloud/src/main/java/fiftyone/geolocation/flowelements/GeoLocationCloudEngine.java +++ b/location/geo-location.cloud/src/main/java/fiftyone/geolocation/flowelements/GeoLocationCloudEngine.java @@ -43,6 +43,8 @@ import org.json.JSONObject; import org.slf4j.Logger; +import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse; + import java.util.*; import org.json.JSONArray; @@ -137,6 +139,14 @@ protected void processEngine(FlowData data, CloudGeoData aspectData) { CloudRequestData requestData = data.getFromElement(cloudRequestEngine); String json = requestData.getJsonResponse(); + // A failed cloud request leaves no response. The Cloud Request + // Engine reports why, so this engine adds no error of its own. + if (json == null || json.isEmpty()) { + logger.warn(String.format( + NoCloudResponse, getClass().getSimpleName())); + return; + } + JSONObject jsonObj = new JSONObject(json); JSONObject geoObj = getElementJson(jsonObj); diff --git a/location/geo-location.cloud/src/test/java/fiftyone/geolocation/flowelements/GeoLocationCloudEngineNullValueTests.java b/location/geo-location.cloud/src/test/java/fiftyone/geolocation/flowelements/GeoLocationCloudEngineNullValueTests.java index 8bfe09c1b..74391dba6 100644 --- a/location/geo-location.cloud/src/test/java/fiftyone/geolocation/flowelements/GeoLocationCloudEngineNullValueTests.java +++ b/location/geo-location.cloud/src/test/java/fiftyone/geolocation/flowelements/GeoLocationCloudEngineNullValueTests.java @@ -36,6 +36,7 @@ import fiftyone.pipeline.engines.services.MissingPropertyServiceDefault; import org.junit.Test; import org.slf4j.ILoggerFactory; +import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.Arrays; @@ -46,8 +47,12 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; /** @@ -142,6 +147,27 @@ public void SetNoValueReasons_ChangesNothing() throws Exception { } } + /** + * A failed cloud request leaves no response. The engine processes + * nothing, throws nothing and logs one warning each time, as the Cloud + * Request Engine's error says why. + */ + @Test + public void NoResponse_ProcessesNothing() throws Exception { + Logger logger = mock(Logger.class); + ILoggerFactory warnings = mock(ILoggerFactory.class); + when(warnings.getLogger(anyString())).thenReturn(logger); + try (GeoLocationCloudEngine engine = buildEngine(warnings)) { + for (String response : new String[] { null, "" }) { + CloudGeoData data = process(engine, response); + + assertFalse(data.asKeyMap().containsKey("Town")); + } + } + verify(logger, times(2)).warn(String.format( + NoCloudResponse, GeoLocationCloudEngine.class.getSimpleName())); + } + private static void assertMissing(Runnable read) { try { read.run(); @@ -183,8 +209,16 @@ private static void assertValue(Object expected, Object value) * the Suburb and Town properties. */ private GeoLocationCloudEngine buildEngine() throws Exception { + return buildEngine(loggerFactory); + } + + /** + * The same engine, logging through the given factory. + */ + private GeoLocationCloudEngine buildEngine(ILoggerFactory engineLoggers) + throws Exception { GeoLocationCloudEngine engine = - new GeoLocationCloudEngineBuilder(loggerFactory) + new GeoLocationCloudEngineBuilder(engineLoggers) .build(Enums.GeoLocationProvider.FiftyOneDegrees); ProductMetaData product = new ProductMetaData(); diff --git a/location/pom.xml b/location/pom.xml index 05c2eb72b..0aefaf454 100644 --- a/location/pom.xml +++ b/location/pom.xml @@ -63,7 +63,7 @@ 4.5.130 4.13.2 - 1.7.36 + 2.0.20 1.3.16 4.3.1 20211205 diff --git a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java index eebb82eb4..73b0756a9 100644 --- a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java +++ b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java @@ -58,6 +58,16 @@ public class Messages { public static final String EvidenceConflict = "'%s:%s' evidence conflicts with %s"; + + /** + * Warning a cloud engine logs when the Cloud Request Engine has no + * response for it to read. The argument is the engine's class name. + */ + public static final String NoCloudResponse = + "The %s did not process, as the Cloud Request Engine has no " + + "response. The Cloud Request Engine's error in the log says " + + "why."; + public static final String MessageNoDataInResponse = "No data in response from cloud service at '%s'"; public static final String MessageErrorCodeReturned = diff --git a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudAspectEngineBase.java b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudAspectEngineBase.java index 80b39e9a6..fd037341a 100644 --- a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudAspectEngineBase.java +++ b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudAspectEngineBase.java @@ -23,6 +23,7 @@ package fiftyone.pipeline.cloudrequestengine.flowelements; import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.MessageProductNotInResourceKey; +import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse; import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.ProcessCloudEngineNotImplemented; import fiftyone.pipeline.cloudrequestengine.NotImplementedException; @@ -432,11 +433,8 @@ protected void processEngine(FlowData data, TData aspectData) { if (json != null && !json.isEmpty()) { processCloudEngine(data, aspectData, json); } else { - logger.warn("The " + this.getClass().getSimpleName() + " did not process " + - "as the JSON response from the CloudRequestEngine was null " + - "or empty. Please refer to errors generated by the " + - "CloudRequestEngine in the logs as this indicates an error " + - "occurred there."); + logger.warn(String.format( + NoCloudResponse, getClass().getSimpleName())); } } diff --git a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java index f8758e126..6b1017929 100644 --- a/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java +++ b/pipeline/pipeline.core/src/main/java/fiftyone/pipeline/core/flowelements/FlowDataDefault.java @@ -121,7 +121,7 @@ public void addError(FlowError error) { error.getFlowElement().getClass().getSimpleName() + "-" + error.getFlowElement().hashCode(); } - logger.error(message); + logger.error(message, error.getThrowable()); } } diff --git a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java index 613033fa2..e2235f987 100644 --- a/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java +++ b/pipeline/pipeline.core/src/test/java/fiftyone/pipeline/core/flowelements/FlowDataTests.java @@ -317,6 +317,25 @@ public void FlowData_GetNotPresent_Element() { assertNull(result); } + /** + * The error logged for a processing failure carries the exception, so + * the log says why processing failed. + */ + @Test + public void FlowData_AddError_LogsTheException() throws Exception { + Logger logger = mock(Logger.class); + when(logger.isErrorEnabled()).thenReturn(true); + Exception cause = new Exception("The cause of the failure."); + try (FlowData data = new FlowDataDefault( + logger, + pipeline, + new EvidenceDefault(mock(Logger.class)))) { + + data.addError(cause, mock(FlowElement.class)); + } + verify(logger).error(anyString(), same(cause)); + } + @Test(expected = IllegalStateException.class) public void FlowData_PipelineDisposed() throws Exception { when(pipeline.isClosed()).thenReturn(true); diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/flowelements/SimpleCloudEngine.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/flowelements/SimpleCloudEngine.java index 89cd49481..cc6d14970 100644 --- a/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/flowelements/SimpleCloudEngine.java +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/flowelements/SimpleCloudEngine.java @@ -34,6 +34,8 @@ import org.json.JSONObject; import org.slf4j.Logger; +import static fiftyone.pipeline.cloudrequestengine.Constants.Messages.NoCloudResponse; + import java.util.ArrayList; import java.util.List; @@ -90,6 +92,14 @@ protected void processEngine(FlowData data, StarSignData aspectData) { CloudRequestData requestData = data.getFromElement(cloudRequestEngine); String json = requestData.getJsonResponse(); + // A failed cloud request leaves no response. The Cloud Request + // Engine reports why, so this engine adds no error of its own. + if (json == null || json.isEmpty()) { + logger.warn(String.format( + NoCloudResponse, getClass().getSimpleName())); + return; + } + // Extract data from json to the aspectData instance. A cloud // service answers only for the products the resource key covers, // so a response without the star sign product leaves the value diff --git a/pipeline/pipeline.engines/src/main/java/fiftyone/pipeline/engines/services/HostLookup.java b/pipeline/pipeline.engines/src/main/java/fiftyone/pipeline/engines/services/HostLookup.java new file mode 100644 index 000000000..fc4939cad --- /dev/null +++ b/pipeline/pipeline.engines/src/main/java/fiftyone/pipeline/engines/services/HostLookup.java @@ -0,0 +1,207 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package fiftyone.pipeline.engines.services; + +import java.io.IOException; +import java.io.InterruptedIOException; +import java.net.InetAddress; +import java.net.Proxy; +import java.net.ProxySelector; +import java.net.SocketTimeoutException; +import java.net.URISyntaxException; +import java.net.URL; +import java.net.UnknownHostException; +import java.util.List; +import java.util.Locale; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +/** + * Looks up the host of a URL within a time limit. The connect timeout of an + * {@link java.net.HttpURLConnection} does not cover the name lookup, which + * waits as long as the system resolver does. Looking the host up here first + * leaves the address in the JVM's cache for the connection to use. Callers + * waiting on the same host share one lookup, so a resolver that does not + * answer holds one thread, not one for each request. + *

+ * Two limits follow from relying on that cache. With the security property + * {@code networkaddress.cache.ttl} set to 0 nothing is cached, so the + * connection looks the host up again itself, with no limit. And every + * request is looked up here, including one the connection would send on a + * kept-alive socket with no lookup at all, so once the cached address + * expires, 30 seconds after it was found by default, a resolver that does + * not answer fails that request although the open socket could have served + * it. + */ +final class HostLookup { + + /** + * Message of the exception thrown when a lookup takes longer than the + * timeout. The arguments are the host and the timeout in milliseconds. + */ + static final String MessageLookupTimedOut = + "Looking up the host '%s' took longer than the timeout of %d ms."; + + /** + * Finds the addresses of a host. + */ + interface Resolver { + InetAddress[] resolve(String host) throws UnknownHostException; + } + + /** + * The lookup {@link HttpClientDefault} uses, through the JVM's resolver. + */ + static final HostLookup DEFAULT = new HostLookup(InetAddress::getAllByName); + + private static final Executor LOOKUPS = Executors.newCachedThreadPool( + runnable -> { + Thread thread = new Thread(runnable, "51Degrees host lookup"); + thread.setDaemon(true); + return thread; + }); + + private final Resolver resolver; + + private final ConcurrentMap> + inProgress = new ConcurrentHashMap<>(); + + HostLookup(Resolver resolver) { + this.resolver = resolver; + } + + /** + * Look up the host of the URL, waiting at most the timeout. Nothing is + * looked up when the timeout is 0, which means no limit, or when a proxy + * makes the request, because the proxy looks up the host. + * @param url the URL to be requested + * @param timeoutMillis the longest wait in milliseconds + * @throws SocketTimeoutException if the lookup takes longer + * @throws UnknownHostException if the host has no address + * @throws IOException if the lookup fails in another way + */ + void lookUp(URL url, int timeoutMillis) throws IOException { + String host = url.getHost(); + if (timeoutMillis <= 0 || + host == null || + host.isEmpty() || + usesProxy(url)) { + return; + } + String key = host.toLowerCase(Locale.ROOT); + CompletableFuture started = new CompletableFuture<>(); + CompletableFuture lookup = + inProgress.putIfAbsent(key, started); + if (lookup == null) { + lookup = started; + start(key, host, started); + } + try { + lookup.get(timeoutMillis, TimeUnit.MILLISECONDS); + } catch (TimeoutException e) { + throw new SocketTimeoutException(String.format( + MessageLookupTimedOut, host, timeoutMillis)); + } catch (ExecutionException e) { + throw failure(host, e.getCause()); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + InterruptedIOException interrupted = new InterruptedIOException( + "Interrupted while looking up the host '" + host + "'."); + interrupted.initCause(e); + throw interrupted; + } + } + + /** + * Start the lookup on its own thread. A finished lookup is removed from + * those in progress, so the next request looks up again, which the JVM's + * cache answers while the address is cached. + */ + private void start( + String key, + String host, + CompletableFuture lookup) { + try { + LOOKUPS.execute(() -> { + try { + lookup.complete(resolver.resolve(host)); + } catch (Throwable e) { + lookup.completeExceptionally(e); + } finally { + inProgress.remove(key, lookup); + } + }); + } catch (RuntimeException | Error e) { + inProgress.remove(key, lookup); + lookup.completeExceptionally(e); + throw e; + } + } + + /** + * The exception for a lookup that failed, thrown on the caller's thread + * with the lookup's own exception as the cause. + */ + private static IOException failure(String host, Throwable cause) { + if (cause instanceof UnknownHostException) { + UnknownHostException unknown = + new UnknownHostException(cause.getMessage()); + unknown.initCause(cause); + return unknown; + } + return new IOException( + "Looking up the host '" + host + "' failed.", cause); + } + + /** + * Whether the default proxy selector sends the request through a proxy. + * A URL it cannot read counts as one, so it is not looked up here. + */ + private static boolean usesProxy(URL url) { + ProxySelector selector = ProxySelector.getDefault(); + if (selector == null) { + return false; + } + List proxies; + try { + proxies = selector.select(url.toURI()); + } catch (URISyntaxException | IllegalArgumentException e) { + return true; + } + if (proxies == null) { + return false; + } + for (Proxy proxy : proxies) { + if (proxy.type() != Proxy.Type.DIRECT) { + return true; + } + } + return false; + } +} diff --git a/pipeline/pipeline.engines/src/main/java/fiftyone/pipeline/engines/services/HttpClientDefault.java b/pipeline/pipeline.engines/src/main/java/fiftyone/pipeline/engines/services/HttpClientDefault.java index 3dfaa5282..4759da850 100644 --- a/pipeline/pipeline.engines/src/main/java/fiftyone/pipeline/engines/services/HttpClientDefault.java +++ b/pipeline/pipeline.engines/src/main/java/fiftyone/pipeline/engines/services/HttpClientDefault.java @@ -29,8 +29,26 @@ import java.net.URL; import java.util.Map; +/** + * Makes requests with {@link HttpURLConnection}. The connect timeout set on + * a connection also limits the lookup of its host, which the connection's + * own timeouts do not cover. A host is not looked up when the default + * {@link java.net.ProxySelector} sends its request through a proxy. A + * subclass whose {@link #connect(URL)} opens the connection through a proxy + * of its own still has the host looked up here, so its requests fail when + * the host cannot be found locally although the proxy could reach it. + */ public class HttpClientDefault implements HttpClient { + private final HostLookup hostLookup; + + public HttpClientDefault() { + this(HostLookup.DEFAULT); + } + + HttpClientDefault(HostLookup hostLookup) { + this.hostLookup = hostLookup; + } @Override public HttpURLConnection connect(URL url) throws IOException { @@ -49,6 +67,7 @@ public String postData( connection.setRequestProperty(e.getKey(), e.getValue()); } } + lookUpHost(connection); if (data == null) { connection.setDoOutput(false); } else { @@ -86,6 +105,7 @@ public String getResponseString( connection.setRequestProperty(e.getKey(), e.getValue()); } } + lookUpHost(connection); BufferedReader in = new BufferedReader( new InputStreamReader(connection.getResponseCode() / 100 == 2 ? @@ -100,4 +120,12 @@ public String getResponseString( in.close(); return response.toString(); } + + /** + * Look up the connection's host within its connect timeout, before the + * connection makes its own lookup, which has no limit. + */ + private void lookUpHost(HttpURLConnection connection) throws IOException { + hostLookup.lookUp(connection.getURL(), connection.getConnectTimeout()); + } } diff --git a/pipeline/pipeline.engines/src/test/java/fiftyone/pipeline/engines/services/HostLookupTests.java b/pipeline/pipeline.engines/src/test/java/fiftyone/pipeline/engines/services/HostLookupTests.java new file mode 100644 index 000000000..06bddbe6b --- /dev/null +++ b/pipeline/pipeline.engines/src/test/java/fiftyone/pipeline/engines/services/HostLookupTests.java @@ -0,0 +1,266 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package fiftyone.pipeline.engines.services; + +import org.junit.After; +import org.junit.Test; + +import java.io.IOException; +import java.net.HttpURLConnection; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.Proxy; +import java.net.ProxySelector; +import java.net.SocketAddress; +import java.net.SocketTimeoutException; +import java.net.URI; +import java.net.URL; +import java.net.UnknownHostException; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Checks that a host lookup that does not answer fails within the connect + * timeout, as the connection's own timeouts do not cover the lookup. + */ +public class HostLookupTests { + + private static final String HOST = "unanswered.example"; + + /** The timeout given to each lookup. */ + private static final int TIMEOUT_MILLIS = 200; + + /** + * How long a resolver that does not answer holds a lookup unless the + * test releases it. It stands in for a system resolver that waits 30 + * seconds before giving up. + */ + private static final long UNANSWERED_MILLIS = 30000; + + /** + * The longest a timed out lookup may take. It is many times the timeout, + * so a slow runner passes, and far below the resolver's wait, so a + * lookup that waited for the resolver fails. + */ + private static final long LIMIT_MILLIS = 5000; + + /** The longest a test waits for another thread to reach a point. */ + private static final long WAIT_MILLIS = 10000; + + private final CountDownLatch release = new CountDownLatch(1); + private final CountDownLatch entered = new CountDownLatch(1); + private final AtomicInteger calls = new AtomicInteger(); + + @After + public void releaseResolver() { + release.countDown(); + } + + /** + * A resolver that holds each lookup until released, then answers with + * the loopback address, or fails if it is never released. + */ + private HostLookup.Resolver held() { + return host -> { + calls.incrementAndGet(); + entered.countDown(); + try { + if (release.await(UNANSWERED_MILLIS, TimeUnit.MILLISECONDS)) { + return new InetAddress[] { InetAddress.getLoopbackAddress() }; + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + throw new UnknownHostException(host); + }; + } + + private static URL url() throws IOException { + return URI.create("https://" + HOST + "/api/v4/evidencekeys").toURL(); + } + + private static long millisSince(long start) { + return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + } + + @Test + public void LookUp_ResolverDoesNotAnswer_TimesOutNamingTheHost() + throws Exception { + HostLookup lookup = new HostLookup(held()); + long start = System.nanoTime(); + try { + lookup.lookUp(url(), TIMEOUT_MILLIS); + fail("The lookup should have timed out."); + } catch (SocketTimeoutException e) { + long took = millisSince(start); + assertTrue("The lookup took " + took + " ms.", took < LIMIT_MILLIS); + assertEquals( + String.format(HostLookup.MessageLookupTimedOut, + HOST, TIMEOUT_MILLIS), + e.getMessage()); + } + } + + @Test + public void LookUp_UnknownHost_ThrowsUnknownHostWithTheCause() + throws Exception { + UnknownHostException unknown = new UnknownHostException(HOST); + HostLookup lookup = new HostLookup(host -> { + throw unknown; + }); + try { + lookup.lookUp(url(), TIMEOUT_MILLIS); + fail("The lookup should have failed."); + } catch (UnknownHostException e) { + assertEquals(HOST, e.getMessage()); + assertSame(unknown, e.getCause()); + } + } + + @Test + public void LookUp_NoTimeout_DoesNotLookUp() throws Exception { + new HostLookup(held()).lookUp(url(), 0); + assertEquals(0, calls.get()); + } + + @Test + public void LookUp_AfterAFailure_LooksUpAgain() throws Exception { + HostLookup lookup = new HostLookup(host -> { + calls.incrementAndGet(); + throw new UnknownHostException(host); + }); + for (int i = 0; i < 2; i++) { + try { + lookup.lookUp(url(), TIMEOUT_MILLIS); + fail("The lookup should have failed."); + } catch (UnknownHostException e) { + // Expected. + } + } + assertEquals(2, calls.get()); + } + + @Test + public void LookUp_SameHostAtOnce_SharesOneLookup() throws Exception { + HostLookup lookup = new HostLookup(held()); + AtomicReference firstFailure = new AtomicReference<>(); + AtomicReference secondFailure = new AtomicReference<>(); + Thread first = lookUpOnThread(lookup, firstFailure); + assertTrue(entered.await(WAIT_MILLIS, TimeUnit.MILLISECONDS)); + Thread second = lookUpOnThread(lookup, secondFailure); + // The second caller is waiting on the first one's lookup once it is + // in a timed wait. + long start = System.nanoTime(); + while (second.getState() != Thread.State.TIMED_WAITING && + millisSince(start) < WAIT_MILLIS) { + Thread.sleep(10); + } + release.countDown(); + first.join(WAIT_MILLIS); + second.join(WAIT_MILLIS); + assertNull(firstFailure.get()); + assertNull(secondFailure.get()); + assertEquals(1, calls.get()); + } + + private static Thread lookUpOnThread( + HostLookup lookup, + AtomicReference failure) { + Thread thread = new Thread(() -> { + try { + lookup.lookUp(url(), (int) WAIT_MILLIS); + } catch (Throwable e) { + failure.set(e); + } + }); + thread.start(); + return thread; + } + + @Test + public void LookUp_ThroughAProxy_DoesNotLookUp() throws Exception { + ProxySelector original = ProxySelector.getDefault(); + ProxySelector.setDefault(new ProxySelector() { + @Override + public List select(URI uri) { + return Collections.singletonList(new Proxy( + Proxy.Type.HTTP, + InetSocketAddress.createUnresolved("proxy.example", 8080))); + } + + @Override + public void connectFailed( + URI uri, SocketAddress address, IOException e) { + } + }); + try { + new HostLookup(held()).lookUp(url(), TIMEOUT_MILLIS); + } finally { + ProxySelector.setDefault(original); + } + assertEquals(0, calls.get()); + } + + @Test + public void GetResponseString_HostDoesNotAnswer_FailsWithinConnectTimeout() + throws Exception { + HttpClient client = new HttpClientDefault(new HostLookup(held())); + HttpURLConnection connection = client.connect(url()); + connection.setConnectTimeout(TIMEOUT_MILLIS); + long start = System.nanoTime(); + try { + client.getResponseString(connection); + fail("The request should have timed out."); + } catch (SocketTimeoutException e) { + long took = millisSince(start); + assertTrue("The request took " + took + " ms.", took < LIMIT_MILLIS); + } + } + + @Test + public void PostData_HostDoesNotAnswer_FailsWithinConnectTimeout() + throws Exception { + HttpClient client = new HttpClientDefault(new HostLookup(held())); + HttpURLConnection connection = client.connect(url()); + connection.setConnectTimeout(TIMEOUT_MILLIS); + long start = System.nanoTime(); + try { + client.postData( + connection, Collections.emptyMap(), new byte[] { 1 }); + fail("The request should have timed out."); + } catch (SocketTimeoutException e) { + long took = millisSince(start); + assertTrue("The request took " + took + " ms.", took < LIMIT_MILLIS); + } + } +}