From 23c894f53671ef98781b62ee6b13186a8314ab61 Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Wed, 30 Oct 2024 19:16:48 -0700 Subject: [PATCH 1/8] Always render script element as plain HTML --- Co-authored-by: James Hutchison <122519877+JamesHutchison@users.noreply.github.com> --- .../@reactpy/client/src/components.tsx | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/js/packages/@reactpy/client/src/components.tsx b/src/js/packages/@reactpy/client/src/components.tsx index 2319f81c7..42d68b787 100644 --- a/src/js/packages/@reactpy/client/src/components.tsx +++ b/src/js/packages/@reactpy/client/src/components.tsx @@ -125,23 +125,15 @@ function ScriptElement({ model }: { model: ReactPyVdom }) { (value): value is string => typeof value == "string", )[0]; - let scriptElement: HTMLScriptElement; - if (model.attributes) { - scriptElement = document.createElement("script"); - for (const [k, v] of Object.entries(model.attributes)) { - scriptElement.setAttribute(k, v); - } - if (scriptContent) { - scriptElement.appendChild(document.createTextNode(scriptContent)); - } - ref.current.appendChild(scriptElement); - } else if (scriptContent) { - const scriptResult = eval(scriptContent); - if (typeof scriptResult == "function") { - return scriptResult(); - } + const scriptElement: HTMLScriptElement = document.createElement("script"); + for (const [k, v] of Object.entries(model.attributes || {})) { + scriptElement.setAttribute(k, v); + } + if (scriptContent) { + scriptElement.appendChild(document.createTextNode(scriptContent)); } - }, [model.key, ref.current]); + ref.current.appendChild(scriptElement); + }, [model.key]); return
; } From f33cc4fa208c9ef35401217616e956e123e658c0 Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Tue, 21 Jan 2025 02:09:48 -0800 Subject: [PATCH 2/8] Re-add changes from deleted commits --- pyproject.toml | 2 +- src/reactpy/html.py | 57 +++++----------------------------------- tests/test_html.py | 63 +++------------------------------------------ 3 files changed, 10 insertions(+), 112 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 371bed107..868e884ac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ artifacts = ["/src/reactpy/static/"] artifacts = ["/src/reactpy/static/"] [tool.hatch.metadata] -license-files = { paths = ["LICENSE.md"] } +license-files = { paths = ["LICENSE"] } [tool.hatch.envs.default] installer = "uv" diff --git a/src/reactpy/html.py b/src/reactpy/html.py index 941af949f..91d73d240 100644 --- a/src/reactpy/html.py +++ b/src/reactpy/html.py @@ -422,54 +422,10 @@ def _script( key is given, the key is inferred to be the content of the script or, lastly its 'src' attribute if that is given. - If no attributes are given, the content of the script may evaluate to a function. - This function will be called when the script is initially created or when the - content of the script changes. The function may itself optionally return a teardown - function that is called when the script element is removed from the tree, or when - the script content changes. - Notes: Do not use unsanitized data from untrusted sources anywhere in your script. - Doing so may allow for malicious code injection. Consider this **insecure** - code: - - .. code-block:: - - my_script = html.script(f"console.log('{user_bio}');") - - A clever attacker could construct ``user_bio`` such that they could escape the - string and execute arbitrary code to perform cross-site scripting - (`XSS `__`). For example, - what if ``user_bio`` were of the form: - - .. code-block:: text - - '); attackerCodeHere(); (' - - This would allow the following Javascript code to be executed client-side: - - .. code-block:: js - - console.log(''); attackerCodeHere(); (''); - - One way to avoid this could be to escape ``user_bio`` so as to prevent the - injection of Javascript code. For example: - - .. code-block:: python - - import json - - my_script = html.script(f"console.log({json.dumps(user_bio)});") - - This would prevent the injection of Javascript code by escaping the ``user_bio`` - string. In this case, the following client-side code would be executed instead: - - .. code-block:: js - - console.log("'); attackerCodeHere(); ('"); - - This is a very simple example, but it illustrates the point that you should - always be careful when using unsanitized data from untrusted sources. + Doing so may allow for malicious code injection + (`XSS `__`). """ model: VdomDict = {"tagName": "script"} @@ -481,13 +437,12 @@ def _script( if len(children) > 1: msg = "'script' nodes may have, at most, one child." raise ValueError(msg) - elif not isinstance(children[0], str): + if not isinstance(children[0], str): msg = "The child of a 'script' must be a string." raise ValueError(msg) - else: - model["children"] = children - if key is None: - key = children[0] + model["children"] = children + if key is None: + key = children[0] if attributes: model["attributes"] = attributes diff --git a/tests/test_html.py b/tests/test_html.py index 334fcab03..41942dfc1 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -3,47 +3,7 @@ from reactpy import component, config, html from reactpy.testing import DisplayFixture, poll from reactpy.utils import Ref -from tests.tooling.hooks import use_counter, use_toggle - - -async def test_script_mount_unmount(display: DisplayFixture): - toggle_is_mounted = Ref() - - @component - def Root(): - is_mounted, toggle_is_mounted.current = use_toggle(True) - return html.div( - html.div({"id": "mount-state", "data_value": False}), - HasScript() if is_mounted else html.div(), - ) - - @component - def HasScript(): - return html.script( - """() => { - const mapping = {"false": false, "true": true}; - const mountStateEl = document.getElementById("mount-state"); - mountStateEl.setAttribute( - "data-value", !mapping[mountStateEl.getAttribute("data-value")]); - return () => mountStateEl.setAttribute( - "data-value", !mapping[mountStateEl.getAttribute("data-value")]); - }""" - ) - - await display.show(Root) - - mount_state = await display.page.wait_for_selector("#mount-state", state="attached") - poll_mount_state = poll(mount_state.get_attribute, "data-value") - - await poll_mount_state.until_equals("true") - - toggle_is_mounted.current() - - await poll_mount_state.until_equals("false") - - toggle_is_mounted.current() - - await poll_mount_state.until_equals("true") +from tests.tooling.hooks import use_counter async def test_script_re_run_on_content_change(display: DisplayFixture): @@ -54,14 +14,9 @@ def HasScript(): count, incr_count.current = use_counter(1) return html.div( html.div({"id": "mount-count", "data_value": 0}), - html.div({"id": "unmount-count", "data_value": 0}), html.script( - f"""() => {{ - const mountCountEl = document.getElementById("mount-count"); - const unmountCountEl = document.getElementById("unmount-count"); - mountCountEl.setAttribute("data-value", {count}); - return () => unmountCountEl.setAttribute("data-value", {count});; - }}""" + 'const mountCountEl = document.getElementById("mount-count");' + f'mountCountEl.setAttribute("data-value", {count});' ), ) @@ -70,23 +25,11 @@ def HasScript(): mount_count = await display.page.wait_for_selector("#mount-count", state="attached") poll_mount_count = poll(mount_count.get_attribute, "data-value") - unmount_count = await display.page.wait_for_selector( - "#unmount-count", state="attached" - ) - poll_unmount_count = poll(unmount_count.get_attribute, "data-value") - await poll_mount_count.until_equals("1") - await poll_unmount_count.until_equals("0") - incr_count.current() - await poll_mount_count.until_equals("2") - await poll_unmount_count.until_equals("1") - incr_count.current() - await poll_mount_count.until_equals("3") - await poll_unmount_count.until_equals("2") async def test_script_from_src(display: DisplayFixture): From 9d4f978f1843abccfaf0871e5a7dbf326b4fe7d4 Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Tue, 21 Jan 2025 02:46:47 -0800 Subject: [PATCH 3/8] Fix tests --- tests/conftest.py | 5 +++++ tests/test_html.py | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index eaeb37f64..17231a2ac 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -36,6 +36,11 @@ def install_playwright(): subprocess.run(["playwright", "install", "chromium"], check=True) # noqa: S607, S603 +@pytest.fixture(autouse=True, scope="session") +def rebuild_javascript(): + subprocess.run(["hatch", "run", "javascript:build"], check=True) # noqa: S607, S603 + + @pytest.fixture async def display(server, page): async with DisplayFixture(server, page) as display: diff --git a/tests/test_html.py b/tests/test_html.py index 41942dfc1..30b02ce99 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -15,8 +15,7 @@ def HasScript(): return html.div( html.div({"id": "mount-count", "data_value": 0}), html.script( - 'const mountCountEl = document.getElementById("mount-count");' - f'mountCountEl.setAttribute("data-value", {count});' + f'document.getElementById("mount-count").setAttribute("data-value", {count});' ), ) From f025cb09630476fca04b673d37670f324241f27d Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Tue, 21 Jan 2025 02:52:02 -0800 Subject: [PATCH 4/8] Allow test_module_from_url to rerun --- tests/test_web/test_module.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_web/test_module.py b/tests/test_web/test_module.py index 75c819a32..388794741 100644 --- a/tests/test_web/test_module.py +++ b/tests/test_web/test_module.py @@ -50,6 +50,7 @@ def ShowCurrentComponent(): await display.page.wait_for_selector("#unmount-flag", state="attached") +@pytest.mark.flaky(reruns=3) async def test_module_from_url(browser): app = Sanic("test_module_from_url") From 07bf70da943f4afa52eda775868193c67170106d Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Tue, 21 Jan 2025 03:14:11 -0800 Subject: [PATCH 5/8] Clean up ScriptElement source code --- .../@reactpy/client/src/components.tsx | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/js/packages/@reactpy/client/src/components.tsx b/src/js/packages/@reactpy/client/src/components.tsx index 609a4fba3..4d29a9ff2 100644 --- a/src/js/packages/@reactpy/client/src/components.tsx +++ b/src/js/packages/@reactpy/client/src/components.tsx @@ -120,21 +120,30 @@ function ScriptElement({ model }: { model: ReactPyVdom }) { const ref = useRef(null); React.useEffect(() => { - if (!ref.current) { - return; - } + // Fetch the script's content const scriptContent = model?.children?.filter( (value): value is string => typeof value == "string", )[0]; + // Don't run if the parent element or script content is missing + if (!ref.current || !scriptContent) { + return; + } + + // Create the script element const scriptElement: HTMLScriptElement = document.createElement("script"); for (const [k, v] of Object.entries(model.attributes || {})) { scriptElement.setAttribute(k, v); } - if (scriptContent) { - scriptElement.appendChild(document.createTextNode(scriptContent)); - } + + // Append the script content to the script element + scriptElement.appendChild(document.createTextNode(scriptContent)); ref.current.appendChild(scriptElement); + + // Remove the script element when the component is unmounted + return () => { + ref.current?.removeChild(scriptElement); + }; }, [model.key]); return
; From 832a3986e8f177df99e591da5836a5874bcbca7b Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Tue, 21 Jan 2025 03:14:26 -0800 Subject: [PATCH 6/8] Add changelog --- docs/source/about/changelog.rst | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/docs/source/about/changelog.rst b/docs/source/about/changelog.rst index 45aefe401..ccbd3d728 100644 --- a/docs/source/about/changelog.rst +++ b/docs/source/about/changelog.rst @@ -8,18 +8,21 @@ Changelog scheme for the project adheres to `Semantic Versioning `__. -.. INSTRUCTIONS FOR CHANGELOG CONTRIBUTORS -.. !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -.. If you're adding a changelog entry, be sure to read the "Creating a Changelog Entry" -.. section of the documentation before doing so for instructions on how to adhere to the -.. "Keep a Changelog" style guide (https://keepachangelog.com). +.. Using the following categories, list your changes in this order: +.. [Added, Changed, Deprecated, Removed, Fixed, Security] +.. Don't forget to remove deprecated code on each major release! Unreleased ---------- **Changed** -- :pull:`1251` Substitute client-side usage of ``react`` with ``preact``. +- :pull:`1251` - Substitute client-side usage of ``react`` with ``preact``. +- :pull:`1239` - Script elements no longer support behaving like effects. They now strictly behave like plain HTML script elements. + +**Fixed** + +- :pull:`1239` - Fixed a bug where script elements would not render to the DOM as plain text. v1.1.0 ------ From 43c7a4451b6d0c5cbae49b946c0b53d07415ef03 Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Tue, 21 Jan 2025 03:30:41 -0800 Subject: [PATCH 7/8] Fix tests --- .../@reactpy/client/src/components.tsx | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/js/packages/@reactpy/client/src/components.tsx b/src/js/packages/@reactpy/client/src/components.tsx index 4d29a9ff2..efaa7a759 100644 --- a/src/js/packages/@reactpy/client/src/components.tsx +++ b/src/js/packages/@reactpy/client/src/components.tsx @@ -120,13 +120,8 @@ function ScriptElement({ model }: { model: ReactPyVdom }) { const ref = useRef(null); React.useEffect(() => { - // Fetch the script's content - const scriptContent = model?.children?.filter( - (value): value is string => typeof value == "string", - )[0]; - - // Don't run if the parent element or script content is missing - if (!ref.current || !scriptContent) { + // Don't run if the parent element is missing + if (!ref.current) { return; } @@ -136,8 +131,15 @@ function ScriptElement({ model }: { model: ReactPyVdom }) { scriptElement.setAttribute(k, v); } - // Append the script content to the script element - scriptElement.appendChild(document.createTextNode(scriptContent)); + // Add the script content as text + const scriptContent = model?.children?.filter( + (value): value is string => typeof value == "string", + )[0]; + if (scriptContent) { + scriptElement.appendChild(document.createTextNode(scriptContent)); + } + + // Append the script element to the parent element ref.current.appendChild(scriptElement); // Remove the script element when the component is unmounted From 39e8568c48e8ef3f9d205a20e5b6370a729d6c98 Mon Sep 17 00:00:00 2001 From: Archmonger <16909269+Archmonger@users.noreply.github.com> Date: Tue, 21 Jan 2025 03:45:32 -0800 Subject: [PATCH 8/8] Fix flakey test_use_linked_inputs --- tests/tooling/common.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/tooling/common.py b/tests/tooling/common.py index c0191bd4e..1803b8aed 100644 --- a/tests/tooling/common.py +++ b/tests/tooling/common.py @@ -1,9 +1,12 @@ +import os from typing import Any from reactpy.core.types import LayoutEventMessage, LayoutUpdateMessage -# see: https://github.com/microsoft/playwright-python/issues/1614 -DEFAULT_TYPE_DELAY = 100 # milliseconds +GITHUB_ACTIONS = os.getenv("GITHUB_ACTIONS", "False") +DEFAULT_TYPE_DELAY = ( + 250 if GITHUB_ACTIONS.lower() in {"y", "yes", "t", "true", "on", "1"} else 25 +) def event_message(target: str, *data: Any) -> LayoutEventMessage: