Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to_serializable #268

Open
raphaelahrens opened this issue Mar 22, 2025 · 0 comments
Open

Fix to_serializable #268

raphaelahrens opened this issue Mar 22, 2025 · 0 comments

Comments

@raphaelahrens
Copy link
Contributor

raphaelahrens commented Mar 22, 2025

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.

pytm/pytm/pytm.py

Lines 1999 to 2002 in 0c8f23c

@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)

pytm/pytm/pytm.py

Lines 2005 to 2016 in 0c8f23c

@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

pytm/pytm/pytm.py

Line 2007 in 0c8f23c

return serialize(obj, nested=True)

Because of this check for not nested in serialize().

pytm/pytm/pytm.py

Line 2048 in 0c8f23c

not nested

pytm/pytm/pytm.py

Lines 2019 to 2054 in 0c8f23c

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.

pytm/pytm/pytm.py

Line 1272 in 0c8f23c

for k, v in serialize(e).items():

Is is unclear if it can be replaced with to_serializable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant