[TogglePassword] htmlspecialchars for button-classes

[Janvdv]

When passing the button-classes option to the PasswordType form type while overwriting default input templates, the attributes becomes mangled. The attributes are not properly escaped.

escaping happens here: https://github.com/symfony/ux-toggle-password/blob/2.x/src/Form/TogglePasswordTypeExtension.php#L94

 $controllerValues['button-classes'] = json_encode($options['button_classes'], \JSON_THROW_ON_ERROR);

We have a temporary fix in place:

$controllerValues['button-classes'] = htmlspecialchars(json_encode($options['button_classes'], \JSON_THROW_ON_ERROR), ENT_QUOTES, 'UTF-8');

before fix: data-symfony--ux-toggle-password--toggle-password-button-classes-value="[" toggle-password-button"]"

After fix: data-symfony--ux-toggle-password--toggle-password-button-classes-value="["toggle-password-button"]"

[Janvdv]

I have a fork with the fix as well:

https://github.com/janvdv96/ux-toggle-password/blob/2.x/src/Form/TogglePasswordTypeExtension.php#L94

[jmsche]

Instead of reinventing the wheel, the component could require the Stimulus bundle, and then use the Stimulus helper to create controllers with correctly formatted attributes?

[smnandre]

@janvdv96 the htmlspecialchars should not be done before rendering imho.

Do you have an example where this does not work ?

I just tested the ux.symfony.com toggle-password demo code locally, and added two classes with

class TogglePasswordForm extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder
            ->add('email', EmailType::class)
            ->add('password', PasswordType::class, [
                'toggle' => true,
                'button_classes' => ["btn", "btn-primary"],
            ])
        ;
    }
}

and everything seems to work for me.

[Janvdv]

Because of the way we pass attributes to our fields, we would expect the attributes to be properly escaped beforehand

sulu:

{% block attributes -%}
    {% set attr = attr|merge({width: false, widthNumber: false, lastWidth: false}) %}
    {{- parent() -}}
{%- endblock attributes -%}

twig-bridge:

{% block attributes -%}
    {%- for attrname, attrvalue in attr -%}
        {{- " " -}}
        {%- if attrname in ['placeholder', 'title'] -%}
            {{- attrname }}="{{ translation_domain is same as(false) or attrvalue is null ? attrvalue : attrvalue|trans(attr_translation_parameters, translation_domain) }}"
        {%- elseif attrvalue is same as(true) -%}
            {{- attrname }}="{{ attrname }}"
        {%- elseif attrvalue is not same as(false) -%}
            {{- attrname }}="{{ attrvalue }}"
        {%- endif -%}
    {%- endfor -%}
{%- endblock attributes -%}

Perhaps this should be a change in the Sulu bundle instead