diff --git a/src/openflexure_microscope_server/ui.py b/src/openflexure_microscope_server/ui.py index 1ed23127..b6692785 100644 --- a/src/openflexure_microscope_server/ui.py +++ b/src/openflexure_microscope_server/ui.py @@ -3,6 +3,7 @@ from html import escape from html.parser import HTMLParser from typing import Annotated, Any, Literal, Optional +from urllib.parse import urlparse from pydantic import BaseModel, BeforeValidator, RootModel @@ -23,6 +24,25 @@ ALLOWED_TAGS = { "a", } +ALLOWED_SCHEMES = {"http", "https"} + + +def strip_control_chars(input_string: str) -> str: + """Remove unprintable characters.""" + return "".join(char for char in input_string if char.isprintable()) + + +def sanitize_http_url(url: str) -> Optional[str]: + """Santitise any url only allowing http and https without queries.""" + url = strip_control_chars(url) + parsed = urlparse(url) + if parsed.scheme not in ALLOWED_SCHEMES: + return None + if not parsed.netloc: + return None + fragment_str = "#" + parsed.fragment if parsed.fragment else "" + return f"{parsed.scheme}://{parsed.netloc}{fragment_str}" + class SafeHTMLParser(HTMLParser): """An HTMLParser that only allows a very limited subset of HTML.""" @@ -53,7 +73,8 @@ class SafeHTMLParser(HTMLParser): href = None for attr, value in attrs: if attr == "href" and value is not None: - href = value.strip() + # If it is not a httplink this will stay as None + href = sanitize_http_url(value.strip()) if href: safe_href = escape(href, quote=True) @@ -92,7 +113,7 @@ class SafeHTMLParser(HTMLParser): self.output += f"&#{name};" -def sanitize_html(html: str) -> str: +def sanitise_html(html: str) -> str: """Santitise HTML to only have a small list of allowed tags. Tags allowed without attrs: ``

, , ,

,

,

,

,

,
,`` @@ -113,7 +134,7 @@ def sanitize_html(html: str) -> str: return parser.output -HtmlFragment = Annotated[str, BeforeValidator(sanitize_html)] +HtmlFragment = Annotated[str, BeforeValidator(sanitise_html)] class HTMLBlock(BaseModel): diff --git a/tests/shared_utils/md_test_cases.py b/tests/shared_utils/md_test_cases.py new file mode 100644 index 00000000..9a3e3b37 --- /dev/null +++ b/tests/shared_utils/md_test_cases.py @@ -0,0 +1,109 @@ +"""Read multi-line test cases from markdown files. + +When checking if text coercion is working multiline Python can be very difficult +to read and maintain. This file allows the input and expected results to be written +into markdown codeblocks. + +Each test is a second level heading. +""" + +import re + +# Level 2 markdown heading. Group 1 is the text +CASE_REGEX = re.compile(r"^##\s(.*)", re.MULTILINE) +# Level 3 markdown heading. Group 1 is the text +LEVEL3_REGEX = re.compile(r"^###\s(.*)", re.MULTILINE) +# Markdown code block. +# Group 1 is the language specification after the back ticks +# Group 2 is the code +CODE_REGEX = re.compile( + r"^```([a-zA-Z]+)?\s?\n(?:(.*?)\n)?```", re.MULTILINE | re.DOTALL +) + + +class MdTestCase: + """A class to represent a test case read from a markdown file. + + Useful attributes: + + * title - The title of the test case + * input_code - the input code for the test case + * result_code - the expected result code for the test case + """ + + def __init__(self, title: str, case_text: str, block_type: str) -> None: + """Initialise the test case. + + :param title: The title of the test case. + :param case_text: The full text of the test case. + :param block_type: The type of code block expected (the label after the triple + backticks). + """ + self.title = title + self.block_type = block_type + self.input_code, self.result_code = self._parse(case_text) + + def __repr__(self): + """Return the string representation of the test case.""" + return f"" + + def _parse(self, case_text: str) -> tuple[str, str]: + """Parse the case text and return the input and expected return code.""" + h3_matches = list(LEVEL3_REGEX.finditer(case_text)) + n_h3_matches = len(h3_matches) + if n_h3_matches != 2: + raise ValueError( + f"Case {self.title} has {n_h3_matches} level 3 headings. 2 expected." + ) + if h3_matches[0].group(1) != "Input" or h3_matches[1].group(1) != "Result": + raise ValueError( + f"Case {self.title} does not have `Input` and `Result` headings." + ) + + input_section = case_text[h3_matches[0].end() : h3_matches[1].start()] + result_section = case_text[h3_matches[1].end() : None] + + input_code_blocks = list(CODE_REGEX.finditer(input_section)) + result_code_blocks = list(CODE_REGEX.finditer(result_section)) + if len(input_code_blocks) != 1 or len(result_code_blocks) != 1: + raise ValueError( + f"Case {self.title} must have exactly 1 code block per section." + ) + + input_code_type = input_code_blocks[0].group(1) + input_code = input_code_blocks[0].group(2) + result_code_type = result_code_blocks[0].group(1) + result_code = result_code_blocks[0].group(2) + if input_code_type != self.block_type or result_code_type != self.block_type: + raise ValueError( + f"Case {self.title}: each code block must be labeled as " + f"{self.block_type}" + ) + + # If the code block is empty Regex returns None. Coerce to empty string. + input_code = "" if input_code is None else input_code.strip() + result_code = "" if result_code is None else result_code.strip() + return input_code, result_code + + +def find_test_cases(path: str, block_type: str) -> list[MdTestCase]: + """Return all test cases in the markdown file. + + :param path: Path to the markdown file containing cases. + :param block_type: The label for each triple backtick code block. + + :return: A list of MdTestCase objects for each case. + """ + with open(path, "r", encoding="utf-8") as file_obj: + text = file_obj.read() + test_case_headings = list(CASE_REGEX.finditer(text)) + test_cases = [] + n_headings = len(test_case_headings) + for i, heading_match in enumerate(test_case_headings): + title = heading_match.group(1) + start = heading_match.end() + end = test_case_headings[i + 1].start() if i + 1 < n_headings else None + + case_text = text[start:end] + test_cases.append(MdTestCase(title, case_text, block_type)) + return test_cases diff --git a/tests/unit_tests/data/html_sanitisation_test_cases.md b/tests/unit_tests/data/html_sanitisation_test_cases.md new file mode 100644 index 00000000..2e550d78 --- /dev/null +++ b/tests/unit_tests/data/html_sanitisation_test_cases.md @@ -0,0 +1,84 @@ +# HTML Sanitisation Test Cases + +This file is in markdown not python for readability. + +Each test is a level 2 heading. The input and expected values are level 3 headings. + +The HTML to test is in triple backtick code blocks. + +Anything not in code blocks is a comment. + +## Test scripts removed + +The script is removed but the contents remain. + +### Input +```html + +

Hello

+``` +### Result +```html +alert("x") +

Hello

+``` +## Test script in bold tag + +### Input +```html +click me! +``` +### Result +```html +click me! +``` + +## Test script in disallowed tag +### Input +```html + +``` +### Result +```html +``` + +## Test cookie grabber script is escaped +The script should become escaped and have no tags +### Input +```html + +``` +### Result +```html +var adr = '../evil.php?cakemonster=' + escape(document.cookie); +``` + +## Test with non-http links +Remove the javascript from the links. +### Input +```html +link +encoded +Email +Relative +``` +### Result +```html +link +encoded +Email +Relative +``` + +## Test http links become external +Remove the javascript from the links. +### Input +```html +OpenFlexure +``` +### Result +```html +OpenFlexure +``` \ No newline at end of file diff --git a/tests/unit_tests/test_ui.py b/tests/unit_tests/test_ui.py new file mode 100644 index 00000000..1233458c --- /dev/null +++ b/tests/unit_tests/test_ui.py @@ -0,0 +1,24 @@ +"""Testing the server specified user interface.""" + +import os + +import pytest + +from openflexure_microscope_server import ui + +from ..shared_utils.md_test_cases import find_test_cases + +THIS_DIR = os.path.dirname(os.path.abspath(__file__)) +HTML_SANITISATION_TEST_FILE = os.path.join( + THIS_DIR, "data", "html_sanitisation_test_cases.md" +) +HTML_SANITISATION_TEST_CASES = find_test_cases(HTML_SANITISATION_TEST_FILE, "html") + + +@pytest.mark.parametrize("test_case", HTML_SANITISATION_TEST_CASES) +def test_html_block_sanitises(test_case): + """Check each case in the test case file sanitises as expected.""" + sanitised = ui.sanitise_html(test_case.input_code).strip() + assert sanitised == test_case.result_code, ( + f"Test case: '{test_case.title}': Input code doesn't sanitise as expected." + )