Use careful byte-masking/shifting in Mg Input

We really want to operate on byte and not int, the bytes are to
be treated as unsigned, which gets wrecked by sign extension.

Introduce an explicit mask(byte, byte) method and use it where
we are potentially sign-extending.

JIRA: CONTROLLER-1919
Change-Id: I0484f440c1589859fb268947f5a80a2b4969451e
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/AbstractMagnesiumDataInput.java b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/AbstractMagnesiumDataInput.java
index 2d02dd11ca1eefebef1770891b8f62bcf0216814..b06a80f15e3bd23e1cedf81eb36e1f0ae8ccf5e1 100644 (file)
--- a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/AbstractMagnesiumDataInput.java
+++ b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/AbstractMagnesiumDataInput.java
@@ -255,7 +255,7 @@ abstract class AbstractMagnesiumDataInput extends AbstractNormalizedNodeDataInpu
         final NodeIdentifier nodeId = decodeNodeIdentifier(nodeHeader, parent);
 
         final int size;
-        switch (nodeHeader & MagnesiumNode.PREDICATE_MASK) {
+        switch (mask(nodeHeader, MagnesiumNode.PREDICATE_MASK)) {
             case MagnesiumNode.PREDICATE_ZERO:
                 size = 0;
                 break;
@@ -431,7 +431,7 @@ abstract class AbstractMagnesiumDataInput extends AbstractNormalizedNodeDataInpu
     }
 
     private AugmentationIdentifier readAugmentationIdentifier(final byte header) throws IOException {
-        final int count = header & MagnesiumPathArgument.AID_COUNT_MASK;
+        final byte count = mask(header, MagnesiumPathArgument.AID_COUNT_MASK);
         switch (count) {
             case MagnesiumPathArgument.AID_COUNT_1B:
                 return readAugmentationIdentifier(input.readUnsignedByte());
@@ -440,7 +440,7 @@ abstract class AbstractMagnesiumDataInput extends AbstractNormalizedNodeDataInpu
             case MagnesiumPathArgument.AID_COUNT_4B:
                 return readAugmentationIdentifier(input.readInt());
             default:
-                return readAugmentationIdentifier(count >>> MagnesiumPathArgument.AID_COUNT_SHIFT);
+                return readAugmentationIdentifier(rshift(count, MagnesiumPathArgument.AID_COUNT_SHIFT));
         }
     }
 
@@ -479,7 +479,7 @@ abstract class AbstractMagnesiumDataInput extends AbstractNormalizedNodeDataInpu
 
     private NodeIdentifierWithPredicates readNodeIdentifierWithPredicates(final byte header) throws IOException {
         final QName qname = readNodeIdentifier(header).getNodeType();
-        switch (header & MagnesiumPathArgument.SIZE_MASK) {
+        switch (mask(header, MagnesiumPathArgument.SIZE_MASK)) {
             case MagnesiumPathArgument.SIZE_1B:
                 return readNodeIdentifierWithPredicates(qname, input.readUnsignedByte());
             case MagnesiumPathArgument.SIZE_2B:
@@ -487,7 +487,7 @@ abstract class AbstractMagnesiumDataInput extends AbstractNormalizedNodeDataInpu
             case MagnesiumPathArgument.SIZE_4B:
                 return readNodeIdentifierWithPredicates(qname, input.readInt());
             default:
-                return readNodeIdentifierWithPredicates(qname, header >>> MagnesiumPathArgument.SIZE_SHIFT);
+                return readNodeIdentifierWithPredicates(qname, rshift(header, MagnesiumPathArgument.SIZE_SHIFT));
         }
     }
 
@@ -514,7 +514,7 @@ abstract class AbstractMagnesiumDataInput extends AbstractNormalizedNodeDataInpu
     }
 
     private static void verifyPathIdentifierOnly(final byte header) throws InvalidNormalizedNodeStreamException {
-        if ((header & MagnesiumPathArgument.SIZE_MASK) != 0) {
+        if (mask(header, MagnesiumPathArgument.SIZE_MASK) != 0) {
             throw new InvalidNormalizedNodeStreamException("Invalid path argument header " + header);
         }
     }
@@ -842,4 +842,12 @@ abstract class AbstractMagnesiumDataInput extends AbstractNormalizedNodeDataInpu
             throw new InvalidNormalizedNodeStreamException("Invalid bits length " + size);
         }
     }
+
+    private static byte mask(final byte header, final byte mask) {
+        return (byte) (header & mask);
+    }
+
+    private static int rshift(final byte header, final byte shift) {
+        return (header & 0xFF) >>> shift;
+    }
 }

diff --git a/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/NormalizedNodeStreamReaderWriterTest.java b/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/NormalizedNodeStreamReaderWriterTest.java
index 4024cefff85dc74e13817fa28bd3b8d26823ae7f..9456697d86dd5084f5fd6246ec405e5ac7fc7ccd 100644 (file)
--- a/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/NormalizedNodeStreamReaderWriterTest.java
+++ b/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/datastore/node/utils/stream/NormalizedNodeStreamReaderWriterTest.java
@@ -10,11 +10,14 @@ package org.opendaylight.controller.cluster.datastore.node.utils.stream;
 
 import static org.junit.Assert.assertEquals;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.io.ByteStreams;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Optional;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.transform.OutputKeys;
@@ -29,6 +32,7 @@ import org.opendaylight.controller.cluster.datastore.util.TestModel;
 import org.opendaylight.yangtools.yang.common.Empty;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
@@ -293,6 +297,33 @@ public class NormalizedNodeStreamReaderWriterTest {
         assertEquals(expected, nnin.readNormalizedNode());
     }
 
+    @Test
+    public void testAugmentationIdentifier() throws IOException {
+        final List<QName> qnames = new ArrayList<>();
+        for (int i = 0; i < 257; ++i) {
+            qnames.add(QName.create(TestModel.TEST_QNAME, "a" + i));
+        }
+
+        for (int i = 0; i < qnames.size(); ++i) {
+            final AugmentationIdentifier expected = AugmentationIdentifier.create(
+                ImmutableSet.copyOf(qnames.subList(0, i)));
+
+            ByteArrayOutputStream bos = new ByteArrayOutputStream();
+
+            try (NormalizedNodeDataOutput nnout =
+                    NormalizedNodeInputOutput.newDataOutput(ByteStreams.newDataOutput(bos),
+                        NormalizedNodeStreamVersion.SODIUM_SR1)) {
+                nnout.writePathArgument(expected);
+            }
+
+            final byte[] bytes = bos.toByteArray();
+
+            NormalizedNodeDataInput nnin = NormalizedNodeInputOutput.newDataInput(ByteStreams.newDataInput(bytes));
+            PathArgument arg = nnin.readPathArgument();
+            assertEquals(expected, arg);
+        }
+    }
+
     private static String largeString(final int pow) {
         StringBuilder sb = new StringBuilder("X");
         for (int i = 0; i < pow; i++) {