Skip to content

Commit

Permalink
Mark diff colors safe and escape raw diff input
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
haxtibal committed Nov 10, 2024
1 parent 9531d7e commit 80462b6
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
18 changes: 9 additions & 9 deletions strictdoc/git/project_diff_analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"
)

Expand Down Expand Up @@ -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",
Expand Down
12 changes: 7 additions & 5 deletions strictdoc/helpers/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'<span class="lambda_red">{text}</span>'
green = lambda text: f'<span class="lambda_green">{text}</span>'
white = lambda text: f"<span>{text}</span>"
red = lambda text: f'<span class="lambda_red">{escape(text)}</span>'
green = lambda text: f'<span class="lambda_green">{escape(text)}</span>'
white = lambda text: f"<span>{escape(text)}</span>"


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")
Expand All @@ -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)

0 comments on commit 80462b6

Please sign in to comment.