Skip to content

Validate mkdocs.yml config on values that we manipulate #5119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions readthedocs/doc_builder/backends/mkdocs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
Expand Down
10 changes: 10 additions & 0 deletions readthedocs/doc_builder/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels weird to quote this, we should just say it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I read this wrong.

)

INVALID_EXTRA_CONFIG = ugettext_noop(
'The "{config}" config from your MkDocs YAML config file has to be a '
'a list of relative paths.',
)
34 changes: 34 additions & 0 deletions readthedocs/rtd_tests/tests/test_doc_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down