Skip to content

Commit

Permalink
Dependencies: update requirement to pyyaml~=5.4 (#221)
Browse files Browse the repository at this point in the history
The versions of `pyyaml` up to v5.4 contained severe security issues
where the default loaders could be abused for arbitrary code execution.
The default `FullLoader` was patched to, but as a result, data sets that
could be successfully deserialized with it, now will fail. The update
caused a few tests to fail.

The `rmq.test_communications:test_launch_nowait` test was hanging
because the communicator that was being used, used the default decoder
that is defined for the `Communicator`. This is defined in `kiwipy` to
be the `FullLoader`. This safe loader can no longer be used since our
message payloads cannot be deserialized by it, causing the message
receive to fail, which caused the test to hang since it was waiting
indefinitely for a response. The solution is to change the decoder for
the communicator in the test to use the `Loader` which can load the
payloads.

Next, the test `rmq.test_process_comms:test_status` was also hanging for
a very similar reason. The `Process.get_status_info` method returns a
dictionary containing an instance of the `ProcessState` enum. The new
`FullLoader` refuses to load this for safety reasons and so the message
receiver excepts causing the test to hang waiting for the response. The
solution is to serialize the value ourselves simply by converting it to
its string representation. Note that this was already being done for the
`status_info` key, duplicating the information, which is therefore
removed.

Finally, `mypy` complained about using `yaml.Loader` directly. This is
fixed by using the explicit import path for the JSON loader in the
`Excepted.load_instance_state`. Note that `Loader` is identical to
`UnsafeLoader` and the latter is merely kept for backward compatiblity
purposes, so here we change `UnsafeLoader` with the preferred `Loader`.
  • Loading branch information
sphuber authored Aug 10, 2021
1 parent 77e3820 commit 009b3c3
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ repos:
hooks:
- id: pylint
additional_dependencies: [
"pyyaml~=5.1.2", "nest_asyncio~=1.4.0", "aio-pika~=6.6",
"pyyaml~=5.4", "nest_asyncio~=1.4.0", "aio-pika~=6.6",
"aiocontextvars~=0.2.2; python_version<'3.7'", "kiwipy[rmq]~=0.7.4"
]
args:
Expand Down
3 changes: 2 additions & 1 deletion plumpy/process_states.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from typing import Any, Callable, cast, Optional, Tuple, Type, TYPE_CHECKING, Union

import yaml
from yaml.loader import Loader

try:
import tblib
Expand Down Expand Up @@ -373,7 +374,7 @@ def save_instance_state(self, out_state: SAVED_STATE_TYPE, save_context: persist

def load_instance_state(self, saved_state: SAVED_STATE_TYPE, load_context: persistence.LoadSaveContext) -> None:
super().load_instance_state(saved_state, load_context)
self.exception = yaml.load(saved_state[self.EXC_VALUE], Loader=yaml.FullLoader)
self.exception = yaml.load(saved_state[self.EXC_VALUE], Loader=Loader)
if _HAS_TBLIB:
try:
self.traceback = \
Expand Down
3 changes: 1 addition & 2 deletions plumpy/processes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,5 @@ def get_status_info(self, out_status_info: dict) -> None:
'ctime': self.creation_time,
'paused': self.paused,
'process_string': str(self),
'state': self.state,
'state_info': str(self._state)
'state': str(self.state),
})
4 changes: 2 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@
keywords='workflow multithreaded rabbitmq',
python_requires='>=3.6',
install_requires=[
'pyyaml~=5.1.2', 'nest_asyncio~=1.4.0', 'aio-pika~=6.6', 'aiocontextvars~=0.2.2; python_version<"3.7"',
'pyyaml~=5.4', 'nest_asyncio~=1.4.0', 'aio-pika~=6.6', 'aiocontextvars~=0.2.2; python_version<"3.7"',
'kiwipy[rmq]~=0.7.4'
],
extras_require={
'docs': ['sphinx~=3.2.0', 'myst-nb~=0.11.0', 'sphinx-book-theme~=0.0.39', 'ipython~=7.0'],
'pre-commit': ['mypy==0.790', 'pre-commit~=2.2', 'pylint==2.5.2'],
'tests': ['pytest~=5.4', 'shortuuid', 'pytest-asyncio', 'pytest-cov', 'pytest-notebook']
'tests': ['pytest~=5.4', 'shortuuid', 'pytest-asyncio', 'pytest-cov', 'pytest-notebook', 'ipykernel']
},
packages=['plumpy', 'plumpy/base'],
include_package_data=True,
Expand Down
3 changes: 3 additions & 0 deletions test/rmq/test_communicator.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import uuid
import asyncio
import shortuuid
import functools
import yaml

import pytest
from kiwipy import BroadcastFilter, rmq
Expand Down Expand Up @@ -35,6 +37,7 @@ def loop_communicator():
message_exchange=message_exchange,
task_exchange=task_exchange,
task_queue=task_queue,
decoder=functools.partial(yaml.load, Loader=yaml.Loader)
)

loop = asyncio.get_event_loop()
Expand Down

0 comments on commit 009b3c3

Please sign in to comment.