Fixed the reading of arrays referenced more than once in v2

Arrays were given a handle but were never stored, so a reference to an array could not be resolved. A reference found in an array field was also read as a class description instead of the array itself. Class descriptions are now stored before their annotations, as v3 already does. Fixes #62.

Signed-off-by: Thomas Calmant <thomas.calmant@gmail.com>

Author: tcalmant

javaobj/v2/core.py

@@ -404,14 +404,18 @@ def _do_classdesc(self, type_code):
             class_desc.handle = handle
             class_desc.desc_flags = desc_flags
             class_desc.fields = fields
+
+            # Store the reference to the parsed bean before reading the
+            # annotations and the super class: their content can refer to
+            # this class description
+            self._set_handle(handle, class_desc)
+
             class_desc.annotations = self._read_class_annotations(class_desc)
             class_desc.super_class = self._read_classdesc()
 
             if class_desc.super_class:
                 class_desc.super_class.is_super_class = True
 
-            # Store the reference to the parsed bean
-            self._set_handle(handle, class_desc)
             return class_desc
         elif type_code == TerminalCode.TC_NULL:
             # Null reference
@@ -628,7 +632,9 @@ def _read_field_value(self, field_type):
                     # Seems required, according to issue #46
                     return None
                 if sub_type_code == TerminalCode.TC_REFERENCE:
-                    return self._do_classdesc(sub_type_code)
+                    # Reference to an array which has already been read,
+                    # not to a class description
+                    return self._do_reference(sub_type_code)
                 if sub_type_code != TerminalCode.TC_ARRAY:
                     raise ValueError(
                         "Array type listed, but type code != TC_ARRAY"
@@ -716,7 +722,11 @@ def _do_array(self, type_code):
         else:
             content = [self._read_field_value(field_type) for _ in range(size)]
 
-        return JavaArray(handle, cd, field_type, content)
+        array = JavaArray(handle, cd, field_type, content)
+
+        # Store the array, so that it can be found back by a reference
+        self._set_handle(handle, array)
+        return array
 
     def _do_exception(self, type_code):
         # type: (int) -> ParsedJavaContent

tests/java/src/test/java/SharedArrayExample.java

@@ -0,0 +1,47 @@
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+
+/**
+ * Generates the fixture of issue #62: the same array is stored in two
+ * fields, so it is written once and referenced the second time with a
+ * TC_REFERENCE.
+ *
+ * Reading it required two fixes in v2: arrays were given a handle but were
+ * never stored, so the reference could not be resolved, and a reference
+ * found in an array field was read as a class description.
+ *
+ * The trailing 'marker' field detects a desynchronized stream: it is read
+ * right after the shared array.
+ *
+ * Run it with: java SharedArrayExample.java
+ */
+class SharedArrayHolder implements Serializable {
+    private static final long serialVersionUID = 1L;
+
+    private byte[] first;
+    /** Same array as 'first': written as a reference. */
+    private byte[] second;
+    private String[] strings;
+    /** Same array as 'strings': written as a reference. */
+    private String[] sameStrings;
+    /** Read after the references: wrong if the stream is desynchronized. */
+    private int marker = 443;
+
+    SharedArrayHolder() {
+        first = new byte[] {1, 2, 3};
+        second = first;
+        strings = new String[] {"a", "b"};
+        sameStrings = strings;
+    }
+}
+
+public class SharedArrayExample {
+    public static void main(String[] args) throws IOException {
+        try (ObjectOutputStream oos = new ObjectOutputStream(
+                new FileOutputStream("testSharedArray.ser"))) {
+            oos.writeObject(new SharedArrayHolder());
+        }
+    }
+}

tests/testSharedArray.ser

@@ -586,6 +586,29 @@ def test_collections(self):
 
         # FIXME: referencing problems with the collection class
 
+    def test_shared_array(self):
+        """
+        Tests the reference to an array stored in two fields (issue #62)
+
+        The array is written once and referenced the second time: the
+        reference must be resolved to the array, and not read as a class
+        description.
+        """
+        pobj = javaobj.loads(self.read_file("testSharedArray.ser"))
+
+        self.assertEqual(list(pobj.first), [1, 2, 3])
+        self.assertEqual(list(pobj.second), [1, 2, 3])
+        self.assertEqual(list(pobj.strings), ["a", "b"])
+        self.assertEqual(list(pobj.sameStrings), ["a", "b"])
+
+        # Both fields must give the very same array
+        self.assertIs(pobj.first, pobj.second)
+        self.assertIs(pobj.strings, pobj.sameStrings)
+
+        # Field written after the shared arrays: a wrong value here means
+        # the stream has been desynchronized
+        self.assertEqual(pobj.marker, 443)
+
     def test_jceks_issue_5(self):
         """
         Tests the handling of JCEKS issue #5

tests/test_v2.py

@@ -447,6 +447,21 @@ def test_collections_obj(self) -> None:
         self.assertIsInstance(pobj.hashMap, dict)
         self.assertIsInstance(pobj.linkedList, list)
 
+    def test_shared_array(self) -> None:
+        """testSharedArray.ser - an array referenced by two fields (#62)."""
+        pobj = self.load_bytes("testSharedArray.ser")
+
+        self.assertEqual(list(pobj.first), [1, 2, 3])
+        self.assertEqual(list(pobj.second), [1, 2, 3])
+        self.assertEqual(list(pobj.strings), ["a", "b"])
+
+        # Both fields must give the very same array
+        self.assertIs(pobj.first, pobj.second)
+        self.assertIs(pobj.strings, pobj.sameStrings)
+
+        # Detects a desynchronized stream
+        self.assertEqual(pobj.marker, 443)
+
     def test_bool_int_long(self) -> None:
         """testBoolIntLong.ser – HashMap with Boolean / Integer / Long values."""
         pobj = self.load_bytes("testBoolIntLong.ser")