Merge pull request #5119 from rtfd/humitos/validate-mkdocs-configs

Validate mkdocs.yml config on values that we manipulate

Author: humitos

readthedocs/doc_builder/backends/mkdocs.py

@@ -84,7 +84,7 @@ def load_yaml_config(self):
         """
         Load a YAML config.
 
-        Raise BuildEnvironmentError if failed due to syntax errors.
+        :raises: ``MkDocsYAMLParseError`` if failed due to syntax errors.
         """
         try:
             return yaml.safe_load(
@@ -105,20 +105,40 @@ def load_yaml_config(self):
             )
 
     def append_conf(self, **__):
-        """Set mkdocs config values."""
+        """
+        Set mkdocs config values.
+
+        :raises: ``MkDocsYAMLParseError`` if failed due to known type errors
+                 (i.e. expecting a list and a string is found).
+        """
         if not self.yaml_file:
             self.yaml_file = os.path.join(self.root_path, 'mkdocs.yml')
 
         user_config = self.load_yaml_config()
 
         # Handle custom docs dirs
         user_docs_dir = user_config.get('docs_dir')
+        if not isinstance(user_docs_dir, (type(None), str)):
+            raise MkDocsYAMLParseError(
+                MkDocsYAMLParseError.INVALID_DOCS_DIR_CONFIG,
+            )
+
         docs_dir = self.docs_dir(docs_dir=user_docs_dir)
         self.create_index(extension='md')
         user_config['docs_dir'] = docs_dir
 
         # Set mkdocs config values
         static_url = get_absolute_static_url()
+
+        for config in ('extra_css', 'extra_javascript'):
+            user_value = user_config.get(config, [])
+            if not isinstance(user_value, list):
+                raise MkDocsYAMLParseError(
+                    MkDocsYAMLParseError.INVALID_EXTRA_CONFIG.format(
+                        config=config,
+                    ),
+                )
+
         user_config.setdefault('extra_javascript', []).extend([
             'readthedocs-data.js',
             '%score/js/readthedocs-doc-embed.js' % static_url,

readthedocs/doc_builder/exceptions.py

@@ -59,3 +59,13 @@ class MkDocsYAMLParseError(BuildEnvironmentError):
     GENERIC_WITH_PARSE_EXCEPTION = ugettext_noop(
         'Problem parsing MkDocs YAML configuration. {exception}',
     )
+
+    INVALID_DOCS_DIR_CONFIG = ugettext_noop(
+        'The "docs_dir" config from your MkDocs YAML config file has to be a '
+        'string with relative or absolute path.',
+    )
+
+    INVALID_EXTRA_CONFIG = ugettext_noop(
+        'The "{config}" config from your MkDocs YAML config file has to be a '
+        'a list of relative paths.',
+    )

readthedocs/rtd_tests/tests/test_doc_builder.py

@@ -18,6 +18,7 @@
 from readthedocs.builds.models import Version
 from readthedocs.doc_builder.backends.mkdocs import MkdocsHTML
 from readthedocs.doc_builder.backends.sphinx import BaseSphinx
+from readthedocs.doc_builder.exceptions import MkDocsYAMLParseError
 from readthedocs.doc_builder.python_environments import Virtualenv
 from readthedocs.projects.exceptions import ProjectConfigurationError
 from readthedocs.projects.models import Feature, Project
@@ -388,6 +389,39 @@ def test_append_conf_existing_yaml_on_root(self, checkout_path, run):
             'mkdocs'
         )
 
+    @patch('readthedocs.doc_builder.base.BaseBuilder.run')
+    @patch('readthedocs.projects.models.Project.checkout_path')
+    def test_append_conf_existing_yaml_on_root_with_invalid_setting(self, checkout_path, run):
+        tmpdir = tempfile.mkdtemp()
+        os.mkdir(os.path.join(tmpdir, 'docs'))
+        yaml_file = os.path.join(tmpdir, 'mkdocs.yml')
+        checkout_path.return_value = tmpdir
+
+        python_env = Virtualenv(
+            version=self.version,
+            build_env=self.build_env,
+            config=None,
+        )
+        self.searchbuilder = MkdocsHTML(
+            build_env=self.build_env,
+            python_env=python_env,
+        )
+
+        # We can't use ``@pytest.mark.parametrize`` on a Django test case
+        yaml_contents = [
+            {'docs_dir': ['docs']},
+            {'extra_css': 'a string here'},
+            {'extra_javascript': None},
+        ]
+        for content in yaml_contents:
+            yaml.safe_dump(
+                content,
+                open(yaml_file, 'w'),
+            )
+            with self.assertRaises(MkDocsYAMLParseError):
+                self.searchbuilder.append_conf()
+
+
     @patch('readthedocs.doc_builder.base.BaseBuilder.run')
     @patch('readthedocs.projects.models.Project.checkout_path')
     def test_dont_override_theme(self, checkout_path, run):