mirror of
https://github.com/django-components/django-components.git
synced 2025-08-31 19:27:19 +00:00
refactor: move slot escaping inside Slot, remve Slot.escaped, and remove render kwarg escape_slots_content
(#1211)
This commit is contained in:
parent
046569e16d
commit
2e08af9a13
6 changed files with 76 additions and 56 deletions
32
CHANGELOG.md
32
CHANGELOG.md
|
@ -153,7 +153,6 @@ Summary:
|
|||
args: Optional[Tuple[Any, ...]] = None,
|
||||
kwargs: Optional[Mapping] = None,
|
||||
slots: Optional[Mapping] = None,
|
||||
escape_slots_content: bool = True,
|
||||
deps_strategy: DependenciesStrategy = "document",
|
||||
render_dependencies: bool = True,
|
||||
request: Optional[HttpRequest] = None,
|
||||
|
@ -161,6 +160,30 @@ Summary:
|
|||
) -> HttpResponse:
|
||||
```
|
||||
|
||||
- `Component.render()` and `Component.render_to_response()` NO LONGER accept `escape_slots_content` kwarg.
|
||||
|
||||
Instead, slots are now always escaped.
|
||||
|
||||
To disable escaping, wrap the result of `slots` in
|
||||
[`mark_safe()`](https://docs.djangoproject.com/en/5.2/ref/utils/#django.utils.safestring.mark_safe).
|
||||
|
||||
Before:
|
||||
|
||||
```py
|
||||
html = component.render(
|
||||
slots={"my_slot": "CONTENT"},
|
||||
escape_slots_content=False,
|
||||
)
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```py
|
||||
html = component.render(
|
||||
slots={"my_slot": mark_safe("CONTENT")}
|
||||
)
|
||||
```
|
||||
|
||||
- The `Component.Url` class was merged with `Component.View`.
|
||||
|
||||
Instead of `Component.Url.public`, use `Component.View.public`.
|
||||
|
@ -257,6 +280,13 @@ Summary:
|
|||
slot = Slot(lambda ctx: "CONTENT")
|
||||
```
|
||||
|
||||
- The undocumented `Slot.escaped` attribute was removed.
|
||||
|
||||
Instead, slots are now always escaped.
|
||||
|
||||
To disable escaping, wrap the result of `slots` in
|
||||
[`mark_safe()`](https://docs.djangoproject.com/en/5.2/ref/utils/#django.utils.safestring.mark_safe).
|
||||
|
||||
- Slot functions behavior has changed. See the new [Slots](https://django-components.github.io/django-components/latest/concepts/fundamentals/slots/) docs for more info.
|
||||
|
||||
- Function signature:
|
||||
|
|
|
@ -1,4 +1,4 @@
|
|||
django-components has THE most extensive slot system of all the popular Python templating engines.
|
||||
django-components has the most extensive slot system of all the popular Python templating engines.
|
||||
|
||||
The slot system is based on [Vue](https://vuejs.org/guide/components/slots.html), and works across both Django templates and Python code.
|
||||
|
||||
|
|
|
@ -2344,7 +2344,6 @@ class Component(metaclass=ComponentMeta):
|
|||
args: Optional[Any] = None,
|
||||
kwargs: Optional[Any] = None,
|
||||
slots: Optional[Any] = None,
|
||||
escape_slots_content: bool = True,
|
||||
deps_strategy: DependenciesStrategy = "document",
|
||||
# TODO_v1 - Remove, superseded by `deps_strategy`
|
||||
type: Optional[DependenciesStrategy] = None,
|
||||
|
@ -2409,7 +2408,6 @@ class Component(metaclass=ComponentMeta):
|
|||
kwargs=kwargs,
|
||||
context=context,
|
||||
slots=slots,
|
||||
escape_slots_content=escape_slots_content,
|
||||
deps_strategy=deps_strategy,
|
||||
# TODO_v1 - Remove, superseded by `deps_strategy`
|
||||
type=type,
|
||||
|
@ -2426,7 +2424,6 @@ class Component(metaclass=ComponentMeta):
|
|||
args: Optional[Any] = None,
|
||||
kwargs: Optional[Any] = None,
|
||||
slots: Optional[Any] = None,
|
||||
escape_slots_content: bool = True,
|
||||
deps_strategy: DependenciesStrategy = "document",
|
||||
# TODO_v1 - Remove, superseded by `deps_strategy`
|
||||
type: Optional[DependenciesStrategy] = None,
|
||||
|
@ -2574,10 +2571,6 @@ class Component(metaclass=ComponentMeta):
|
|||
|
||||
Read more about [Working with HTTP requests](../../concepts/fundamentals/http_request).
|
||||
|
||||
- `escape_slots_content` - Optional. Whether the content from `slots` should be escaped with Django's
|
||||
[`escape`](https://docs.djangoproject.com/en/5.2/ref/templates/builtins/#std-templatefilter-escape).
|
||||
Defaults to `True`.
|
||||
|
||||
**Type hints:**
|
||||
|
||||
`Component.render()` is NOT typed. To add type hints, you can wrap the inputs
|
||||
|
@ -2639,9 +2632,7 @@ class Component(metaclass=ComponentMeta):
|
|||
if not render_dependencies:
|
||||
deps_strategy = "ignore"
|
||||
|
||||
return comp._render_with_error_trace(
|
||||
context, args, kwargs, slots, escape_slots_content, deps_strategy, request
|
||||
)
|
||||
return comp._render_with_error_trace(context, args, kwargs, slots, deps_strategy, request)
|
||||
|
||||
# This is the internal entrypoint for the render function
|
||||
def _render_with_error_trace(
|
||||
|
@ -2650,13 +2641,12 @@ class Component(metaclass=ComponentMeta):
|
|||
args: Optional[Any] = None,
|
||||
kwargs: Optional[Any] = None,
|
||||
slots: Optional[Any] = None,
|
||||
escape_slots_content: bool = True,
|
||||
deps_strategy: DependenciesStrategy = "document",
|
||||
request: Optional[HttpRequest] = None,
|
||||
) -> str:
|
||||
# Modify the error to display full component path (incl. slots)
|
||||
with component_error_message([self.name]):
|
||||
return self._render_impl(context, args, kwargs, slots, escape_slots_content, deps_strategy, request)
|
||||
return self._render_impl(context, args, kwargs, slots, deps_strategy, request)
|
||||
|
||||
def _render_impl(
|
||||
self,
|
||||
|
@ -2664,7 +2654,6 @@ class Component(metaclass=ComponentMeta):
|
|||
args: Optional[Any] = None,
|
||||
kwargs: Optional[Any] = None,
|
||||
slots: Optional[Any] = None,
|
||||
escape_slots_content: bool = True,
|
||||
deps_strategy: DependenciesStrategy = "document",
|
||||
request: Optional[HttpRequest] = None,
|
||||
) -> str:
|
||||
|
@ -2691,7 +2680,6 @@ class Component(metaclass=ComponentMeta):
|
|||
kwargs_dict = to_dict(kwargs) if kwargs is not None else {}
|
||||
slots_dict = normalize_slot_fills(
|
||||
to_dict(slots) if slots is not None else {},
|
||||
escape_slots_content,
|
||||
component_name=self.name,
|
||||
)
|
||||
# Use RequestContext if request is provided, so that child non-component template tags
|
||||
|
|
|
@ -128,9 +128,6 @@ class DynamicComponent(Component):
|
|||
args=self.input.args,
|
||||
kwargs=cleared_kwargs,
|
||||
slots=self.input.slots,
|
||||
# NOTE: Since we're accessing slots as `self.input.slots`, the content of slot functions
|
||||
# was already escaped (if set so).
|
||||
escape_slots_content=False,
|
||||
deps_strategy=self.input.deps_strategy,
|
||||
)
|
||||
|
||||
|
|
|
@ -229,8 +229,6 @@ class Slot(Generic[TSlotData]):
|
|||
|
||||
Read more about [Rendering slot functions](../../concepts/fundamentals/slots#rendering-slots).
|
||||
"""
|
||||
escaped: bool = False
|
||||
"""Whether the slot content has been escaped."""
|
||||
|
||||
# Following fields are only for debugging
|
||||
component_name: Optional[str] = None
|
||||
|
@ -273,7 +271,8 @@ class Slot(Generic[TSlotData]):
|
|||
context: Optional[Context] = None,
|
||||
) -> SlotResult:
|
||||
slot_ctx: SlotContext = SlotContext(context=context, data=data or {}, fallback=fallback)
|
||||
return self.content_func(slot_ctx)
|
||||
result = self.content_func(slot_ctx)
|
||||
return conditional_escape(result)
|
||||
|
||||
# Make Django pass the instances of this class within the templates without calling
|
||||
# the instances as a function.
|
||||
|
@ -293,6 +292,8 @@ class Slot(Generic[TSlotData]):
|
|||
def _resolve_contents(self, contents: Any) -> Tuple[Any, NodeList, SlotFunc[TSlotData]]:
|
||||
# Case: Content is a string / scalar, so we can use `TextNode` to render it.
|
||||
if not callable(contents):
|
||||
contents = str(contents) if not isinstance(contents, (str, SafeString)) else contents
|
||||
contents = conditional_escape(contents)
|
||||
slot = _nodelist_to_slot(
|
||||
component_name=self.component_name or "<Slot._resolve_contents>",
|
||||
slot_name=self.slot_name,
|
||||
|
@ -762,8 +763,6 @@ class SlotNode(BaseNode):
|
|||
contents=self.contents,
|
||||
data_var=None,
|
||||
fallback_var=None,
|
||||
# Escaped because this was defined in the template
|
||||
escaped=True,
|
||||
)
|
||||
|
||||
# Check: If a slot is marked as 'required', it must be filled.
|
||||
|
@ -1307,8 +1306,6 @@ def resolve_fills(
|
|||
contents=contents,
|
||||
data_var=None,
|
||||
fallback_var=None,
|
||||
# Escaped because this was defined in the template
|
||||
escaped=True,
|
||||
)
|
||||
|
||||
# The content has fills
|
||||
|
@ -1329,8 +1326,6 @@ def resolve_fills(
|
|||
data_var=fill.data_var,
|
||||
fallback_var=fill.fallback_var,
|
||||
extra_context=fill.extra_context,
|
||||
# Escaped because this was defined in the template
|
||||
escaped=True,
|
||||
)
|
||||
slots[fill.name] = slot_fill
|
||||
|
||||
|
@ -1380,28 +1375,21 @@ def _extract_fill_content(
|
|||
|
||||
def normalize_slot_fills(
|
||||
fills: Mapping[SlotName, SlotInput],
|
||||
escape_content: bool = True,
|
||||
component_name: Optional[str] = None,
|
||||
) -> Dict[SlotName, Slot]:
|
||||
# Preprocess slots to escape content if `escape_content=True`
|
||||
norm_fills = {}
|
||||
|
||||
# NOTE: `gen_escaped_content_func` is defined as a separate function, instead of being inlined within
|
||||
# NOTE: `copy_slot` is defined as a separate function, instead of being inlined within
|
||||
# the forloop, because the value the forloop variable points to changes with each loop iteration.
|
||||
def gen_escaped_content_func(content: Union[SlotFunc, Slot], slot_name: str) -> Slot:
|
||||
# Case: Already Slot, already escaped, and names assigned, so nothing to do.
|
||||
if isinstance(content, Slot) and content.escaped and content.slot_name and content.component_name:
|
||||
def copy_slot(content: Union[SlotFunc, Slot], slot_name: str) -> Slot:
|
||||
# Case: Already Slot and names assigned, so nothing to do.
|
||||
if isinstance(content, Slot) and content.slot_name and content.component_name:
|
||||
return content
|
||||
|
||||
# Otherwise, we create a new instance of Slot, whether `content` was already Slot or not.
|
||||
# so we can assign metadata to our internal copies.
|
||||
if not isinstance(content, Slot) or not content.escaped:
|
||||
# We wrap the original function so we post-process it by escaping the result.
|
||||
def content_fn(ctx: SlotContext) -> SlotResult:
|
||||
rendered = content(ctx)
|
||||
return conditional_escape(rendered) if escape_content else rendered
|
||||
|
||||
content_func = cast(SlotFunc, content_fn)
|
||||
# Otherwise, we create a new instance of Slot, whether we've been given Slot or not,
|
||||
# so we can assign metadata to our internal copies without affecting the original.
|
||||
if not isinstance(content, Slot):
|
||||
content_func = content
|
||||
else:
|
||||
content_func = content.content_func
|
||||
|
||||
|
@ -1423,7 +1411,6 @@ def normalize_slot_fills(
|
|||
component_name=used_component_name,
|
||||
slot_name=used_slot_name,
|
||||
nodelist=used_nodelist,
|
||||
escaped=True,
|
||||
)
|
||||
|
||||
return slot
|
||||
|
@ -1432,16 +1419,13 @@ def normalize_slot_fills(
|
|||
# Case: No content, so nothing to do.
|
||||
if content is None:
|
||||
continue
|
||||
# Case: Content is a string / scalar
|
||||
# Case: Content is a string / non-slot / non-callable
|
||||
elif not callable(content):
|
||||
escaped_content = conditional_escape(content) if escape_content else content
|
||||
# NOTE: `Slot.content_func` and `Slot.nodelist` are set in `Slot.__init__()`
|
||||
slot: Slot = Slot(
|
||||
contents=escaped_content, component_name=component_name, slot_name=slot_name, escaped=True
|
||||
)
|
||||
# NOTE: `Slot.content_func` and `Slot.nodelist` will be set in `Slot.__init__()`
|
||||
slot: Slot = Slot(contents=content, component_name=component_name, slot_name=slot_name)
|
||||
# Case: Content is a callable, so either a plain function or a `Slot` instance.
|
||||
else:
|
||||
slot = gen_escaped_content_func(content, slot_name)
|
||||
slot = copy_slot(content, slot_name)
|
||||
|
||||
norm_fills[slot_name] = slot
|
||||
|
||||
|
@ -1455,7 +1439,6 @@ def _nodelist_to_slot(
|
|||
contents: Optional[str] = None,
|
||||
data_var: Optional[str] = None,
|
||||
fallback_var: Optional[str] = None,
|
||||
escaped: bool = False,
|
||||
extra_context: Optional[Dict[str, Any]] = None,
|
||||
) -> Slot:
|
||||
if data_var:
|
||||
|
@ -1543,7 +1526,6 @@ def _nodelist_to_slot(
|
|||
content_func=cast(SlotFunc, render_func),
|
||||
component_name=component_name,
|
||||
slot_name=slot_name,
|
||||
escaped=escaped,
|
||||
nodelist=nodelist,
|
||||
# The `contents` param passed to this function may be `None`, because it's taken from
|
||||
# `BaseNode.contents` which is `None` for self-closing tags like `{% fill "footer" / %}`.
|
||||
|
|
|
@ -7,6 +7,7 @@ import re
|
|||
|
||||
import pytest
|
||||
from django.template import Context, Template, TemplateSyntaxError
|
||||
from django.utils.safestring import mark_safe
|
||||
from django.template.base import NodeList, TextNode
|
||||
from pytest_django.asserts import assertHTMLEqual
|
||||
|
||||
|
@ -174,6 +175,30 @@ class TestSlot:
|
|||
"FROM_INSIDE_SLOT_FN | SLOT_DEFAULT",
|
||||
)
|
||||
|
||||
def test_render_slot_unsafe_content__func(self):
|
||||
def slot_fn1(ctx: SlotContext):
|
||||
return mark_safe("<script>alert('XSS')</script>")
|
||||
|
||||
def slot_fn2(ctx: SlotContext):
|
||||
return "<script>alert('XSS')</script>"
|
||||
|
||||
slot1: Slot = Slot(slot_fn1)
|
||||
slot2: Slot = Slot(slot_fn2)
|
||||
|
||||
rendered1 = slot1()
|
||||
rendered2 = slot2()
|
||||
assert rendered1 == "<script>alert('XSS')</script>"
|
||||
assert rendered2 == "<script>alert('XSS')</script>"
|
||||
|
||||
def test_render_slot_unsafe_content__string(self):
|
||||
slot1: Slot = Slot(mark_safe("<script>alert('XSS')</script>"))
|
||||
slot2: Slot = Slot("<script>alert('XSS')</script>")
|
||||
|
||||
rendered1 = slot1()
|
||||
rendered2 = slot2()
|
||||
assert rendered1 == "<script>alert('XSS')</script>"
|
||||
assert rendered2 == "<script>alert('XSS')</script>"
|
||||
|
||||
# Part of the slot caching feature - test that static content slots reuse the slot function.
|
||||
# See https://github.com/django-components/django-components/issues/1164#issuecomment-2854682354
|
||||
def test_slots_reuse_functions__string(self):
|
||||
|
@ -258,9 +283,7 @@ class TestSlot:
|
|||
assert callable(second_slot_func.contents)
|
||||
assert second_slot_func.nodelist is None
|
||||
|
||||
# NOTE: Both are functions, but different, because internally we wrap the function
|
||||
# to escape the results.
|
||||
assert first_slot_func.contents is not second_slot_func.contents
|
||||
assert first_slot_func.contents is second_slot_func.contents
|
||||
|
||||
# Part of the slot caching feature - test that `Slot` instances with identical function
|
||||
# passed as slots reuse the slot function.
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue