Problem/Motivation

This was initially reported in #3525103: Allow Xss::filter() to restrict allowed attributes.

Having the strict argument set to true is a good practice, otherwise it can lead to unexpected behavior (see #3295295: [META] Always call in_array() in strict mode).

Steps to reproduce

Proposed resolution

Add the strict argument.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3608303

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

prudloff created an issue. See original summary.

prudloff opened merge request !16213

prudloff’s picture

prudloff commented
Status: Active » Needs review
ghost of drupal past’s picture

ghost of drupal past
he/him
commented

this is superfluous because unlike the issue i reported in you do know the type of the needle and the haystack both:

$attribute_name = strtolower($match[1]);
$skip = (
in_array($attribute_name, ['style', 'srcdoc']

the needle is a string since it comes from strtolower and the haystack is a list of fixed non empty strings so there's no need to strict match them. In the other issue I raised a red flag because heaven knows what's in allowed_attributes

of course it doesn't hurt but it's not necessary either

smustgrave’s picture

smustgrave commented

@prudloff thoughts for arguing for?

prudloff’s picture

prudloff commented

I don't have a strong opinion about this.
ghost of drupal past is probably right and I might have been a bit too quick to create this issue.

That said, I think we should have a CI rule that forces using strict mode everywhere (this is proposed in #3295295: [META] Always call in_array() in strict mode).
So we could either:

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

MR is already up lets see what committers say

catch made their first commit to this issue’s fork.

  • catch committed e67d0fbd on 11.x 
    task: #3608303 in_array() calls in Xss::filter() don't have strict...

  • catch committed 4a4d1096 on main 
    task: #3608303 in_array() calls in Xss::filter() don't have strict...
catch’s picture

catch
he/him
Primary language English
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Yeah while this might not fix anything it will be one less thing to review when we introduce the rule and has no downside.

Committed/pushed to main and 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !16213

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.