Skip to content

Commit 80462b6

Browse files
committed
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.
1 parent 9531d7e commit 80462b6

File tree

4 files changed

+37
-31
lines changed

4 files changed

+37
-31
lines changed

strictdoc/export/html/generators/view_objects/diff_screen_results_view_object.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
from datetime import datetime
44
from typing import Optional
55

6+
from markupsafe import Markup
7+
68
from strictdoc import __version__
79
from strictdoc.core.project_config import ProjectConfig
810
from strictdoc.export.html.html_templates import JinjaEnvironment
@@ -58,31 +60,31 @@ def __init__(
5860
self.strictdoc_version = __version__
5961
self.error_message: Optional[str] = None
6062

61-
def render_screen(self, jinja_environment: JinjaEnvironment):
63+
def render_screen(self, jinja_environment: JinjaEnvironment) -> Markup:
6264
template = jinja_environment.environment.overlay(
6365
autoescape=False
6466
).get_template("screens/git/index.jinja")
65-
return template.render(view_object=self)
67+
return Markup(template.render(view_object=self))
6668

67-
def render_url(self, url: str):
68-
return self.link_renderer.render_url(url)
69+
def render_url(self, url: str) -> Markup:
70+
return Markup(self.link_renderer.render_url(url))
6971

70-
def render_node_link(self, incoming_link, document, document_type):
72+
def render_node_link(self, incoming_link, document, document_type) -> str:
7173
return self.link_renderer.render_node_link(
7274
incoming_link, document, document_type
7375
)
7476

75-
def render_static_url(self, url: str):
76-
return self.link_renderer.render_static_url(url)
77+
def render_static_url(self, url: str) -> Markup:
78+
return Markup(self.link_renderer.render_static_url(url))
7779

7880
def render_static_url_with_prefix(self, url: str) -> str:
7981
return self.link_renderer.render_static_url_with_prefix(url)
8082

81-
def render_local_anchor(self, node):
83+
def render_local_anchor(self, node) -> str:
8284
return self.link_renderer.render_local_anchor(node)
8385

8486
def is_empty_tree(self) -> bool:
8587
return self.document_tree_iterator.is_empty_tree()
8688

87-
def date_today(self):
89+
def date_today(self) -> str:
8890
return datetime.today().strftime("%Y-%m-%d")

strictdoc/export/html/generators/view_objects/diff_screen_view_object.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
from dataclasses import dataclass
33
from datetime import datetime
44

5+
from markupsafe import Markup
6+
57
from strictdoc import __version__
68
from strictdoc.core.project_config import ProjectConfig
79
from strictdoc.export.html.html_templates import JinjaEnvironment
@@ -39,27 +41,27 @@ def __init__(
3941
self.is_running_on_server: bool = project_config.is_running_on_server
4042
self.strictdoc_version = __version__
4143

42-
def render_screen(self, jinja_environment: JinjaEnvironment):
44+
def render_screen(self, jinja_environment: JinjaEnvironment) -> Markup:
4345
return jinja_environment.render_template_as_markup(
4446
"screens/git/index.jinja", view_object=self
4547
)
4648

47-
def render_url(self, url: str):
48-
return self.link_renderer.render_url(url)
49+
def render_url(self, url: str) -> Markup:
50+
return Markup(self.link_renderer.render_url(url))
4951

50-
def render_node_link(self, incoming_link, document, document_type):
52+
def render_node_link(self, incoming_link, document, document_type) -> str:
5153
return self.link_renderer.render_node_link(
5254
incoming_link, document, document_type
5355
)
5456

55-
def render_static_url(self, url: str):
56-
return self.link_renderer.render_static_url(url)
57+
def render_static_url(self, url: str) -> Markup:
58+
return Markup(self.link_renderer.render_static_url(url))
5759

5860
def render_static_url_with_prefix(self, url: str) -> str:
5961
return self.link_renderer.render_static_url_with_prefix(url)
6062

61-
def render_local_anchor(self, node):
63+
def render_local_anchor(self, node) -> str:
6264
return self.link_renderer.render_local_anchor(node)
6365

64-
def date_today(self):
66+
def date_today(self) -> str:
6567
return datetime.today().strftime("%Y-%m-%d")

strictdoc/git/project_diff_analyzer.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
SectionChange,
2525
)
2626
from strictdoc.helpers.cast import assert_cast, assert_optional_cast
27-
from strictdoc.helpers.diff import get_colored_diff_string, similar
27+
from strictdoc.helpers.diff import get_colored_html_diff_string, similar
2828
from strictdoc.helpers.mid import MID
2929

3030

@@ -366,12 +366,12 @@ def _iterate_one_index(
366366
if other_document_or_none is not None:
367367
if document.title != other_document_or_none.title:
368368
title_modified = True
369-
lhs_colored_title_diff = get_colored_diff_string(
369+
lhs_colored_title_diff = get_colored_html_diff_string(
370370
document.title,
371371
other_document_or_none.title,
372372
"left",
373373
)
374-
rhs_colored_title_diff = get_colored_diff_string(
374+
rhs_colored_title_diff = get_colored_html_diff_string(
375375
document.title,
376376
other_document_or_none.title,
377377
"right",
@@ -481,14 +481,14 @@ def _iterate_one_index(
481481
if node.title != other_section_or_none.title:
482482
title_modified = True
483483
lhs_colored_title_diff = (
484-
get_colored_diff_string(
484+
get_colored_html_diff_string(
485485
node.title,
486486
other_section_or_none.title,
487487
"left",
488488
)
489489
)
490490
rhs_colored_title_diff = (
491-
get_colored_diff_string(
491+
get_colored_html_diff_string(
492492
node.title,
493493
other_section_or_none.title,
494494
"right",
@@ -769,10 +769,10 @@ def create_field_change(
769769
other_requirement_field_value = (
770770
other_requirement_field.get_text_value()
771771
)
772-
left_diff = get_colored_diff_string(
772+
left_diff = get_colored_html_diff_string(
773773
requirement_field_value, other_requirement_field_value, "left"
774774
)
775-
right_diff = get_colored_diff_string(
775+
right_diff = get_colored_html_diff_string(
776776
requirement_field_value, other_requirement_field_value, "right"
777777
)
778778

@@ -857,12 +857,12 @@ def create_comment_field_changes(
857857
comment_other_value = changed_other_field_.get_text_value()
858858
assert comment_other_value is not None
859859

860-
left_diff = get_colored_diff_string(
860+
left_diff = get_colored_html_diff_string(
861861
comment_value,
862862
comment_other_value,
863863
"left",
864864
)
865-
right_diff = get_colored_diff_string(
865+
right_diff = get_colored_html_diff_string(
866866
comment_value,
867867
comment_other_value,
868868
"right",

strictdoc/helpers/diff.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,19 @@
22
import difflib
33
from difflib import SequenceMatcher
44

5+
from markupsafe import Markup, escape
6+
57

68
def similar(a, b):
79
return SequenceMatcher(None, a, b).ratio()
810

911

10-
red = lambda text: f'<span class="lambda_red">{text}</span>'
11-
green = lambda text: f'<span class="lambda_green">{text}</span>'
12-
white = lambda text: f"<span>{text}</span>"
12+
red = lambda text: f'<span class="lambda_red">{escape(text)}</span>'
13+
green = lambda text: f'<span class="lambda_green">{escape(text)}</span>'
14+
white = lambda text: f"<span>{escape(text)}</span>"
1315

1416

15-
def get_colored_diff_string(old: str, new: str, flag: str):
17+
def get_colored_html_diff_string(old: str, new: str, flag: str) -> Markup:
1618
assert old is not None
1719
assert new is not None
1820
assert flag in ("left", "right")
@@ -33,4 +35,4 @@ def get_colored_diff_string(old: str, new: str, flag: str):
3335
result += red(old[code[1] : code[2]])
3436
else:
3537
result += green(new[code[3] : code[4]])
36-
return result
38+
return Markup(result)

0 commit comments

Comments
 (0)