You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2021/08/30 10:30:46 UTC
[brooklyn-server] branch master updated: more exclusion of edge
cases for out of range double/integer values
This is an automated email from the ASF dual-hosted git repository.
heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
The following commit(s) were added to refs/heads/master by this push:
new ec8c21a more exclusion of edge cases for out of range double/integer values
new 3b92f82 Merge branch 'master' of https://gitbox.apache.org/repos/asf/brooklyn-server
ec8c21a is described below
commit ec8c21aece322b187b2e87a271db8ac72f51ee22
Author: Alex Heneveld <al...@cloudsoftcorp.com>
AuthorDate: Mon Aug 30 11:13:23 2021 +0100
more exclusion of edge cases for out of range double/integer values
---
.../brooklyn/camp/brooklyn/ConfigYamlTest.java | 77 ++++++++++++++++++++++
.../coerce/PrimitiveStringTypeCoercions.java | 34 ++++++----
.../java/org/apache/brooklyn/util/yaml/Yamls.java | 54 ++++++++++++++-
3 files changed, 151 insertions(+), 14 deletions(-)
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java
index 9c77478..645155c 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java
@@ -18,6 +18,10 @@
*/
package org.apache.brooklyn.camp.brooklyn;
+import java.util.Map;
+import org.apache.brooklyn.util.internal.BrooklynSystemProperties;
+import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
+import org.apache.brooklyn.util.yaml.Yamls;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
@@ -52,6 +56,8 @@ public class ConfigYamlTest extends AbstractYamlTest {
private static final Logger LOG = LoggerFactory.getLogger(ConfigYamlTest.class);
+ final static String DOUBLE_MAX_VALUE_TIMES_TEN = "" + Double.MAX_VALUE + "0";
+
private ExecutorService executor;
@BeforeMethod(alwaysRun = true)
@@ -450,6 +456,32 @@ public class ConfigYamlTest extends AbstractYamlTest {
}
@Test
+ public void testConfigOtherVeryLargeIntegerCoercionFails() throws Exception {
+ String yaml = Joiner.on("\n").join(
+ "services:",
+ "- type: org.apache.brooklyn.core.test.entity.TestEntity",
+ " brooklyn.config:",
+ " test.confInteger: 100000000000000000000000000000000000",
+ "");
+
+ Asserts.assertFailsWith(() -> createStartWaitAndLogApplication(yaml),
+ e -> e.toString().contains("100000000000000000000000000000000000"));
+ }
+
+ @Test
+ public void testConfigSlightlyLargeIntegerCoercionFails() throws Exception {
+ String yaml = Joiner.on("\n").join(
+ "services:",
+ "- type: org.apache.brooklyn.core.test.entity.TestEntity",
+ " brooklyn.config:",
+ " test.confInteger: 2147483648",
+ "");
+
+ Asserts.assertFailsWith(() -> createStartWaitAndLogApplication(yaml),
+ e -> e.toString().contains("2147483648"));
+ }
+
+ @Test
public void testConfigVeryLargeDoubleCoercionFails() throws Exception {
String yaml = Joiner.on("\n").join(
"services:",
@@ -463,6 +495,51 @@ public class ConfigYamlTest extends AbstractYamlTest {
}
@Test
+ public void testConfigSlightlyLargeDoubleParseFails() throws Exception {
+ Object xm = null;
+ try {
+ xm = Yamls.parseAll("x: " + DOUBLE_MAX_VALUE_TIMES_TEN).iterator().next();
+
+// Object x = ((Map)xm).get("x");
+// LOG.info("x: "+x);
+// if (x instanceof Double) {
+// Asserts.fail("Should not be a double: "+x);
+// }
+
+ Asserts.shouldHaveFailedPreviously();
+ } catch (Exception e) {
+ Asserts.expectedFailureContainsIgnoreCase(e, "yaml parser", "out of range", DOUBLE_MAX_VALUE_TIMES_TEN);
+ }
+ }
+
+ @Test
+ public void testConfigSlightlyLargeDoubleCoercionFails() throws Exception {
+ String yaml = Joiner.on("\n").join(
+ "services:",
+ "- type: org.apache.brooklyn.core.test.entity.TestEntity",
+ " brooklyn.config:",
+ " test.confDouble: "+DOUBLE_MAX_VALUE_TIMES_TEN,
+ "");
+
+ Asserts.assertFailsWith(() -> createStartWaitAndLogApplication(yaml),
+ e -> Asserts.expectedFailureContainsIgnoreCase(e, "yaml parser", "out of range", DOUBLE_MAX_VALUE_TIMES_TEN));
+ }
+
+
+ @Test
+ public void testConfigSlightlyLargeDoubleAsStringCoercionFails() throws Exception {
+ String yaml = Joiner.on("\n").join(
+ "services:",
+ "- type: org.apache.brooklyn.core.test.entity.TestEntity",
+ " brooklyn.config:",
+ " test.confDouble: "+ JavaStringEscapes.wrapJavaString(DOUBLE_MAX_VALUE_TIMES_TEN),
+ "");
+
+ Asserts.assertFailsWith(() -> createStartWaitAndLogApplication(yaml),
+ e -> Asserts.expectedFailureContainsIgnoreCase(e, "cannot coerce", DOUBLE_MAX_VALUE_TIMES_TEN));
+ }
+
+ @Test
public void testConfigFloatAsIntegerCoercionFails() throws Exception {
String yaml = Joiner.on("\n").join(
"services:",
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/PrimitiveStringTypeCoercions.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/PrimitiveStringTypeCoercions.java
index 2cc6804..9e86fad 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/PrimitiveStringTypeCoercions.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/PrimitiveStringTypeCoercions.java
@@ -132,10 +132,10 @@ public class PrimitiveStringTypeCoercions {
islong = false;
}
if (islong) {
- if (targetWrapType == Character.class) return Maybe.of((T) Character.valueOf((char)v));
- if (targetWrapType == Byte.class) return Maybe.of((T) Byte.valueOf((byte)v));
- if (targetWrapType == Short.class) return Maybe.of((T) Short.valueOf((short)v));
- if (targetWrapType == Integer.class) return Maybe.of((T) Integer.valueOf((int)v));
+ if (targetWrapType == Character.class) return Maybe.of((T) Character.valueOf((char)v));
+ if (targetWrapType == Byte.class) return Maybe.of((T) (Byte) Byte.parseByte(""+v));
+ if (targetWrapType == Short.class) return Maybe.of((T) (Short) Short.parseShort(""+v));
+ if (targetWrapType == Integer.class) return Maybe.of((T) (Integer) Integer.parseInt(""+v));
if (targetWrapType == Long.class) return Maybe.of((T) Long.valueOf(v));
if (targetWrapType == Float.class) return Maybe.of((T) Float.valueOf(v));
if (targetWrapType == Double.class) return Maybe.of((T) Double.valueOf(v));
@@ -188,17 +188,17 @@ public class PrimitiveStringTypeCoercions {
}
@SuppressWarnings("unchecked")
- public static <T> Maybe<T> stringToPrimitiveMaybe(String value, Class<T> targetType) {
+ public static <T> Maybe<T> stringToPrimitiveMaybe(String origValue, Class<T> targetType) {
assert Primitives.allPrimitiveTypes().contains(targetType) || Primitives.allWrapperTypes().contains(targetType) : "targetType="+targetType;
// If char, then need to do explicit conversion
if (targetType == Character.class || targetType == char.class) {
- if (value.length() == 1) {
- return Maybe.of((T) (Character) value.charAt(0));
- } else if (value.length() != 1) {
- throw new ClassCoercionException("Cannot coerce type String to "+targetType.getCanonicalName()+" ("+value+"): adapting failed");
+ if (origValue.length() == 1) {
+ return Maybe.of((T) (Character) origValue.charAt(0));
+ } else if (origValue.length() != 1) {
+ throw new ClassCoercionException("Cannot coerce type String to "+targetType.getCanonicalName()+" ("+origValue+"): adapting failed");
}
}
- value = value.trim();
+ String value = origValue.trim();
// For boolean we could use valueOf, but that returns false whereas we'd rather throw errors on bad values
if (targetType == Boolean.class || targetType == boolean.class) {
if ("true".equalsIgnoreCase(value)) return Maybe.of((T) Boolean.TRUE);
@@ -221,13 +221,25 @@ public class PrimitiveStringTypeCoercions {
wrappedType = targetType;
}
+ Object v;
try {
- return Maybe.of((T) wrappedType.getMethod("valueOf", String.class).invoke(null, value));
+ v = wrappedType.getMethod("valueOf", String.class).invoke(null, value);
} catch (Exception e) {
ClassCoercionException tothrow = new ClassCoercionException("Cannot coerce "+value.getClass().getSimpleName()+" "+JavaStringEscapes.wrapJavaString(value)+" to "+targetType.getCanonicalName()+": adapting failed");
tothrow.initCause(e);
return Maybe.absent(tothrow);
}
+
+ if (isNanOrInf(v)) {
+ return Maybe.absent(() -> new NumberFormatException("Invalid number for "+value+" as "+targetType+": "+v));
+ }
+ return Maybe.of((T) v);
+ }
+
+ public static boolean isNanOrInf(Object o) {
+ if (o instanceof Double) return !Double.isFinite((double)o);
+ if (o instanceof Float) return !Float.isFinite((float)o);
+ return false;
}
}
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/yaml/Yamls.java b/utils/common/src/main/java/org/apache/brooklyn/util/yaml/Yamls.java
index efe1ffc..986d9b3 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/yaml/Yamls.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/yaml/Yamls.java
@@ -35,10 +35,12 @@ import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.exceptions.UserFacingException;
import org.apache.brooklyn.util.internal.BrooklynSystemProperties;
+import org.apache.brooklyn.util.javalang.coerce.PrimitiveStringTypeCoercions;
import org.apache.brooklyn.util.text.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.yaml.snakeyaml.Yaml;
+import org.yaml.snakeyaml.constructor.BaseConstructor;
import org.yaml.snakeyaml.constructor.Constructor;
import org.yaml.snakeyaml.constructor.SafeConstructor;
import org.yaml.snakeyaml.error.Mark;
@@ -51,19 +53,65 @@ import org.yaml.snakeyaml.nodes.SequenceNode;
import com.google.common.annotations.Beta;
import com.google.common.collect.Iterables;
+import org.yaml.snakeyaml.nodes.Tag;
public class Yamls {
private static final Logger log = LoggerFactory.getLogger(Yamls.class);
private static Yaml newYaml() {
+ BaseConstructor constructor;
+ if (BrooklynSystemProperties.YAML_TYPE_INSTANTIATION.isEnabled()) {
+ // allows instantiation of arbitrary Java types;
+ constructor = new Constructor() {
+
+ };
+ } else {
+ constructor = new SafeConstructor() {
+
+ };
+ }
return new Yaml(
BrooklynSystemProperties.YAML_TYPE_INSTANTIATION.isEnabled()
- ? new Constructor() // allows instantiation of arbitrary Java types
- : new SafeConstructor() // allows instantiation of limited set of types only
+ ? new ConstructorExcludingNonNumbers() // allows instantiation of arbitrary Java types
+ : new SafeConstructorExcludingNonNumbers() // allows instantiation of limited set of types only
);
}
+ private static class ConstructorExcludingNonNumbers extends Constructor {
+ public ConstructorExcludingNonNumbers() {
+ super();
+ this.yamlConstructors.put(Tag.FLOAT, new ConstructYamlFloatExcludingNonNumbers());
+ }
+ class ConstructYamlFloatExcludingNonNumbers extends ConstructYamlFloat {
+ @Override
+ public Object construct(Node node) {
+ return numericDoublesOnly(super.construct(node), node);
+ }
+ }
+ }
+
+ private static class SafeConstructorExcludingNonNumbers extends SafeConstructor {
+ public SafeConstructorExcludingNonNumbers() {
+ super();
+ this.yamlConstructors.put(Tag.FLOAT, new ConstructYamlFloatExcludingNonNumbers());
+ }
+ class ConstructYamlFloatExcludingNonNumbers extends ConstructYamlFloat {
+ @Override
+ public Object construct(Node node) {
+ return numericDoublesOnly(super.construct(node), node);
+ }
+ }
+ }
+
+ private static Object numericDoublesOnly(Object construct, Node node) {
+ if (PrimitiveStringTypeCoercions.isNanOrInf(construct)) {
+ throw new IllegalStateException("YAML parser forbids out of range doubles; consider wrapping as string and coercing to type BigDecimal: "+node);
+ }
+
+ return construct;
+ }
+
/** returns the given (yaml-parsed) object as the given yaml type.
* <p>
* if the object is an iterable or iterator this method will fully expand it as a list.
@@ -99,7 +147,7 @@ public class Yamls {
/**
* Parses the given yaml, and walks the given path to return the referenced object.
*
- * @see #getAt(Object, List)
+ * @see #getAtPreParsed(Object, List)
*/
@Beta
public static Object getAt(String yaml, List<String> path) {