Skip to content

Commit

Permalink
Fix(web-twig): Custom escaping of HTML attributes
Browse files Browse the repository at this point in the history
  * while removing double quotes in #742 we let Twig to escape our
    attributes automagically
  * this lead us to bug where double quotes where missing in HTML output
    for `placeholder` attribute and thus from multiple words of
placeholder only first was displayed
  * we are moving back and returning double quotes to concatenation of
    attributes while also adding the escape functionality
  * because of custom fabrication of the HTML tags and using double
    quotes and Twig's automated escaping we must escape values by
ourselves
  * @see: https://twig.symfony.com/doc/3.x/filters/escape.html ->
    Caution
  * `{{ var|escape(strategy)|raw }} {# won't be double-escaped #}`
  * @see: #742

refs #DS-760
  • Loading branch information
literat committed May 18, 2023
1 parent 2612484 commit 42176b2
Show file tree
Hide file tree
Showing 30 changed files with 119 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
{%- set _rootClassName = _spiritClassPrefix ~ 'Accordion' -%}

{# Attributes #}
{%- set _idAttr = _id ? 'id=' ~ _id : null -%}
{%- set _idAttr = _id ? 'id="' ~ _id | escape('html_attr') ~ '"' : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
Expand All @@ -17,7 +17,7 @@
<{{ _elementType }}
{{ mainProps(_mainPropsWithoutId) }}
{{ styleProp(_styleProps) }}
{{ _idAttr }}
{{ _idAttr | raw }}
{{ classProp(_classNames) }}
data-toggle="accordion"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
{%- set _isOpenClassName = _isOpen ? 'is-open' : null -%}

{# Attributes #}
{%- set _idAttr = _id ? 'id=' ~ _id : null -%}
{%- set _labelledIdAttr = _labelledById ? 'aria-labelledby=' ~ _labelledById : null -%}
{%- set _dataParentAttr = _parent ? 'data-parent=' ~ _parent : null -%}
{%- set _idAttr = _id ? 'id="' ~ _id | escape('html_attr') ~ '"' : null -%}
{%- set _labelledIdAttr = _labelledById ? 'aria-labelledby="' ~ _labelledById | escape('html_attr') ~ '"' : null -%}
{%- set _dataParentAttr = _parent ? 'data-parent="' ~ _parent | escape('html_attr') ~ '"' : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
Expand All @@ -25,10 +25,10 @@
<div
{{ mainProps(_mainPropsWithoutId) }}
{{ styleProp(_styleProps) }}
{{ _idAttr }}
{{ _idAttr | raw }}
{{ classProp(_collapseClassNames) }}
{{ _labelledIdAttr }}
{{ _dataParentAttr }}
{{ _labelledIdAttr | raw }}
{{ _dataParentAttr | raw }}
>
<div class="{{ _collapseClassName }}">
<div {{ classProp(_contentClassNames) }}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
{%- set _iconClassName = _spiritClassPrefix ~ 'Accordion__itemIcon' -%}

{# Attributes #}
{%- set _idAttr = _id ? 'id=' ~ _id : null -%}
{%- set _dataTargetAttr = _for ? 'data-target=' ~ _for : null -%}
{%- set _idAttr = _id ? 'id="' ~ _id | escape('html_attr') ~ '"' : null -%}
{%- set _dataTargetAttr = _for ? 'data-target="' ~ _for | escape('html_attr') ~ '"' : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
Expand All @@ -26,14 +26,14 @@
<{{ _elementType }}
{{ mainProps(_mainPropsWithoutId) }}
{{ styleProp(_styleProps) }}
{{ _idAttr }}
{{ _idAttr | raw }}
{{ classProp(_classNames) }}
>
<button
type="button"
class="{{ _toggleClassName }}"
data-toggle="collapse"
{{ _dataTargetAttr }}
{{ _dataTargetAttr | raw }}
aria-expanded="{{ _ariaExpanded }}"
aria-controls="{{ _for }}"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
{%- set _rootClassName = _spiritClassPrefix ~ 'Accordion__item' -%}

{# Attributes #}
{%- set _idAttr = _id ? 'id=' ~ _id : null -%}
{%- set _idAttr = _id ? 'id="' ~ _id | escape('html_attr') ~ '"' : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
Expand All @@ -17,7 +17,7 @@
<{{ _elementType }}
{{ mainProps(_mainPropsWithoutId) }}
{{ styleProp(_styleProps) }}
{{ _idAttr }}
{{ _idAttr | raw }}
{{ classProp(_classNames) }}
>
{% block content %}{% endblock %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
{%- set _isLoading = props.isLoading | default(false) | boolprop -%}
{%- set _isSquare = props.isSquare | default(false) | boolprop -%}
{%- set _onClick = props.onClick | default(null) -%}
{%- set _target = props.target | default(null) -%}

{# Class names #}
{%- set _rootClassName = _spiritClassPrefix ~ 'Button' -%}
Expand All @@ -21,13 +20,13 @@

{# Attributes #}
{%- set _onClickAttr = _onClick ? 'onclick=' ~ _onClick | escape('html_attr') : null -%}
{%- set _targetAttr = _target ? 'target=' ~ _target : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
{%- set _classNames = [ _rootClassName, _rootColorClassName, _rootBlockClassName, _rootDisabledClassName, _rootLoadingClassName, _rootSizeClassName, _rootSquareClassName, _styleProps.className ] -%}
{%- set _allowedAttributes = [
'title',
'target',
] -%}

{# Deprecations #}
Expand All @@ -41,7 +40,6 @@
{{ classProp(_classNames) }}
href="{{ _href }}"
{{ _onClickAttr | raw }}
{{ _targetAttr }}
>
{%- block content %}{% endblock %}
{%- if _isLoading -%}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
{%- set _contentClassName = _spiritClassPrefix ~ 'Collapse__content' -%}

{# Attributes #}
{%- set _idAttr = _id ? 'id=' ~ _id : null -%}
{%- set _dataBreakpointAttr = _breakpoint ? 'data-breakpoint=' ~ _breakpoint : null -%}
{%- set _dataParentAttr = _parent ? 'data-parent=' ~ _parent : null -%}
{%- set _idAttr = _id ? 'id="' ~ _id | escape('html_attr') ~ '"' : null -%}
{%- set _dataBreakpointAttr = _breakpoint ? 'data-breakpoint="' ~ _breakpoint | escape('html_attr') ~ '"' : null -%}
{%- set _dataParentAttr = _parent ? 'data-parent="' ~ _parent | escape('html_attr') ~ '"' : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
Expand All @@ -24,9 +24,9 @@
<{{ _elementType }}
{{ mainProps(_mainPropsWithoutId) }}
{{ styleProp(_styleProps) }}
{{ _idAttr }}
{{ _dataBreakpointAttr }}
{{ _dataParentAttr }}
{{ _idAttr | raw }}
{{ _dataBreakpointAttr | raw }}
{{ _dataParentAttr | raw }}
{{ classProp(_classNames) }}
>
<{{ _elementType }} class="{{ _contentClassName }}">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
{%- set _leftClassName = _spiritClassPrefix ~ 'Dropdown--left' -%}

{# Attributes #}
{%- set _dataBreakpointAttr = _breakpoint ? 'data-breakpoint=' ~ _breakpoint : null -%}
{%- set _dataFullWidthModeAttr = _fullWidthMode ? 'data-fullwidthmode=' ~ _fullWidthMode : null -%}
{%- set _dataBreakpointAttr = _breakpoint ? 'data-breakpoint="' ~ _breakpoint | escape('html_attr') ~ '"' : null -%}
{%- set _dataFullWidthModeAttr = _fullWidthMode ? 'data-fullwidthmode="' ~ _fullWidthMode | escape('html_attr') ~ '"' : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
Expand All @@ -40,8 +40,8 @@
{{ mainProps(props) }}
{{ styleProp(_styleProps) }}
{{ classProp(_classNames) }}
{{ _dataBreakpointAttr }}
{{ _dataFullWidthModeAttr }}
{{ _dataBreakpointAttr | raw }}
{{ _dataFullWidthModeAttr | raw }}
>
{%- block content %}{% endblock -%}
</{{ _elementType }}>
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
{%- set _isCurrent = props.isCurrent | default(false) -%}
{%- set _onClick = props.onClick | default(null) -%}
{%- set _rootClass = props.rootClass -%}
{%- set _target = props.target | default(null) -%}

{# Class names #}
{%- set _rootClassName = _spiritClassPrefix ~ _rootClass -%}
Expand All @@ -14,23 +13,24 @@
{# Attributes #}
{%- set _ariaCurrentAttr = _isCurrent ? 'aria-current=page' : null -%}
{%- set _onClickAttr = _onClick ? 'onclick=' ~ _onClick | escape('html_attr') : null -%}
{%- set _targetAttr = _target ? 'target=' ~ _target : null -%}

{# Miscellaneous #}
{%- set _classNames = [ _rootClassName, _rootCurrentClassName, _class ] -%}
{%- set _allowedAttributes = [
'target',
] -%}

{# Deprecations #}
{% if _onClick %}
{% deprecated 'Header: The "onClick" property will be removed in the next major version. Use native "onclick" attribute instead.' %}
{% endif %}

<a
{{ mainProps(props) }}
{{ mainProps(props, _allowedAttributes) }}
{{ classProp(_classNames) }}
href="{{ _href }}"
{{ _ariaCurrentAttr }}
{{ _onClickAttr | raw }}
{{ _targetAttr }}
>
{% block content %}{% endblock %}
</a>
4 changes: 1 addition & 3 deletions packages/web-twig/src/Resources/components/Link/Link.twig
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
{%- set _isDisabled = props.isDisabled | default(false) | boolprop -%}
{%- set _isUnderlined = props.isUnderlined | default(false) | boolprop -%}
{%- set _onClick = props.onClick | default(null) -%}
{%- set _target = props.target | default(null) -%}

{# Class names #}
{%- set _colorClassName = _spiritClassPrefix ~ 'link-' ~ _color -%}
Expand All @@ -14,13 +13,13 @@

{# Attributes #}
{%- set _onClickAttr = _onClick ? 'onclick=' ~ _onClick | escape('html_attr') : null -%}
{%- set _targetAttr = _target ? 'target=' ~ _target : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
{%- set _classNames = [ _colorClassName, _rootDisabledClassName, _rootUnderlinedClassName, _styleProps.className ] -%}
{%- set _allowedAttributes = [
'title',
'target',
] -%}

{# Deprecations #}
Expand All @@ -34,7 +33,6 @@
{{ styleProp(_styleProps) }}
{{ classProp(_classNames) }}
{{ _onClickAttr | raw }}
{{ _targetAttr }}
>
{%- block content %}{% endblock %}
</a>
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
{%- set _rootClassName = _spiritClassPrefix ~ 'Modal' -%}

{# Attributes #}
{%- set _idAttr = _id ? 'id=' ~ _id : null -%}
{%- set _idAttr = _id ? 'id="' ~ _id | escape('html_attr') ~ '"' : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
{%- set _rootComposedClassName = _spiritClassPrefix ~ 'Modal--composed' -%}

{# Attributes #}
{%- set _idAttr = _id ? 'id=' ~ _id : null -%}
{%- set _ariaLabelledbyAttr = _id ? 'aria-labelledby=' ~ _titleId : null -%}
{%- set _idAttr = _id ? 'id="' ~ _id | escape('html_attr') ~ '"' : null -%}
{%- set _ariaLabelledbyAttr = _id ? 'aria-labelledby="' ~ _titleId | escape('html_attr') ~ '"' : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
Expand All @@ -23,8 +23,8 @@
{{ mainProps(_mainPropsWithoutId) }}
{{ styleProp(_styleProps) }}
{{ classProp(_classNames) }}
{{ _idAttr }}
{{ _ariaLabelledbyAttr }}
{{ _idAttr | raw }}
{{ _ariaLabelledbyAttr | raw }}
>
{% block content %}{% endblock %}
</dialog>
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
{%- set _titleClassName = _spiritClassPrefix ~ 'ModalHeader__title' -%}

{# Attributes #}
{%- set _titleIdAttr = _titleId ? 'id=' ~ _titleId : null -%}
{%- set _titleIdAttr = _titleId ? 'id="' ~ _titleId | escape('html_attr') ~ '"' : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
Expand All @@ -24,7 +24,7 @@
{% if block('content') is not empty %}
<h2
class="{{ _titleClassName }}"
{{ _titleIdAttr }}
{{ _titleIdAttr | raw }}
>
{% block content %}{% endblock %}
</h2>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,26 @@
{%- set _checkedAttr = _isChecked ? 'checked' : null -%}
{%- set _disabledAttr = _isDisabled ? 'disabled' : null -%}
{%- set _valueAttr = _value ? 'value=' ~ _value : null -%}
{%- set _idAttr = _id ? 'id=' ~ _id : null -%}
{%- set _labelForAttr = _id ? 'for=' ~ _id : null -%}
{%- set _autocompleteAttr = _autocomplete ? 'autocomplete=' ~ _autocomplete : null -%}
{%- set _idAttr = _id ? 'id="' ~ _id | escape('html_attr') ~ '"' : null -%}
{%- set _labelForAttr = _id ? 'for="' ~ _id | escape('html_attr') ~ '"' : null -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
{%- set _classNames = [ _rootClassName, _rootDisabledClassName, _rootItemClassName, _rootValidationStateClassName, _styleProps.className ] -%}
{%- set _labelClassName = [ _labelClassName, _labelHiddenClassName ] -%}
{%- set _mainPropsWithoutId = props | filter((value, prop) => prop != 'id') -%}
{%- set _allowedInputAttributes = [ 'autocomplete' ] -%}

<label {{ _labelForAttr }} {{ mainProps(_mainPropsWithoutId) }} {{ styleProp(_styleProps) }} {{ classProp(_classNames) }}>
<label {{ _labelForAttr | raw }} {{ mainProps(_mainPropsWithoutId) }} {{ styleProp(_styleProps) }} {{ classProp(_classNames) }}>
<input
{{ inputProps(props) }}
{{ inputProps(props, _allowedInputAttributes) }}
type="radio"
name="{{ _name }}"
class="{{ _inputClassName }}"
{{ _idAttr }}
{{ _idAttr | raw }}
{{ _checkedAttr }}
{{ _disabledAttr }}
{{ _valueAttr }} {# Intentionally without `raw` to prevent XSS. #}
{{ _autocompleteAttr }}
/>
<span class="{{ _textClassName }}">
<span {{ classProp(_labelClassName) }}>{% if _unsafeLabel %}{{ _unsafeLabel | raw }}{% else %}{{ _label }}{% endif %}</span>
Expand Down
1 change: 0 additions & 1 deletion packages/web-twig/src/Resources/components/Tabs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ There is no API for TabItem.
| Prop name | Type | Default | Required | Description |
| ------------ | --------- | ------- | -------- | ---------------------------- |
| `href` | `string` | `null` | no | URL target of a link |
| `id` | `string ` | `null` | no | Tab item identification |
| `isSelected` | `boolean` | `false` | no | Whether is tab item selected |
| `target` | `string` | `null` | no | Target tab pane ID |

Expand Down
28 changes: 12 additions & 16 deletions packages/web-twig/src/Resources/components/Tabs/TabLink.twig
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
{%- set props = props | default([]) -%}
{%- set _ariaTarget = props.ariaTarget | default(null) -%}
{%- set _href = props.href | default(null) -%}
{%- set _id = props.id | default(null) -%}
{%- set _isSelected = props.isSelected | default(false) | boolprop -%}
{%- set _target = props.target | default(null) -%}

Expand All @@ -11,32 +10,29 @@
{%- set _rootClassName = _spiritClassPrefix ~ 'Tabs__link' -%}

{# Attributes #}
{%- set _ariaControlsAttr = _target ? 'aria-controls=' ~ _target : null -%}
{%- set _dataTargetAttr = _target ? 'data-target=#' ~ _target : null -%}
{%- set _dataToggleAttr = _href ? null : 'data-toggle=tabs' -%}
{%- set _hrefAttr = _href ? 'href=' ~ _href : null -%}
{%- set _idAttr = _id ? ' id=' ~ _id : null -%}
{%- set _roleAttr = _href ? 'role=tab' : null -%}
{%- set _typeAttr = _href ? null : 'type=button' -%}
{%- set _ariaControlsAttr = _target ? 'aria-controls="' ~ _target | escape('html_attr') ~ '"' : null -%}
{%- set _dataTargetAttr = _target ? 'data-target="#' ~ _target | escape('html_attr') ~ '"' : null -%}
{%- set _dataToggleAttr = _href ? null : 'data-toggle="tabs"' -%}
{%- set _roleAttr = _href ? 'role="tab"' : null -%}
{%- set _typeAttr = _href ? null : 'type="button"' -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
{%- set _ariaSelected = _isSelected ? 'true' : 'false' -%}
{%- set _classNames = [ _rootClassName, _isSelectedClassName, _styleProps.className ] -%}
{%- set _elementType = _href ? 'a' : 'button' -%}
{%- set _allowedAttributes = _href ? [ 'href' ] : [] -%}

<{{ _elementType }}
{{ mainProps(props) }}
{{ mainProps(props, _allowedAttributes) }}
{{ styleProp(_styleProps) }}
{{ classProp(_classNames) }}
aria-selected="{{ _ariaSelected }}"
{{ _ariaControlsAttr }}
{{ _dataTargetAttr }}
{{ _dataToggleAttr }}
{{ _hrefAttr }}
{{ _idAttr }}
{{ _roleAttr }}
{{ _typeAttr }}
{{ _ariaControlsAttr | raw }}
{{ _dataTargetAttr | raw }}
{{ _dataToggleAttr | raw }}
{{ _roleAttr | raw }}
{{ _typeAttr | raw }}
>
{% block content %}{% endblock %}
</{{ _elementType }}>
Loading

0 comments on commit 42176b2

Please sign in to comment.