Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Hotfix: disable autoescape by default when rendering Jinja2 templates (
Browse files Browse the repository at this point in the history
…#8394)

#8037 changed the default `autoescape` option when rendering Jinja2 templates from `False` to `True`. This caused some bugs, noticeably around redirect URLs being escaped in SAML2 auth confirmation templates, causing those URLs to break for users.

This change returns the previous behaviour as it stood. We may want to look at each template individually and see whether autoescaping is a good idea at some point, but for now lets just fix the breakage.
  • Loading branch information
anoadragon453 authored Sep 24, 2020
1 parent d191dbd commit 3f4a2a7
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog.d/8394.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix URLs being accidentally escaped in Jinja2 templates. Broke in v1.20.0.
10 changes: 8 additions & 2 deletions synapse/config/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,10 @@ def read_file(cls, file_path, config_name):
return file_stream.read()

def read_templates(
self, filenames: List[str], custom_template_directory: Optional[str] = None,
self,
filenames: List[str],
custom_template_directory: Optional[str] = None,
autoescape: bool = False,
) -> List[jinja2.Template]:
"""Load a list of template files from disk using the given variables.
Expand All @@ -210,6 +213,9 @@ def read_templates(
custom_template_directory: A directory to try to look for the templates
before using the default Synapse template directory instead.
autoescape: Whether to autoescape variables before inserting them into the
template.
Raises:
ConfigError: if the file's path is incorrect or otherwise cannot be read.
Expand All @@ -233,7 +239,7 @@ def read_templates(
search_directories.insert(0, custom_template_directory)

loader = jinja2.FileSystemLoader(search_directories)
env = jinja2.Environment(loader=loader, autoescape=True)
env = jinja2.Environment(loader=loader, autoescape=autoescape)

# Update the environment with our custom filters
env.filters.update(
Expand Down
4 changes: 3 additions & 1 deletion synapse/config/saml2_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,10 @@ def read_config(self, config, **kwargs):
saml2_config.get("saml_session_lifetime", "15m")
)

# We enable autoescape here as the message may potentially come from a
# remote resource
self.saml2_error_html_template = self.read_templates(
["saml_error.html"], saml2_config.get("template_dir")
["saml_error.html"], saml2_config.get("template_dir"), autoescape=True
)[0]

def _default_saml_config_dict(
Expand Down

0 comments on commit 3f4a2a7

Please sign in to comment.