From 80462b656f0209f81c89482b6fda7cd3c39d6a53 Mon Sep 17 00:00:00 2001 From: Tobias Deiminger Date: Sun, 10 Nov 2024 13:14:45 +0100 Subject: [PATCH] Mark diff colors safe and escape raw diff input For HTML escaping of the diff view we have to consider two things. 1. Diff input comes from two git checkouts of the project at specific revisions. The revisions sdocs are considered untrusted user input, could contain special characters and must be escaped. 2. After analyzing with difflib we add a bit HTML to colorize the output. This specific HTML fragments are trusted and safe. Relates to #1920. --- .../diff_screen_results_view_object.py | 20 ++++++++++--------- .../view_objects/diff_screen_view_object.py | 18 +++++++++-------- strictdoc/git/project_diff_analyzer.py | 18 ++++++++--------- strictdoc/helpers/diff.py | 12 ++++++----- 4 files changed, 37 insertions(+), 31 deletions(-) diff --git a/strictdoc/export/html/generators/view_objects/diff_screen_results_view_object.py b/strictdoc/export/html/generators/view_objects/diff_screen_results_view_object.py index 626bbff45..ad8162361 100644 --- a/strictdoc/export/html/generators/view_objects/diff_screen_results_view_object.py +++ b/strictdoc/export/html/generators/view_objects/diff_screen_results_view_object.py @@ -3,6 +3,8 @@ from datetime import datetime from typing import Optional +from markupsafe import Markup + from strictdoc import __version__ from strictdoc.core.project_config import ProjectConfig from strictdoc.export.html.html_templates import JinjaEnvironment @@ -58,31 +60,31 @@ def __init__( self.strictdoc_version = __version__ self.error_message: Optional[str] = None - def render_screen(self, jinja_environment: JinjaEnvironment): + def render_screen(self, jinja_environment: JinjaEnvironment) -> Markup: template = jinja_environment.environment.overlay( autoescape=False ).get_template("screens/git/index.jinja") - return template.render(view_object=self) + return Markup(template.render(view_object=self)) - def render_url(self, url: str): - return self.link_renderer.render_url(url) + def render_url(self, url: str) -> Markup: + return Markup(self.link_renderer.render_url(url)) - def render_node_link(self, incoming_link, document, document_type): + def render_node_link(self, incoming_link, document, document_type) -> str: return self.link_renderer.render_node_link( incoming_link, document, document_type ) - def render_static_url(self, url: str): - return self.link_renderer.render_static_url(url) + def render_static_url(self, url: str) -> Markup: + return Markup(self.link_renderer.render_static_url(url)) def render_static_url_with_prefix(self, url: str) -> str: return self.link_renderer.render_static_url_with_prefix(url) - def render_local_anchor(self, node): + def render_local_anchor(self, node) -> str: return self.link_renderer.render_local_anchor(node) def is_empty_tree(self) -> bool: return self.document_tree_iterator.is_empty_tree() - def date_today(self): + def date_today(self) -> str: return datetime.today().strftime("%Y-%m-%d") diff --git a/strictdoc/export/html/generators/view_objects/diff_screen_view_object.py b/strictdoc/export/html/generators/view_objects/diff_screen_view_object.py index 003139136..ed223d2b7 100644 --- a/strictdoc/export/html/generators/view_objects/diff_screen_view_object.py +++ b/strictdoc/export/html/generators/view_objects/diff_screen_view_object.py @@ -2,6 +2,8 @@ from dataclasses import dataclass from datetime import datetime +from markupsafe import Markup + from strictdoc import __version__ from strictdoc.core.project_config import ProjectConfig from strictdoc.export.html.html_templates import JinjaEnvironment @@ -39,27 +41,27 @@ def __init__( self.is_running_on_server: bool = project_config.is_running_on_server self.strictdoc_version = __version__ - def render_screen(self, jinja_environment: JinjaEnvironment): + def render_screen(self, jinja_environment: JinjaEnvironment) -> Markup: return jinja_environment.render_template_as_markup( "screens/git/index.jinja", view_object=self ) - def render_url(self, url: str): - return self.link_renderer.render_url(url) + def render_url(self, url: str) -> Markup: + return Markup(self.link_renderer.render_url(url)) - def render_node_link(self, incoming_link, document, document_type): + def render_node_link(self, incoming_link, document, document_type) -> str: return self.link_renderer.render_node_link( incoming_link, document, document_type ) - def render_static_url(self, url: str): - return self.link_renderer.render_static_url(url) + def render_static_url(self, url: str) -> Markup: + return Markup(self.link_renderer.render_static_url(url)) def render_static_url_with_prefix(self, url: str) -> str: return self.link_renderer.render_static_url_with_prefix(url) - def render_local_anchor(self, node): + def render_local_anchor(self, node) -> str: return self.link_renderer.render_local_anchor(node) - def date_today(self): + def date_today(self) -> str: return datetime.today().strftime("%Y-%m-%d") diff --git a/strictdoc/git/project_diff_analyzer.py b/strictdoc/git/project_diff_analyzer.py index 342cc228c..3e3b94bfb 100644 --- a/strictdoc/git/project_diff_analyzer.py +++ b/strictdoc/git/project_diff_analyzer.py @@ -24,7 +24,7 @@ SectionChange, ) from strictdoc.helpers.cast import assert_cast, assert_optional_cast -from strictdoc.helpers.diff import get_colored_diff_string, similar +from strictdoc.helpers.diff import get_colored_html_diff_string, similar from strictdoc.helpers.mid import MID @@ -366,12 +366,12 @@ def _iterate_one_index( if other_document_or_none is not None: if document.title != other_document_or_none.title: title_modified = True - lhs_colored_title_diff = get_colored_diff_string( + lhs_colored_title_diff = get_colored_html_diff_string( document.title, other_document_or_none.title, "left", ) - rhs_colored_title_diff = get_colored_diff_string( + rhs_colored_title_diff = get_colored_html_diff_string( document.title, other_document_or_none.title, "right", @@ -481,14 +481,14 @@ def _iterate_one_index( if node.title != other_section_or_none.title: title_modified = True lhs_colored_title_diff = ( - get_colored_diff_string( + get_colored_html_diff_string( node.title, other_section_or_none.title, "left", ) ) rhs_colored_title_diff = ( - get_colored_diff_string( + get_colored_html_diff_string( node.title, other_section_or_none.title, "right", @@ -769,10 +769,10 @@ def create_field_change( other_requirement_field_value = ( other_requirement_field.get_text_value() ) - left_diff = get_colored_diff_string( + left_diff = get_colored_html_diff_string( requirement_field_value, other_requirement_field_value, "left" ) - right_diff = get_colored_diff_string( + right_diff = get_colored_html_diff_string( requirement_field_value, other_requirement_field_value, "right" ) @@ -857,12 +857,12 @@ def create_comment_field_changes( comment_other_value = changed_other_field_.get_text_value() assert comment_other_value is not None - left_diff = get_colored_diff_string( + left_diff = get_colored_html_diff_string( comment_value, comment_other_value, "left", ) - right_diff = get_colored_diff_string( + right_diff = get_colored_html_diff_string( comment_value, comment_other_value, "right", diff --git a/strictdoc/helpers/diff.py b/strictdoc/helpers/diff.py index f0b0822f8..f66bde583 100644 --- a/strictdoc/helpers/diff.py +++ b/strictdoc/helpers/diff.py @@ -2,17 +2,19 @@ import difflib from difflib import SequenceMatcher +from markupsafe import Markup, escape + def similar(a, b): return SequenceMatcher(None, a, b).ratio() -red = lambda text: f'{text}' -green = lambda text: f'{text}' -white = lambda text: f"{text}" +red = lambda text: f'{escape(text)}' +green = lambda text: f'{escape(text)}' +white = lambda text: f"{escape(text)}" -def get_colored_diff_string(old: str, new: str, flag: str): +def get_colored_html_diff_string(old: str, new: str, flag: str) -> Markup: assert old is not None assert new is not None assert flag in ("left", "right") @@ -33,4 +35,4 @@ def get_colored_diff_string(old: str, new: str, flag: str): result += red(old[code[1] : code[2]]) else: result += green(new[code[3] : code[4]]) - return result + return Markup(result)