diff --git a/readthedocs/config/tests/test_yaml_loader.py b/readthedocs/config/tests/test_yaml_loader.py index 0f39548afc3..89cf53dff30 100644 --- a/readthedocs/config/tests/test_yaml_loader.py +++ b/readthedocs/config/tests/test_yaml_loader.py @@ -1,20 +1,38 @@ -import textwrap -from readthedocs.doc_builder.backends.mkdocs import yaml_load_safely +from readthedocs.doc_builder.backends.mkdocs import ( + yaml_dump_safely, + yaml_load_safely, + ProxyPythonName +) + +content = ''' +int: 3 +float: !!float 3 +function: !!python/name:python_function +other_function: !!python/name:module.other.function +unknown: !!python/module:python_module +''' def test_yaml_load_safely(): expected = { 'int': 3, 'float': 3.0, + 'function': ProxyPythonName('python_function'), + 'other_function': ProxyPythonName('module.other.function'), 'unknown': None, } - - content = textwrap.dedent(''' - int: 3 - float: !!float 3 - unknown: !!python/name:unknown_funtion - ''') data = yaml_load_safely(content) + assert data == expected assert type(data['int']) is int assert type(data['float']) is float + assert type(data['function']) is ProxyPythonName + assert type(data['other_function']) is ProxyPythonName + assert data['function'].value == 'python_function' + assert data['other_function'].value == 'module.other.function' + + +def test_yaml_dump_safely(): + data = yaml_load_safely(content) + assert yaml_load_safely(yaml_dump_safely(data)) == data + diff --git a/readthedocs/doc_builder/backends/mkdocs.py b/readthedocs/doc_builder/backends/mkdocs.py index 62c797ebd1c..8569f588b9c 100644 --- a/readthedocs/doc_builder/backends/mkdocs.py +++ b/readthedocs/doc_builder/backends/mkdocs.py @@ -214,7 +214,7 @@ def append_conf(self): user_config['theme'] = self.DEFAULT_THEME_NAME # Write the modified mkdocs configuration - yaml.safe_dump( + yaml_dump_safely( user_config, open(self.yaml_file, 'w'), ) @@ -326,27 +326,67 @@ class MkdocsHTML(BaseMkdocs): build_dir = '_build/html' -class SafeLoaderIgnoreUnknown(yaml.SafeLoader): # pylint: disable=too-many-ancestors +# TODO: find a better way to integrate with MkDocs. +# See https://github.com/readthedocs/readthedocs.org/issues/7844 + + +class ProxyPythonName(yaml.YAMLObject): + def __init__(self, value): + self.value = value + + def __eq__(self, other): + return self.value == other.value + + +class SafeLoader(yaml.SafeLoader): # pylint: disable=too-many-ancestors """ - YAML loader to ignore unknown tags. + Safe YAML loader. + + This loader parses special ``!!python/name:`` tags without actually + importing or executing code. Every other special tag is ignored. Borrowed from https://stackoverflow.com/a/57121993 + Issue https://github.com/readthedocs/readthedocs.org/issues/7461 """ def ignore_unknown(self, node): # pylint: disable=no-self-use, unused-argument return None + def construct_python_name(self, suffix, node): # pylint: disable=no-self-use, unused-argument + return ProxyPythonName(suffix) + + +class SafeDumper(yaml.SafeDumper): -SafeLoaderIgnoreUnknown.add_constructor(None, SafeLoaderIgnoreUnknown.ignore_unknown) + """ + Safe YAML dumper. + + This dumper allows to avoid losing values of special tags that + were parsed by our safe loader. + """ + + def represent_name(self, data): + return self.represent_scalar("tag:yaml.org,2002:python/name:" + data.value, "") + + +SafeLoader.add_multi_constructor("tag:yaml.org,2002:python/name:", SafeLoader.construct_python_name) +SafeLoader.add_constructor(None, SafeLoader.ignore_unknown) +SafeDumper.add_representer(ProxyPythonName, SafeDumper.represent_name) def yaml_load_safely(content): """ - Uses ``SafeLoaderIgnoreUnknown`` loader to skip unknown tags. + Uses ``SafeLoader`` loader to skip unknown tags. - When a YAML contains ``!!python/name:int`` it will complete ignore it an - return ``None`` for those fields instead of failing. We need this to avoid - executing random code, but still support these YAML files. + When a YAML contains ``!!python/name:int`` it will store the ``int`` + suffix temporarily to be able to re-dump it later. We need this to avoid + executing random code, but still support these YAML files without + information loss. """ - return yaml.load(content, Loader=SafeLoaderIgnoreUnknown) + return yaml.load(content, Loader=SafeLoader) + + +def yaml_dump_safely(content, stream=None): + """Uses ``SafeDumper`` dumper to write YAML contents.""" + return yaml.dump(content, stream=stream, Dumper=SafeDumper) diff --git a/readthedocs/rtd_tests/tests/test_doc_builder.py b/readthedocs/rtd_tests/tests/test_doc_builder.py index 5622e1e956c..6efe5825e6e 100644 --- a/readthedocs/rtd_tests/tests/test_doc_builder.py +++ b/readthedocs/rtd_tests/tests/test_doc_builder.py @@ -13,7 +13,7 @@ from readthedocs.builds.constants import EXTERNAL from readthedocs.builds.models import Version -from readthedocs.doc_builder.backends.mkdocs import MkdocsHTML, yaml_load_safely +from readthedocs.doc_builder.backends.mkdocs import MkdocsHTML, SafeDumper, yaml_load_safely from readthedocs.doc_builder.backends.sphinx import ( BaseSphinx, HtmlBuilder, @@ -392,7 +392,7 @@ def test_get_theme_name_with_feature_flag(self, checkout_path, run): } builder.append_conf() - mock_yaml.safe_dump.assert_called_once_with( + mock_yaml.dump.assert_called_once_with( { 'site_name': mock.ANY, 'docs_dir': mock.ANY, @@ -401,7 +401,8 @@ def test_get_theme_name_with_feature_flag(self, checkout_path, run): 'google_analytics': mock.ANY, 'theme': 'readthedocs', }, - mock.ANY, + stream=mock.ANY, + Dumper=SafeDumper, ) mock_yaml.reset_mock() @@ -417,7 +418,7 @@ def test_get_theme_name_with_feature_flag(self, checkout_path, run): } builder.append_conf() - mock_yaml.safe_dump.assert_called_once_with( + mock_yaml.dump.assert_called_once_with( { 'site_name': mock.ANY, 'docs_dir': mock.ANY, @@ -426,7 +427,8 @@ def test_get_theme_name_with_feature_flag(self, checkout_path, run): 'google_analytics': mock.ANY, 'theme': 'customtheme', }, - mock.ANY, + stream=mock.ANY, + Dumper=SafeDumper, ) @patch('readthedocs.doc_builder.base.BaseBuilder.run')