In a recent PR #263 the to_serializable function misbehaved and serialized a set of strings into the string "{'pytm/threatlib/threats.json'}".
The issue is caused by the default to_serializable, which is used because there seems to be no rule to serialize the member variable threatsFile of the class TM.
to_serializable is a @singledispatch function.
|
@singledispatch |
|
def to_serializable(val): |
|
"""Used by default.""" |
|
return str(val) |
But the potential of @singledispatch is not fully used here since the it is only used in the default variant (seen above) and the two overload variants serialize(obj, nested=True) and serialize(obj, nested=False)
|
@to_serializable.register(TM) |
|
def ts_tm(obj): |
|
return serialize(obj, nested=True) |
|
|
|
|
|
@to_serializable.register(Controls) |
|
@to_serializable.register(Data) |
|
@to_serializable.register(Threat) |
|
@to_serializable.register(Element) |
|
@to_serializable.register(Finding) |
|
def ts_element(obj): |
|
return serialize(obj, nested=False) |
The reason why the string "{'pytm/threatlib/threats.json'}" is created is that the default to_serializable is used.
This is just the same as
json.dumps(str(set(["./a.json"])))
The issue is created by
|
return serialize(obj, nested=True) |
Because of this check for not nested in serialize().
|
def serialize(obj, nested=False): |
|
"""Used if *obj* is an instance of TM, Element, Threat or Finding.""" |
|
klass = obj.__class__ |
|
result = {} |
|
if isinstance(obj, (Actor, Asset)): |
|
result["__class__"] = klass.__name__ |
|
for i in dir(obj): |
|
if ( |
|
i.startswith("__") |
|
or callable(getattr(klass, i, {})) |
|
or ( |
|
isinstance(obj, TM) |
|
and i in ("_sf", "_duplicate_ignored_attrs", "_threats") |
|
) |
|
or (isinstance(obj, Element) and i in ("_is_drawn", "uuid")) |
|
or (isinstance(obj, Finding) and i == "element") |
|
): |
|
continue |
|
value = getattr(obj, i) |
|
if isinstance(obj, TM) and i == "_elements": |
|
value = [e for e in value if isinstance(e, (Actor, Asset))] |
|
if value is not None: |
|
if isinstance(value, (Element, Data)): |
|
value = value.name |
|
elif isinstance(obj, Threat) and i == "target": |
|
value = [v.__name__ for v in value] |
|
elif i in ("levels", "sourceFiles", "assumptions"): |
|
value = list(value) |
|
elif ( |
|
not nested |
|
and not isinstance(value, str) |
|
and isinstance(value, Iterable) |
|
): |
|
value = [v.id if isinstance(v, Finding) else v.name for v in value] |
|
result[i.lstrip("_")] = value |
|
return result |
The serialize function is overloaded with special handling of member variables and requires a rewrite.
All the checks seem to be for specific classes which are all handle in the same function.
Also why is there the check for nested?
This is basically equivalent to checking if the class is TM.
In summary we have two types of overloading in the function to_serializable, one via @singledispatch and one via the use of the use of isinstance and the nested parameter in serialize.
I propose to move to just use @singledispatch and move the isinstance functionality of serialize into to_serializable. Maybe remove serialize in the process or at least make it less complex.
The serialize function is also used in sqlDump.
|
for k, v in serialize(e).items(): |
Is is unclear if it can be replaced with
to_serializable.
In a recent PR #263 the
to_serializablefunction misbehaved and serialized a set of strings into the string"{'pytm/threatlib/threats.json'}".The issue is caused by the default
to_serializable, which is used because there seems to be no rule to serialize the member variablethreatsFileof the classTM.to_serializableis a @singledispatch function.pytm/pytm/pytm.py
Lines 1999 to 2002 in 0c8f23c
But the potential of
@singledispatchis not fully used here since the it is only used in the default variant (seen above) and the two overload variantsserialize(obj, nested=True)andserialize(obj, nested=False)pytm/pytm/pytm.py
Lines 2005 to 2016 in 0c8f23c
The reason why the string
"{'pytm/threatlib/threats.json'}"is created is that the defaultto_serializableis used.This is just the same as
The issue is created by
pytm/pytm/pytm.py
Line 2007 in 0c8f23c
Because of this check for
not nestedinserialize().pytm/pytm/pytm.py
Line 2048 in 0c8f23c
pytm/pytm/pytm.py
Lines 2019 to 2054 in 0c8f23c
The
serializefunction is overloaded with special handling of member variables and requires a rewrite.All the checks seem to be for specific classes which are all handle in the same function.
Also why is there the check for
nested?This is basically equivalent to checking if the class is
TM.In summary we have two types of overloading in the function
to_serializable, one via@singledispatchand one via the use of the use ofisinstanceand thenestedparameter inserialize.I propose to move to just use
@singledispatchand move theisinstancefunctionality ofserializeintoto_serializable. Maybe removeserializein the process or at least make it less complex.The
serializefunction is also used insqlDump.pytm/pytm/pytm.py
Line 1272 in 0c8f23c
Is is unclear if it can be replaced with
to_serializable.