Skip to content

Commit

Permalink
yaml: Raise an exception when dumping unsupported types
Browse files Browse the repository at this point in the history
Failure to represent an object is a bug so it should be noisy, not
quietly produce `NULL`.  The representer this commit disables was
added in commit 2e2ce5f.

Behavior before:

```pycon
>>> import salt.utils.yaml as y
>>> y.safe_dump(type("GeneratedObjectSubclass", (), {}))
'NULL\n'
```

Behavior after:

```pycon
>>> import salt.utils.yaml as y
>>> y.safe_dump(type("GeneratedObjectSubclass", (), {}))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "salt/utils/yamldumper.py", line 270, in safe_dump
    return dump(data, stream, Dumper=SafeOrderedDumper, **kwargs)
  File "salt/utils/yamldumper.py", line 261, in dump
    return yaml.dump(data, stream, **kwargs)
  File "venv/lib/python3.8/site-packages/yaml/__init__.py", line 253, in dump
    return dump_all([data], stream, Dumper=Dumper, **kwds)
  File "venv/lib/python3.8/site-packages/yaml/__init__.py", line 241, in dump_all
    dumper.represent(data)
  File "venv/lib/python3.8/site-packages/yaml/representer.py", line 27, in represent
    node = self.represent_data(data)
  File "venv/lib/python3.8/site-packages/yaml/representer.py", line 58, in represent_data
    node = self.yaml_representers[None](self, data)
  File "venv/lib/python3.8/site-packages/yaml/representer.py", line 231, in represent_undefined
    raise RepresenterError("cannot represent an object", data)
yaml.representer.RepresenterError: ('cannot represent an object', <class '__main__.GeneratedObjectSubclass'>)
```
  • Loading branch information
rhansen committed Dec 19, 2022
1 parent bd2b756 commit ac8d18e
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 3 deletions.
4 changes: 4 additions & 0 deletions changelog/62932.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,7 @@ but can be previewed now by setting the option `yaml_compatibility` to `3007`:
instead of `yaml.Dumper`.
* Dumping a YAML sequence with `salt.utils.yaml.IndentedSafeOrderedDumper`
will be properly indented.
* Dumping an object with an unsupported type via `salt.utils.yaml.safe_dump()`
(or via `dump()` with the `SafeOrderedDumper` or `IndentedSafeOrderedDumper`
classes) will raise an exception. (Currently such objects produce a null
node.)
9 changes: 9 additions & 0 deletions doc/topics/troubleshooting/yaml_idiosyncrasies.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ unexpected times.
YAML compatibility warnings can be disabled by setting the
``yaml_compatibility_warnings`` option to False.

.. versionchanged:: 3007.0

Dumping an object of unsupported type to YAML with a one of the safe dumpers
(:py:func:`~salt.utils.yaml.safe_dump`, or :py:func:`~salt.utils.yaml.dump`
with the :py:class:`~salt.utils.yaml.SafeOrderedDumper` or
:py:class:`~salt.utils.yaml.IndentedSafeOrderedDumper` classes) now raises
an exception. Previously it produced a YAML ``NULL`` node. Set the
``yaml_compatibility`` option to 3006 to revert to the previous behavior.

Spaces vs Tabs
==============

Expand Down
4 changes: 1 addition & 3 deletions salt/utils/yamldumper.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,7 @@ def _rep_default(self, data):
return self.represent_scalar("tag:yaml.org,2002:null", "NULL")


# TODO: Why does this registration exist? Isn't it better to raise an exception
# for unsupported types?
_CommonMixin.add_representer(None, _CommonMixin._rep_default)
_CommonMixin.V3006.add_representer(None, _CommonMixin._rep_default)
_CommonMixin.V3006.add_representer(
OrderedDict, _CommonMixin._rep_ordereddict_as_plain_map
)
Expand Down
37 changes: 37 additions & 0 deletions tests/pytests/unit/utils/test_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,43 @@ def test_dump_tuple(mktuple, dumpercls):
assert got == want


@pytest.mark.parametrize("obj", [type("Generated", (), {})()])
@pytest.mark.parametrize(
"yaml_compatibility,dumpercls,want",
[
(
3006,
salt_yaml.OrderedDumper,
"!!python/object:tests.pytests.unit.utils.test_yaml.Generated {}\n",
),
# With v3006, sometimes it prints "..." on a new line as an end of
# document indicator depending on whether yaml.CSafeDumper is available
# on the system or not (yaml.CSafeDumper and yaml.SafeDumper do not
# always behave the same).
(3006, salt_yaml.SafeOrderedDumper, re.compile(r"NULL\n(?:\.\.\.\n)?")),
(3006, salt_yaml.IndentedSafeOrderedDumper, re.compile(r"NULL\n(?:\.\.\.\n)?")),
(
3007,
salt_yaml.OrderedDumper,
"!!python/object:tests.pytests.unit.utils.test_yaml.Generated {}\n",
),
(3007, salt_yaml.SafeOrderedDumper, yaml.representer.RepresenterError),
(3007, salt_yaml.IndentedSafeOrderedDumper, yaml.representer.RepresenterError),
],
indirect=["yaml_compatibility"],
)
def test_dump_unsupported(yaml_compatibility, dumpercls, obj, want):
if isinstance(want, type) and issubclass(want, Exception):
with pytest.raises(want):
salt_yaml.dump(obj, Dumper=dumpercls)
else:
got = salt_yaml.dump(obj, Dumper=dumpercls)
try:
assert want.fullmatch(got)
except AttributeError:
assert got == want


def render_yaml(data):
"""
Takes a YAML string, puts it into a mock file, passes that to the YAML
Expand Down

0 comments on commit ac8d18e

Please sign in to comment.