Make sure binary values are properly wrapped 64/61464/1
authorRobert Varga <robert.varga@pantheon.tech>
Thu, 10 Aug 2017 00:23:46 +0000 (02:23 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 10 Aug 2017 00:23:46 +0000 (02:23 +0200)
Leaking byte[] in case binary leaves an avenue for data modification.
Disallow that by always cloning byte[].

Change-Id: Ic3cde28a4af19800d00f0f1e1ab9084b6e6951ef
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableLeafNodeBuilder.java
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableNormalizedValueAttrNode.java
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableNormalizedValueNode.java

index a5ea2e5ca4f3a952f16c4a9d4446c7dad7ac2d89..adab1e3d749dd48e37715beb46ae9770702081f0 100644 (file)
@@ -21,14 +21,33 @@ public class ImmutableLeafNodeBuilder<T> extends AbstractImmutableNormalizedNode
     }
 
     @Override
+    @SuppressWarnings("unchecked")
     public LeafNode<T> build() {
-        return new ImmutableLeafNode<>(getNodeIdentifier(), getValue(), getAttributes());
-    }
+        final T value = getValue();
+        if (value instanceof byte[]) {
+            return (LeafNode<T>) new ImmutableBinaryLeafNode(getNodeIdentifier(), (byte[]) value, getAttributes());
+        }
 
-    private static final class ImmutableLeafNode<T> extends AbstractImmutableNormalizedValueAttrNode<NodeIdentifier, T> implements LeafNode<T> {
+        return new ImmutableLeafNode<>(getNodeIdentifier(), value, getAttributes());
+    }
 
+    private static final class ImmutableLeafNode<T>
+            extends AbstractImmutableNormalizedValueAttrNode<NodeIdentifier, T> implements LeafNode<T> {
         ImmutableLeafNode(final NodeIdentifier nodeIdentifier, final T value, final Map<QName, String> attributes) {
             super(nodeIdentifier, value, attributes);
         }
     }
+
+    private static final class ImmutableBinaryLeafNode
+            extends AbstractImmutableNormalizedValueAttrNode<NodeIdentifier, byte[]> implements LeafNode<byte[]> {
+        ImmutableBinaryLeafNode(final NodeIdentifier nodeIdentifier, final byte[] value,
+            final Map<QName, String> attributes) {
+            super(nodeIdentifier, value, attributes);
+        }
+
+        @Override
+        protected byte[] wrapValue(final byte[] value) {
+            return value.clone();
+        }
+    }
 }
index 6fbcc6967257c1854cc0bf6937fe9cdb00dd12cf..bbc797f59b4c4130a2cffd068d0ca32f34078285 100644 (file)
@@ -43,7 +43,8 @@ public abstract class AbstractImmutableNormalizedValueAttrNode<K extends PathArg
 
     @Override
     protected int valueHashCode() {
-        final int result = getValue() != null ? getValue().hashCode() : 1;
+        final V local = value();
+        final int result = local != null ? local.hashCode() : 1;
         // FIXME: are attributes part of hashCode/equals?
         return result;
     }
@@ -53,12 +54,11 @@ public abstract class AbstractImmutableNormalizedValueAttrNode<K extends PathArg
         // We can not call directly getValue.equals because of Empty Type
         // Definition leaves which allways have NULL value
 
-        if (!Objects.deepEquals(getValue(), other.getValue())) {
+        if (!Objects.deepEquals(value(), other.getValue())) {
             return false;
         }
 
         // FIXME: are attributes part of hashCode/equals?
         return true;
     }
-
 }
index 62b1bce8c87e9e6726101f32323ac02d64bee875..a37a44450e5df1b6ff83bfa284d9d7e7361c0387 100644 (file)
@@ -21,12 +21,14 @@ public abstract class AbstractImmutableNormalizedValueNode<K extends PathArgumen
 
     protected AbstractImmutableNormalizedValueNode(final K nodeIdentifier, @Nullable final V value) {
         super(nodeIdentifier);
+
+        /*
+         * Null value is allowed for empty type definition so it should be debug,
+         * but still we are logging it in case we need to debug missing values.
+         */
+        // FIXME: one we do not map YANG 'void' to java.lang.Void we should be enforcing non-null here
         if (value == null) {
-            /*
-             * Null value is allowed for empty type definition so it should be debug,
-             * but still we are logging it in case we need to debug missing values.
-             */
-            LOG.debug("The value of node {} is null",nodeIdentifier.getNodeType());
+            LOG.debug("The value of node {} is null", nodeIdentifier.getNodeType());
         }
         this.value = value;
     }
@@ -34,6 +36,15 @@ public abstract class AbstractImmutableNormalizedValueNode<K extends PathArgumen
     @Nullable
     @Override
     public final V getValue() {
+        return wrapValue(value);
+    }
+
+    @Nullable
+    protected final V value() {
+        return value;
+    }
+
+    protected V wrapValue(final V value) {
         return value;
     }
 }