Interpret all negative timeouts as infinite (#2810)

zeroc-ice · Sep 27, 2024 · aa38119 · aa38119
1 parent 006580b
commit aa38119
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 98 deletions.
diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/DefaultsAndOverrides.java b/java/src/Ice/src/main/java/com/zeroc/Ice/DefaultsAndOverrides.java
@@ -7,7 +7,7 @@
 import java.time.Duration;
 
 final class DefaultsAndOverrides {
-    DefaultsAndOverrides(Properties properties, Logger logger) {
+    DefaultsAndOverrides(Properties properties) {
         String value;
         int intValue;
 
@@ -67,29 +67,12 @@ final class DefaultsAndOverrides {
                             + "' in property Ice.Default.EndpointSelection; expected 'Random' or 'Ordered'");
         }
 
-        intValue = properties.getIcePropertyAsInt("Ice.Default.LocatorCacheTimeout");
-        if (intValue < -1) {
-            defaultLocatorCacheTimeout = Duration.ofSeconds(-1);
-            StringBuffer msg =
-                    new StringBuffer("invalid value for Ice.Default.LocatorCacheTimeout `");
-            msg.append(properties.getIceProperty("Ice.Default.LocatorCacheTimeout"));
-            msg.append("': defaulting to -1");
-            logger.warning(msg.toString());
-        } else {
-            defaultLocatorCacheTimeout = Duration.ofSeconds(intValue);
-        }
+        defaultLocatorCacheTimeout =
+                Duration.ofSeconds(
+                        properties.getIcePropertyAsInt("Ice.Default.LocatorCacheTimeout"));
 
-        intValue = properties.getIcePropertyAsInt("Ice.Default.InvocationTimeout");
-        if (intValue < 1 && intValue != -1) {
-            defaultInvocationTimeout = Duration.ofMillis(-1);
-            StringBuffer msg =
-                    new StringBuffer("invalid value for Ice.Default.InvocationTimeout `");
-            msg.append(properties.getIceProperty("Ice.Default.InvocationTimeout"));
-            msg.append("': defaulting to -1");
-            logger.warning(msg.toString());
-        } else {
-            defaultInvocationTimeout = Duration.ofMillis(intValue);
-        }
+        defaultInvocationTimeout =
+                Duration.ofMillis(properties.getIcePropertyAsInt("Ice.Default.InvocationTimeout"));
 
         defaultPreferSecure = properties.getIcePropertyAsInt("Ice.Default.PreferSecure") > 0;
 

diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/Instance.java b/java/src/Ice/src/main/java/com/zeroc/Ice/Instance.java
@@ -772,7 +772,7 @@ public void initialize(Communicator communicator, InitializationData initData) {
 
             _traceLevels = new TraceLevels(properties);
 
-            _defaultsAndOverrides = new DefaultsAndOverrides(properties, _initData.logger);
+            _defaultsAndOverrides = new DefaultsAndOverrides(properties);
 
             _clientConnectionOptions = readConnectionOptions("Ice.Connection.Client");
             _serverConnectionOptions = readConnectionOptions("Ice.Connection.Server");

diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/ReferenceFactory.java b/java/src/Ice/src/main/java/com/zeroc/Ice/ReferenceFactory.java
@@ -793,40 +793,16 @@ private Reference create(
             }
 
             property = propertyPrefix + ".LocatorCacheTimeout";
-            if (!properties.getProperty(property).isEmpty()) {
-                int locatorCacheTimeoutValue =
-                        properties.getPropertyAsIntWithDefault(
-                                property, (int) locatorCacheTimeout.toSeconds());
-                if (locatorCacheTimeoutValue < -1) {
-                    locatorCacheTimeoutValue = -1;
-
-                    StringBuffer msg = new StringBuffer("invalid value for ");
-                    msg.append(property);
-                    msg.append(" '");
-                    msg.append(properties.getProperty(property));
-                    msg.append("': defaulting to -1");
-                    _instance.initializationData().logger.warning(msg.toString());
-                }
-                locatorCacheTimeout = Duration.ofSeconds(locatorCacheTimeoutValue);
-            }
+            locatorCacheTimeout =
+                    Duration.ofSeconds(
+                            properties.getPropertyAsIntWithDefault(
+                                    property, (int) locatorCacheTimeout.toSeconds()));
 
             property = propertyPrefix + ".InvocationTimeout";
-            if (!properties.getProperty(property).isEmpty()) {
-                int invocationTimeoutValue =
-                        properties.getPropertyAsIntWithDefault(
-                                property, (int) invocationTimeout.toMillis());
-                if (invocationTimeoutValue < 1 && invocationTimeoutValue != -1) {
-                    invocationTimeoutValue = -1;
-
-                    StringBuffer msg = new StringBuffer("invalid value for ");
-                    msg.append(property);
-                    msg.append(" '");
-                    msg.append(properties.getProperty(property));
-                    msg.append("': defaulting to -1");
-                    _instance.initializationData().logger.warning(msg.toString());
-                }
-                invocationTimeout = Duration.ofMillis(invocationTimeoutValue);
-            }
+            invocationTimeout =
+                    Duration.ofMillis(
+                            properties.getPropertyAsIntWithDefault(
+                                    property, (int) invocationTimeout.toMillis()));
 
             property = propertyPrefix + ".Context.";
             java.util.Map<String, String> contexts = properties.getPropertiesForPrefix(property);

diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/_ObjectPrxI.java b/java/src/Ice/src/main/java/com/zeroc/Ice/_ObjectPrxI.java
@@ -391,10 +391,6 @@ public ObjectPrx ice_locatorCacheTimeout(int newTimeout) {
 
     @Override
     public ObjectPrx ice_locatorCacheTimeout(Duration newTimeout) {
-        if (newTimeout.compareTo(Duration.ofSeconds(-1)) < 0) {
-            throw new IllegalArgumentException(
-                    "invalid value passed to ice_locatorCacheTimeout: " + newTimeout.toString());
-        }
         if (newTimeout.equals(_reference.getLocatorCacheTimeout())) {
             return this;
         } else {
@@ -409,10 +405,6 @@ public ObjectPrx ice_invocationTimeout(int newTimeout) {
 
     @Override
     public ObjectPrx ice_invocationTimeout(Duration newTimeout) {
-        if (newTimeout.compareTo(Duration.ZERO) <= 0 && !newTimeout.equals(Duration.ofMillis(-1))) {
-            throw new IllegalArgumentException(
-                    "invalid value passed to ice_invocationTimeout: " + newTimeout.toString());
-        }
         if (newTimeout.equals(_reference.getInvocationTimeout())) {
             return this;
         } else {

diff --git a/java/test/src/main/java/test/Ice/proxy/AllTests.java b/java/test/src/main/java/test/Ice/proxy/AllTests.java
@@ -651,41 +651,33 @@ public static MyClassPrx allTests(test.TestHelper helper) {
                         .ice_getEncodingVersion()
                         .equals(Util.Encoding_1_1));
 
-        try {
-            base.ice_invocationTimeout(0);
-            test(false);
-        } catch (IllegalArgumentException e) {
-        }
-
-        try {
-            base.ice_invocationTimeout(-1);
-        } catch (IllegalArgumentException e) {
-            test(false);
-        }
-
-        try {
-            base.ice_invocationTimeout(-2);
-            test(false);
-        } catch (IllegalArgumentException e) {
-        }
-
-        try {
-            base.ice_locatorCacheTimeout(0);
-        } catch (IllegalArgumentException e) {
-            test(false);
-        }
-
-        try {
-            base.ice_locatorCacheTimeout(-1);
-        } catch (IllegalArgumentException e) {
-            test(false);
-        }
+        test(
+                base.ice_invocationTimeout(10)
+                        .ice_getInvocationTimeout()
+                        .equals(Duration.ofMillis(10)));
+        test(base.ice_invocationTimeout(0).ice_getInvocationTimeout().equals(Duration.ZERO));
+        test(
+                base.ice_invocationTimeout(-1)
+                        .ice_getInvocationTimeout()
+                        .equals(Duration.ofMillis(-1)));
+        test(
+                base.ice_invocationTimeout(-2)
+                        .ice_getInvocationTimeout()
+                        .equals(Duration.ofMillis(-2)));
 
-        try {
-            base.ice_locatorCacheTimeout(-2);
-            test(false);
-        } catch (IllegalArgumentException e) {
-        }
+        test(
+                base.ice_locatorCacheTimeout(10)
+                        .ice_getLocatorCacheTimeout()
+                        .equals(Duration.ofSeconds(10)));
+        test(base.ice_locatorCacheTimeout(0).ice_getLocatorCacheTimeout().equals(Duration.ZERO));
+        test(
+                base.ice_locatorCacheTimeout(-1)
+                        .ice_getLocatorCacheTimeout()
+                        .equals(Duration.ofSeconds(-1)));
+        test(
+                base.ice_locatorCacheTimeout(-2)
+                        .ice_getLocatorCacheTimeout()
+                        .equals(Duration.ofSeconds(-2)));
 
         // Ensure that the proxy methods can be called unambiguously with the correct return type.
         var diamondClass = DiamondClassPrx.uncheckedCast(base);