Repository: SINTEF/dlite (language: python)

Issue:
Misspelling of shape in the datamodel may cause DLite to crash
To reproduce the segfault, uncomment the two last lines in the following code segment at end of bindings/python/tests/test_entity.py:

```python
# Test loading invalid json input
with dlite.errctl(hide=True):
    Invalid1 = dlite.get_instance("http://onto-ns.com/meta/0.1/Invalid1")
with raises(dlite.DLiteMissingInstanceError, dlite.DLiteSyntaxError):
    invalid1 = Invalid1([2], properties={"name": "a", "f": [3.14, 2.72]})

# For issue #686. Uncommenting the two last lines will result in segfault
Invalid2 = dlite.get_instance("http://onto-ns.com/meta/0.1/Invalid2")
#with raises(dlite.DLiteMissingInstanceError, dlite.DLiteSyntaxError):
#    invalid2 = Invalid2([2])
```

It seems that the segfault is related to invalid shape name of a 'ref' type property, since Invalid1 produces an expected exception instead of causing a segfault.



Discussion / hints:

Fix patch:
diff --git a/bindings/python/CMakeLists.txt b/bindings/python/CMakeLists.txt
index e307abdc5..24d6a2aa7 100644
--- a/bindings/python/CMakeLists.txt
+++ b/bindings/python/CMakeLists.txt
@@ -8,6 +8,7 @@ set(py_sources
   mappings.py
   datamodel.py
   rdf.py
+  testutils.py
   )
 
 # Python sub-packages
diff --git a/bindings/python/dlite-collection-python.i b/bindings/python/dlite-collection-python.i
index b30482d73..a8392dc00 100644
--- a/bindings/python/dlite-collection-python.i
+++ b/bindings/python/dlite-collection-python.i
@@ -326,4 +326,5 @@ def get_collection(id):
     inst.__class__ = Collection
     return inst
 
+del warn
 %}
diff --git a/bindings/python/dlite-entity-python.i b/bindings/python/dlite-entity-python.i
index 1178fd380..3a0581078 100644
--- a/bindings/python/dlite-entity-python.i
+++ b/bindings/python/dlite-entity-python.i
@@ -2,17 +2,9 @@
 
 /* Python-specific extensions to dlite-entity.i */
 %pythoncode %{
-import sys
-import json
-import base64
 import warnings
 from uuid import UUID
 
-if sys.version_info >= (3, 7):
-    OrderedDict = dict
-else:
-    from collections import OrderedDict
-
 import numpy as np
 
 
@@ -183,7 +175,7 @@ def get_instance(id: str, metaid: str = None, check_storages: bool = True) -> "I
 
     def asdict(self):
         """Returns a dict representation of self."""
-        d = OrderedDict()
+        d = {}
         d['name'] = self.name
         if self.description:
             d['description'] = self.description
@@ -217,7 +209,7 @@ def get_instance(id: str, metaid: str = None, check_storages: bool = True) -> "I
             soft7: Whether to use new SOFT7 format.
             exclude_name: Whether to exclude "name" from the returned dict.
         """
-        d = OrderedDict()
+        d = {}
         if not exclude_name:
             d['name'] = self.name
         d['type'] = self.get_type()
@@ -268,10 +260,10 @@ def get_instance(id: str, metaid: str = None, check_storages: bool = True) -> "I
 
     def asdict(self):
         """Returns a dict representation of self."""
+        d = dict(s=self.s, p=self.p, o=self.o)
         if self.id:
-            d = OrderedDict(s=self.s, p=self.p, o=self.o, id=self.id)
-        else:
-            d = OrderedDict(s=self.s, p=self.p, o=self.o)
+            d[id] = self.id
+
         return d
 
     def asstrings(self):
@@ -353,9 +345,9 @@ def get_instance(id: str, metaid: str = None, check_storages: bool = True) -> "I
 
     meta = property(get_meta, doc="Reference to the metadata of this instance.")
     dimensions = property(
-        lambda self: OrderedDict((d.name, int(v))
-                                 for d, v in zip(self.meta['dimensions'],
-                                                 self.get_dimensions())),
+        lambda self: dict((d.name, int(v))
+                          for d, v in zip(self.meta['dimensions'],
+                                          self.get_dimensions())),
         doc='Dictionary with dimensions name-value pairs.')
     properties = property(lambda self:
         {p.name: self[p.name] for p in self.meta['properties']},
@@ -680,7 +672,7 @@ def get_instance(id: str, metaid: str = None, check_storages: bool = True) -> "I
                 If None, single-entity format is used for metadata and
                 multi-entity format for data instances.
         """
-        dct = d = OrderedDict()
+        dct = d = {}
         if single is None:
             single = self.is_meta
 
@@ -778,6 +770,8 @@ def get_instance(id: str, metaid: str = None, check_storages: bool = True) -> "I
             "Instance.get_copy() is deprecated.  Use Instance.copy() instead.",
             DeprecationWarning, stacklevel=2)
         return self.copy()
+
 %}
 
+
 }
diff --git a/bindings/python/dlite-misc-python.i b/bindings/python/dlite-misc-python.i
index f13575f76..f3b595ed7 100644
--- a/bindings/python/dlite-misc-python.i
+++ b/bindings/python/dlite-misc-python.i
@@ -12,9 +12,13 @@ class errctl():
         hide: Sequence of DLiteException subclasses or exception names
             corresponding to error messages to hide.  A single class or
             name is also allowed.
+            May also be a boolean, in which case error messages will be
+            hidden/shown.
         show: Sequence of DLiteException subclasses or exception names
             corresponding to error messages to show.  A single class or
             name is also allowed.
+            May also be a boolean, in which case error messages will be
+            shown/hidden.
         filename: Filename to redirect errors to.  The following values
             are handled specially:
               - "None" or empty: No output is written.
@@ -23,8 +27,18 @@ class errctl():
 
     """
     def __init__(self, hide=(), show=(), filename="<stderr>"):
-        self.hide = self._as_codes(hide)
-        self.show = self._as_codes(show)
+        allcodes = [-i for i in range(1, _dlite._get_number_of_errors())]
+
+        if hide is True or show is False:
+            self.hide = allcodes
+        elif hide is not False:
+            self.hide = self._as_codes(hide)
+
+        if show is True or hide is False:
+            self.show = allcodes
+        elif show is not False:
+            self.show = self._as_codes(show)
+
         self.filename = filename
 
     def __enter__(self):
diff --git a/bindings/python/dlite-python.i b/bindings/python/dlite-python.i
index 36542818b..f90bc58d8 100644
--- a/bindings/python/dlite-python.i
+++ b/bindings/python/dlite-python.i
@@ -1374,4 +1374,5 @@ int _get_number_of_errors(void);
       setattr(_dlite, exc.__name__, exc)
   DLiteStorageBase = _dlite._get_storage_base()
   DLiteMappingBase = _dlite._get_mapping_base()
+  del n, exc
 %}
diff --git a/requirements_dev.txt b/requirements_dev.txt
index 6f25f0004..67ad54720 100644
--- a/requirements_dev.txt
+++ b/requirements_dev.txt
@@ -2,4 +2,6 @@
 cmake>=1.21.6
 mongomock>=4.1.2,<5
 openpyxl>=3.0.9,<3.2
-pytest>=7,<8
+
+# Requirements for building wheel
+cmake>=1.21.6


Test patch:
diff --git a/bindings/python/tests/input/Invalid1.json b/bindings/python/tests/input/Invalid1.json
new file mode 100644
index 000000000..d7693cff0
--- /dev/null
+++ b/bindings/python/tests/input/Invalid1.json
@@ -0,0 +1,19 @@
+{
+  "uri": "http://onto-ns.com/meta/0.1/Invalid1",
+  "description": "An datamodel for an item with float type and invalid shape name.",
+  "dimensions": {
+      "nf": "Number of eigen-frequencies."
+  },
+  "properties": {
+    "name": {
+      "type": "str",
+      "description": "Name of the item."
+    },
+    "f": {
+      "type": "float64",
+      "shape": ["nf_MISPELLED"],
+      "unit": "Hz",
+      "description": "The magic eigen-frequencies of the item."
+    }
+  }
+}
diff --git a/bindings/python/tests/input/Invalid2.json b/bindings/python/tests/input/Invalid2.json
new file mode 100644
index 000000000..e0a51fdf3
--- /dev/null
+++ b/bindings/python/tests/input/Invalid2.json
@@ -0,0 +1,15 @@
+{
+  "uri": "http://onto-ns.com/meta/0.1/Invalid2",
+  "description": "A datamodel with ref-types with invalid shape name...",
+  "dimensions": {
+    "nrefs": "Number of references to Ref datamodels."
+  },
+  "properties": {
+    "items": {
+      "type": "ref",
+      "$ref": "http://onto-ns.com/meta/0.1/Item",
+      "shape": ["INVALID_nrefs"],
+      "description": "List of Item datamodels."
+    }
+  }
+}
diff --git a/bindings/python/tests/test_entity.py b/bindings/python/tests/test_entity.py
index 2bb5c3848..eb0598682 100755
--- a/bindings/python/tests/test_entity.py
+++ b/bindings/python/tests/test_entity.py
@@ -1,24 +1,19 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
-import os
 import pickle
+from pathlib import Path
 
 import numpy as np
 
 import dlite
 from dlite import Instance, Dimension, Property, Relation
-
-try:
-    import pytest
-
-    HAVE_PYTEST = True
-except ModuleNotFoundError:
-    HAVE_PYTEST = False
+from dlite.testutils import raises
 
 
-thisdir = os.path.abspath(os.path.dirname(__file__))
+thisdir = Path(__file__).absolute().parent
+dlite.storage_path.append(thisdir / "input" / "*.json")
 
-url = "json://" + thisdir + "/MyEntity.json"
+url = f"json://{thisdir}/MyEntity.json"
 
 
 # Load metadata (i.e. an instance of meta-metadata) from url
@@ -295,9 +290,8 @@
 prop = dlite.Property("newprop", "int")
 prop.shape = ("a", "b", "c")
 assert prop.ndims == 3
-if HAVE_PYTEST:
-    with pytest.raises(AttributeError):
-        prop.ndims = 10
+with raises(AttributeError):
+    prop.ndims = 10
 
 
 # Test that metadata is callable, but not instances
@@ -312,3 +306,15 @@
 
 mm = dlite.Instance.from_url("json://entity_schema.json")
 assert mm.uri == dlite.ENTITY_SCHEMA
+
+
+# Test loading invalid json input
+with dlite.errctl(hide=True):
+    Invalid1 = dlite.get_instance("http://onto-ns.com/meta/0.1/Invalid1")
+with raises(dlite.DLiteMissingInstanceError, dlite.DLiteSyntaxError):
+    invalid1 = Invalid1([2], properties={"name": "a", "f": [3.14, 2.72]})
+
+# For issue #686. Uncommenting the two last lines will result in segfault
+Invalid2 = dlite.get_instance("http://onto-ns.com/meta/0.1/Invalid2")
+#with raises(dlite.DLiteMissingInstanceError, dlite.DLiteSyntaxError):
+#    invalid2 = Invalid2([2])
diff --git a/bindings/python/tests/test_storage.py b/bindings/python/tests/test_storage.py
index 96ae5b421..21ec2f946 100755
--- a/bindings/python/tests/test_storage.py
+++ b/bindings/python/tests/test_storage.py
@@ -3,13 +3,7 @@
 import os
 
 import dlite
-
-try:
-    import pytest
-
-    HAVE_PYTEST = True
-except ModuleNotFoundError:
-    HAVE_PYTEST = False
+from dlite.testutils import raises
 
 try:
     import yaml
@@ -113,10 +107,9 @@
 if HAVE_YAML:
     bytearr = inst.to_bytes("yaml")
     #print(bytes(bytearr).decode())
-if HAVE_PYTEST:
-    with pytest.raises(dlite.DLiteError):
-        inst.to_bytes("json")
-    dlite.errclr()
+
+with raises(dlite.DLiteError):
+    inst.to_bytes("json")
 
 
 dlite.storage_path.append(thisdir)
diff --git a/bindings/python/tests/test_testutils.py b/bindings/python/tests/test_testutils.py
new file mode 100644
index 000000000..07a6f61f7
--- /dev/null
+++ b/bindings/python/tests/test_testutils.py
@@ -0,0 +1,26 @@
+"""Test dlite.testutils"""
+from dlite.testutils import (
+    raises, UnexpectedSuccessError, UnexpectedExceptionError
+)
+
+
+with raises(ValueError):
+    raise ValueError("an expected exception, should not be reported")
+
+
+try:
+    with raises(ValueError):
+        pass
+except UnexpectedSuccessError:
+    pass
+else:
+    assert False, "An UnexpectedSuccessError was not produced!"
+
+
+try:
+    with raises(ValueError):
+        raise TypeError("raining an unexpected exception")
+except UnexpectedExceptionError:
+    pass
+else:
+    assert False,  "An UnexpectedExceptionError was not produced!"
diff --git a/bindings/python/tests/test_utils.py b/bindings/python/tests/test_utils.py
index 1c4c872a7..cd2c3351a 100644
--- a/bindings/python/tests/test_utils.py
+++ b/bindings/python/tests/test_utils.py
@@ -14,7 +14,7 @@
 
 
 thisdir = Path(__file__).absolute().parent
-dlite.storage_path.append(thisdir / "input/*.json")
+dlite.storage_path.append(thisdir / "input" / "*.json")
 
 with open(thisdir / "Person.json", "rt") as f:
     d = json.load(f)
diff --git a/bindings/python/testutils.py b/bindings/python/testutils.py
new file mode 100644
index 000000000..b60da1309
--- /dev/null
+++ b/bindings/python/testutils.py
@@ -0,0 +1,57 @@
+"""Some utilities for testing."""
+import dlite
+
+
+class UnexpectedSuccessError(Exception):
+    """Code that was expected to raise an exeption didn't do so."""
+
+
+class UnexpectedExceptionError(Exception):
+    """Code did not raise the expected exception."""
+
+
+class raises():
+    """Assert that a code block raises one of the expected exceptions
+    listed in `exceptions`.
+
+    Arguments:
+        *exceptions: Expected exception classes.
+        silent: Whether to silent DLite error messages in the code block.
+    """
+    def __init__(self, *exceptions, silent=True):
+        self.exceptions = exceptions
+        self.silent = silent
+
+    def __enter__(self):
+        if self.silent:
+            # Silence DLite error messages from expected exceptions and save the
+            # current err_ignore state
+            self.save_silenced = {}
+            for exc in self.exceptions:
+                code = dlite._dlite._err_getcode(exc.__name__)
+                self.save_silenced[code] = dlite._dlite._err_ignore_get(code)
+                dlite._dlite._err_ignore_set(code, 1)
+
+    def __exit__(self, exc_type, exc_value, tb):
+        if self.silent:
+            # Restore the err_ignore state
+            for code, value in self.save_silenced.items():
+                dlite._dlite._err_ignore_set(code, value)
+
+        excnames = ", ".join(repr(exc.__name__) for exc in self.exceptions)
+        if exc_type is None:
+            raise UnexpectedSuccessError(
+                f"Expected one of the following exceptions: {excnames}, "
+                "but no exception was raised."
+            )
+
+        # Leave context manager gracefully if one of the expected exceptions
+        # was raised
+        for exc in self.exceptions:
+            if issubclass(exc_type, exc):
+                return True
+
+        raise UnexpectedExceptionError(
+            f"Expected one of the following exceptions: {excnames}, "
+            f"but got: '{exc_type.__name__}'"
+        ) from exc_value