From 7a87a1369020f9a98bb8d4d4589a358cfc767d52 Mon Sep 17 00:00:00 2001 From: Francis Secada Date: Wed, 19 Aug 2026 19:15:13 -0400 Subject: [PATCH 1/2] fix(docs): document JinjaX's _attrs= escape hatch, not attrs= (#78) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JinjaX reserves the prop name `attrs` for its own extra-kwargs collector and unconditionally overwrites whatever a component's `{#def}` declares for it. docs/primitives.md documented `:attrs="{...}"` for JinjaX usage, which compiles and runs but silently discards the dict under a real Catalog — no error, no passthrough. `_attrs=`/`__attrs=` is JinjaX's own escape hatch for this collision and is what actually works; docs now show that instead. Added tests/integration/test_jinja_attrs_passthrough.py against a real Catalog (the unit tier's Jinja2-Environment-only tests never exercise this: without a real Catalog, {#def} is a plain comment and `attrs` never collides with anything). Investigating this surfaced a second, narrower gap worth pinning down rather than leaving implicit: the RESERVED_ATTRS collision guard itself only reliably fires when the colliding key is *not* also one of the component's own declared prop names. For a key that is both (`type`/`href` for Button; `name` for the form controls), JinjaX's arg-filtering routes it straight into that prop before render_attrs ever runs, so PrimitiveConfigError never fires — the caller's real prop, if also passed, silently wins instead. Documented as a JinjaX-level limitation, not a cf-ui bug to patch, and locked in with a test so a future JinjaX version change would be caught rather than assumed away. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Ur2c6d3peuerUGgRdJ6Fvs --- docs/primitives.md | 26 ++++- .../test_jinja_attrs_passthrough.py | 106 ++++++++++++++++++ 2 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 tests/integration/test_jinja_attrs_passthrough.py diff --git a/docs/primitives.md b/docs/primitives.md index b23e3a4..e3f3640 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -221,11 +221,19 @@ is built. === "JinjaX" ```jinja - + Save ``` + JinjaX reserves the prop name `attrs` for its own extra-kwargs collector + and unconditionally overwrites whatever a component's `{#def}` declares + for it — `:attrs="{...}"` compiles, runs, and silently discards the + dict, with no error. `_attrs` (or `__attrs`) is JinjaX's own escape + hatch for this collision; use it instead. This applies only to the + JinjaX path — django-cotton has no equivalent reservation, so its own + `attrs` prop below is unaffected. See #78. + === "django-cotton" ```html @@ -246,9 +254,19 @@ space or `=` forges a brand-new attribute even when its value is fully escaped, because entity escaping never touches either character. A key that collides with a prop the component already renders (`type`, `class`, `href`, …) is rejected outright rather than silently losing to HTML's -keep-the-first-duplicate rule. See [Escaping](escaping.md). Currently -`Button`-only; the same shape is expected to roll out to the rest of Tier 1 -as separate follow-up work (#71, #72). +keep-the-first-duplicate rule — **on the django-cotton and unit-test paths.** +Under a real JinjaX `Catalog`, that guard only reliably fires for a +collision key that is *not* also one of the component's own declared prop +names (`class`, `role`, `aria-disabled`, `disabled` for `Button`). A key +that is both RESERVED_ATTRS-listed and a declared prop (`type`, `href` for +`Button`; `name` for the form controls) never reaches the guard at all — +JinjaX's own arg-filtering routes it straight into that prop before +`render_attrs` runs, so it silently becomes the prop's value instead (the +caller's real prop, if also passed, wins). This is a JinjaX-level gap, not +a cf-ui one; never pass one of a component's own prop names through +`attrs`/`_attrs` — pass it as that prop directly. See +[Escaping](escaping.md). Currently `Button`-only; the same shape is expected +to roll out to the rest of Tier 1 as separate follow-up work (#71, #72). ### Disabled links diff --git a/tests/integration/test_jinja_attrs_passthrough.py b/tests/integration/test_jinja_attrs_passthrough.py new file mode 100644 index 0000000..b916eb2 --- /dev/null +++ b/tests/integration/test_jinja_attrs_passthrough.py @@ -0,0 +1,106 @@ +"""``attrs`` passthrough on the JinjaX path, against a real catalog (#78). + +JinjaX reserves the prop name ``attrs`` for its own extra-kwargs collector +(``jinjax/catalog.py``, ``ARGS_ATTRS = "attrs"``) and unconditionally +overwrites whatever a component's own ``{#def}`` declares for it, on every +render. ``docs/primitives.md`` documented ``:attrs="{...}"`` as the JinjaX +usage — that syntax silently no-ops under a real ``Catalog``, with no error, +because the caller's dict never reaches ``cf_ui.primitives.render_attrs`` at +all. The unit tier's Jinja2-``Environment``-only tests +(``test_attrs_passthrough.py`` et al.) never caught this: without a real +``Catalog``, ``{#def}`` is a plain comment and every prop — ``attrs`` +included — arrives as an ordinary template variable, bypassing JinjaX's +``ARGS_ATTRS`` collision entirely. Only a real ``Catalog`` reproduces it, +which is why these tests live at the integration tier, mirroring +``test_jinja_autoescape.py``. + +``_attrs=``/``__attrs=`` is JinjaX's own escape hatch for this +(``kw.pop("_attrs", kw.pop("__attrs", None))``) and is the syntax +``docs/primitives.md`` documents now. Its keys get merged flat into the +call's kwargs before JinjaX splits declared props from undeclared extras, so +a key that also happens to be one of the component's own declared prop names +(``type``/``href`` for ``Button``; ``name`` for the form controls) never +reaches ``render_attrs``'s ``RESERVED_ATTRS`` guard — it silently becomes +that prop's value instead (the caller's real prop, if also passed, wins). +The guard only reliably fires for reserved names that are *not* also +declared props (``class``, ``role``, ``aria-disabled``, ``disabled`` for +``Button``). Both halves are pinned below so a future JinjaX version change +is caught, not assumed away. +""" + +import pytest +from jinjax import Catalog + +from cf_ui.fastapi import install_cf_ui +from cf_ui.primitives import PrimitiveConfigError + +#: A double quote, a live event handler, and a dangling attribute to swallow +#: the closing quote the template supplies — mirrors test_jinja_autoescape.py. +HOSTILE = '" onmouseover="window.cfPwned=true" x="' + + +@pytest.fixture +def catalog() -> Catalog: + cat = Catalog() + install_cf_ui(cat, theme="bulma") + return cat + + +def test_bare_extra_kwargs_pass_through(catalog: Catalog) -> None: + """The working, undocumented-until-now pattern: unpack a dict of literal + HTML attribute names (hyphens included) as ``**kwargs``.""" + html = catalog.render( + "Cf:Button", _content="Save", **{"data-event": "submit", "hx-post": "/orders"} + ) + assert 'data-event="submit"' in html + assert 'hx-post="/orders"' in html + + +def test_attrs_dict_via_the_underscore_attrs_kwarg_passes_through(catalog: Catalog) -> None: + """The documented fix for #78: ``_attrs={...}``, not ``attrs={...}``.""" + html = catalog.render( + "Cf:Button", + _content="Save", + _attrs={"data-event": "submit", "hx-post": "/orders"}, + ) + assert 'data-event="submit"' in html + assert 'hx-post="/orders"' in html + + +def test_plain_attrs_kwarg_is_silently_discarded_by_jinjax(catalog: Catalog) -> None: + """Pins the bug #78 exists to warn about, not a desired behavior. + + ``docs/primitives.md`` must never show ``attrs=``/``:attrs="..."`` for + JinjaX usage — the two tests above show the syntax that actually works. + """ + html = catalog.render("Cf:Button", _content="Save", attrs={"data-event": "submit"}) + assert "data-event" not in html + + +def test_attrs_collision_with_a_reserved_undeclared_name_still_raises(catalog: Catalog) -> None: + """``class`` is RESERVED_ATTRS-listed for button but not a declared prop + (``extra_class`` is), so it never intercepts before ``render_attrs`` runs.""" + with pytest.raises(PrimitiveConfigError): + catalog.render("Cf:Button", _content="Save", _attrs={"class": "override"}) + + +def test_attrs_collision_with_a_declared_prop_name_is_not_caught_by_the_guard( + catalog: Catalog, +) -> None: + """A JinjaX-level gap, not a cf-ui one — documented in this file's module + docstring and in ``docs/primitives.md``. + + ``type`` is both RESERVED_ATTRS-listed and one of Button's own ``{#def}`` + props. JinjaX's own arg-filtering routes an ``_attrs={"type": ...}`` + collision straight into the ``type`` prop before ``render_attrs`` ever + sees it, so ``PrimitiveConfigError`` never fires here — the caller's + real ``type=`` prop silently wins instead. + """ + html = catalog.render("Cf:Button", _content="Save", type="submit", _attrs={"type": "reset"}) + assert 'type="submit"' in html + assert 'type="reset"' not in html + + +def test_hostile_attrs_value_is_still_escaped(catalog: Catalog) -> None: + html = catalog.render("Cf:Button", _content="Save", _attrs={"data-x": HOSTILE}) + assert 'onmouseover="window.cfPwned=true"' not in html From baa48eb7030d2ffe024f99c6a1b3170899936801 Mon Sep 17 00:00:00 2001 From: Francis Secada Date: Wed, 19 Aug 2026 19:21:01 -0400 Subject: [PATCH 2/2] fix(docs): correct stale Button-only attrs claim, pin the form-control case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /review on PR #81 found two issues: - docs/primitives.md still cited closed issues #71/#72 as pending follow-up for rolling attrs passthrough out past Button, contradicting this same PR's own new text ("name for the form controls") two sentences earlier — the rollout already shipped via #76/#77. Corrected the claim to name what actually ships today. - The declared-prop-name collision-bypass gap was only pinned by a Button test, but both the docs and this test file's docstring claim it for "the form controls" generally. Added a FormField test (name= collision) so a future JinjaX change affecting a different prop signature would be caught, not silently assumed away. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Ur2c6d3peuerUGgRdJ6Fvs --- docs/primitives.md | 5 +++-- tests/integration/test_jinja_attrs_passthrough.py | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/docs/primitives.md b/docs/primitives.md index e3f3640..96b87a8 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -265,8 +265,9 @@ JinjaX's own arg-filtering routes it straight into that prop before caller's real prop, if also passed, wins). This is a JinjaX-level gap, not a cf-ui one; never pass one of a component's own prop names through `attrs`/`_attrs` — pass it as that prop directly. See -[Escaping](escaping.md). Currently `Button`-only; the same shape is expected -to roll out to the rest of Tier 1 as separate follow-up work (#71, #72). +[Escaping](escaping.md). Ships today on `Button`, `Select`, `Textarea` and +`FormField` (#76, #77) — the same shape is expected to roll out to the rest +of Tier 1 as separate follow-up work. ### Disabled links diff --git a/tests/integration/test_jinja_attrs_passthrough.py b/tests/integration/test_jinja_attrs_passthrough.py index b916eb2..61bf7df 100644 --- a/tests/integration/test_jinja_attrs_passthrough.py +++ b/tests/integration/test_jinja_attrs_passthrough.py @@ -101,6 +101,21 @@ def test_attrs_collision_with_a_declared_prop_name_is_not_caught_by_the_guard( assert 'type="reset"' not in html +def test_the_same_declared_prop_name_gap_holds_for_a_form_control(catalog: Catalog) -> None: + """Pins the same gap for a second component with a different prop + signature, not just Button — ``docs/primitives.md`` claims it for "the + form controls" generally, not for Button alone. + + ``name`` is both RESERVED_ATTRS-listed and a declared ``FormField`` prop + (unlike Button's ``type``/``href``, it also has no default — a required + positional-style prop). The same bypass applies: the real ``name=`` prop + wins silently, no ``PrimitiveConfigError``. + """ + html = catalog.render("Cf:FormField", name="email", label="Email", _attrs={"name": "hijacked"}) + assert 'name="email"' in html + assert 'name="hijacked"' not in html + + def test_hostile_attrs_value_is_still_escaped(catalog: Catalog) -> None: html = catalog.render("Cf:Button", _content="Save", _attrs={"data-x": HOSTILE}) assert 'onmouseover="window.cfPwned=true"' not in html