From 2df203ee98e79a744719ba6b581f48dd604e395f Mon Sep 17 00:00:00 2001
From: Manuel Kaufmann <humitos@gmail.com>
Date: Wed, 16 Jan 2019 11:45:27 +0100
Subject: [PATCH] Validate mkdocs.yml config on values that we manipulate

Make sure that the values we will manipulate from the mkdocs.yml file
the correct type we need them to be. If we found a problem with them,
we report a proper message to user to debug it by themselves.
---
 readthedocs/doc_builder/backends/mkdocs.py    | 24 +++++++++++--
 readthedocs/doc_builder/exceptions.py         | 10 ++++++
 .../rtd_tests/tests/test_doc_builder.py       | 34 +++++++++++++++++++
 3 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/readthedocs/doc_builder/backends/mkdocs.py b/readthedocs/doc_builder/backends/mkdocs.py
index 6119673b8fa..67df595bd33 100644
--- a/readthedocs/doc_builder/backends/mkdocs.py
+++ b/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,7 +105,12 @@ 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')
 
@@ -113,12 +118,27 @@ def append_conf(self, **__):
 
         # 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,
diff --git a/readthedocs/doc_builder/exceptions.py b/readthedocs/doc_builder/exceptions.py
index ce2ce3d844b..362b18015fe 100644
--- a/readthedocs/doc_builder/exceptions.py
+++ b/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.',
+    )
diff --git a/readthedocs/rtd_tests/tests/test_doc_builder.py b/readthedocs/rtd_tests/tests/test_doc_builder.py
index a384f5d00b7..b8522679346 100644
--- a/readthedocs/rtd_tests/tests/test_doc_builder.py
+++ b/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):