Guard against empty YangInstanceIdentifier values 67/104867/6
authorRobert Varga <robert.varga@pantheon.tech>
Mon, 13 Mar 2023 22:57:42 +0000 (23:57 +0100)
committerRobert Varga <nite@hq.sk>
Fri, 5 Jan 2024 17:00:26 +0000 (17:00 +0000)
Empty YangInstanceIdentifier should never be used as a value. Check this
invariant.

JIRA: YANGTOOLS-1494
Change-Id: I3763cb45cb0131a9cbc25aaef65673a08f6dc470
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
codec/yang-data-codec-binfmt/src/test/java/org/opendaylight/yangtools/yang/data/codec/binfmt/YiidSerializationTest.java
data/yang-data-spi/src/main/java/org/opendaylight/yangtools/yang/data/spi/node/impl/AbstractImmutableNormalizedNodeBuilder.java
data/yang-data-spi/src/main/java/org/opendaylight/yangtools/yang/data/spi/node/impl/ImmutableLeafNode.java
data/yang-data-spi/src/main/java/org/opendaylight/yangtools/yang/data/spi/node/impl/ImmutableLeafSetEntryNode.java
data/yang-data-spi/src/test/java/org/opendaylight/yangtools/yang/data/spi/node/impl/YT1494Test.java [new file with mode: 0644]

index 4543a78bdf7594dadf2415678f06ceb0ee508182..52f3204e636249c194b81615932a8746a0d27ed3 100644 (file)
@@ -19,23 +19,13 @@ import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 
 class YiidSerializationTest extends AbstractSerializationTest {
-    @ParameterizedTest
-    @MethodSource
-    void testEmptyIdentifier(final NormalizedNodeStreamVersion version, final int size) {
-        assertSame(version, YangInstanceIdentifier.of(), size);
-    }
-
-    static List<Arguments> testEmptyIdentifier() {
-        return List.of(Arguments.of(NormalizedNodeStreamVersion.POTASSIUM, 96));
-    }
-
     @ParameterizedTest
     @MethodSource
     void testOneIdentifier(final NormalizedNodeStreamVersion version, final int size) {
         assertEquals(version, YangInstanceIdentifier.of(TestModel.TEST_QNAME), size);
     }
 
-    static List<Arguments> testOneIdentifier() {
+    private static List<Arguments> testOneIdentifier() {
         return List.of(Arguments.of(NormalizedNodeStreamVersion.POTASSIUM, 98));
     }
 
@@ -46,7 +36,7 @@ class YiidSerializationTest extends AbstractSerializationTest {
         assertEquals(version, fillUniqueIdentifier(31), uniqueSize);
     }
 
-    static List<Arguments> test31() {
+    private static List<Arguments> test31() {
         return List.of(Arguments.of(NormalizedNodeStreamVersion.POTASSIUM, 158, 359));
     }
 
@@ -57,7 +47,7 @@ class YiidSerializationTest extends AbstractSerializationTest {
         assertEquals(version, fillUniqueIdentifier(32), uniqueSize);
     }
 
-    static List<Arguments> test32() {
+    private static List<Arguments> test32() {
         return List.of(Arguments.of(NormalizedNodeStreamVersion.POTASSIUM, 164, 372));
     }
 
@@ -68,7 +58,7 @@ class YiidSerializationTest extends AbstractSerializationTest {
         assertEquals(version, fillUniqueIdentifier(256), uniqueSize);
     }
 
-    static List<Arguments> test256() {
+    private static List<Arguments> test256() {
         return List.of(Arguments.of(NormalizedNodeStreamVersion.POTASSIUM, 612, 2_388));
     }
 
@@ -79,7 +69,7 @@ class YiidSerializationTest extends AbstractSerializationTest {
         assertEquals(version, fillUniqueIdentifier(65792), uniqueSize);
     }
 
-    static List<Arguments> test65792() {
+    private static List<Arguments> test65792() {
         return List.of(Arguments.of(NormalizedNodeStreamVersion.POTASSIUM, 131_684, 719_700));
     }
 
@@ -108,7 +98,7 @@ class YiidSerializationTest extends AbstractSerializationTest {
         }
     }
 
-    static List<Arguments> testTwice65536() {
+    private static List<Arguments> testTwice65536() {
         return List.of(Arguments.of(NormalizedNodeStreamVersion.POTASSIUM, 916_815));
     }
 
index bba17ce8108516e460c504410158a6f5e3285adb..fb502f014568f8cd51c0ca60cdf27309fa103b19 100644 (file)
@@ -11,6 +11,7 @@ import static java.util.Objects.requireNonNull;
 
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.data.api.schema.builder.NormalizedNodeBuilder;
@@ -30,7 +31,7 @@ abstract class AbstractImmutableNormalizedNodeBuilder<I extends PathArgument, V,
 
     @Override
     public NormalizedNodeBuilder<I, V, R> withValue(final V withValue) {
-        this.value = requireNonNull(withValue);
+        this.value = checkValue(withValue);
         return this;
     }
 
@@ -46,4 +47,12 @@ abstract class AbstractImmutableNormalizedNodeBuilder<I extends PathArgument, V,
         }
         return obj;
     }
+
+    protected static final <T> @NonNull T checkValue(final @Nullable T value) {
+        final var nonNull = requireNonNull(value);
+        if (nonNull instanceof YangInstanceIdentifier yiid && yiid.isEmpty()) {
+            throw new IllegalArgumentException("Node value cannot be an empty instance identifier");
+        }
+        return nonNull;
+    }
 }
index 5100e7f37c84ce95ada6eb773655ac2888271086..89a31aa95bfdbca3671098a38a359c4c2bd5753b 100644 (file)
@@ -10,6 +10,7 @@ package org.opendaylight.yangtools.yang.data.spi.node.impl;
 import static java.util.Objects.requireNonNull;
 
 import org.eclipse.jdt.annotation.NonNull;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.AbstractLeafNode;
 
@@ -49,6 +50,8 @@ public abstract sealed class ImmutableLeafNode<T> extends AbstractLeafNode<T> {
             @SuppressWarnings("unchecked")
             final var ret = (ImmutableLeafNode<T>) new Binary(identifier, bytes);
             return ret;
+        } else if (value instanceof YangInstanceIdentifier yiid && yiid.isEmpty()) {
+            throw new IllegalArgumentException("Leaf node value cannot be an empty instance identifier");
         }
         return new Regular<>(identifier, value);
     }
index 14194bb507a5a638a057dda46b5b29859f1cbd4a..39ab9acaa87f6a77101e6eef6d02c3992071d12b 100644 (file)
@@ -11,6 +11,7 @@ import static java.util.Objects.requireNonNull;
 
 import java.util.Arrays;
 import org.eclipse.jdt.annotation.NonNull;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
 import org.opendaylight.yangtools.yang.data.api.schema.AbstractLeafSetEntryNode;
 
@@ -44,10 +45,13 @@ public abstract sealed class ImmutableLeafSetEntryNode<T> extends AbstractLeafSe
     }
 
     public static <T> @NonNull ImmutableLeafSetEntryNode<T> of(final NodeWithValue<T> name) {
-        if (name.getValue() instanceof byte[]) {
+        final var nameValue = name.getValue();
+        if (nameValue instanceof byte[]) {
             @SuppressWarnings("unchecked")
             final var ret = (ImmutableLeafSetEntryNode<T>) new Binary((NodeWithValue<byte[]>) name);
             return ret;
+        } else if (nameValue instanceof YangInstanceIdentifier yiid && yiid.isEmpty()) {
+            throw new IllegalArgumentException("Leafset entry node value cannot be an empty instance identifier");
         }
         return new Regular<>(name);
     }
@@ -60,6 +64,8 @@ public abstract sealed class ImmutableLeafSetEntryNode<T> extends AbstractLeafSe
                 final var ret = (ImmutableLeafSetEntryNode<T>) new Binary((NodeWithValue<byte[]>) name);
                 return ret;
             }
+        } else if (nameValue instanceof YangInstanceIdentifier yiid && yiid.isEmpty()) {
+            throw new IllegalArgumentException("Leafset entry node value cannot be an empty instance identifier");
         } else if (nameValue.equals(body)) {
             return new Regular<>(name);
         }
diff --git a/data/yang-data-spi/src/test/java/org/opendaylight/yangtools/yang/data/spi/node/impl/YT1494Test.java b/data/yang-data-spi/src/test/java/org/opendaylight/yangtools/yang/data/spi/node/impl/YT1494Test.java
new file mode 100644 (file)
index 0000000..2b1516f
--- /dev/null
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2024 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.yangtools.yang.data.spi.node.impl;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import org.junit.jupiter.api.Test;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
+
+class YT1494Test {
+    private static final QName FOO = QName.create("foo", "foo");
+
+    @Test
+    void testLeafNode() {
+        final var ex = assertThrows(IllegalArgumentException.class,
+            () -> ImmutableLeafNode.of(new NodeIdentifier(FOO), YangInstanceIdentifier.of()));
+        assertEquals("Leaf node value cannot be an empty instance identifier", ex.getMessage());
+    }
+
+    @Test
+    void testLeafSetEntryNode() {
+        final var ex = assertThrows(IllegalArgumentException.class,
+            () -> ImmutableLeafSetEntryNode.of(new NodeWithValue<>(FOO, YangInstanceIdentifier.of())));
+        assertEquals("Leafset entry node value cannot be an empty instance identifier", ex.getMessage());
+    }
+}